bad exception handling mechanism in IntelH264Decoder

[shyaki]

If you have studied the IntelH264Decoder sample code, you will find h264_exception(UMC_ERR_INVALID_STREAM)in many places which throw the exception whenever the decoding process detects some error, for example, corruption in the CAVLC data. This exception will be caught by the top-level funciton H264VideoDecoder::GetFrame() and m_pTaskSupplier->AfterErrorRestore() will be called to process this exception.
In the AfterErrorRestore method, all the data including reference pics will be cleared and the decoder will be in fact restored to the intial status.
This is very catastropic since a small corruption in a frame may cause an exception and the decoder will be cleared and the subsequent frames will be lost.
I dont think this is a good way to handle errors. A robust decoder should be able to recover from where the error happens and do the best to continue to decode even though the output may be corrupted. This is much better than losing frames.

[shyaki]

Quoting - shyaki

If you have studied the IntelH264Decoder sample code, you will find h264_exception(UMC_ERR_INVALID_STREAM)in many places which throw the exception whenever the decoding process detects some error, for example, corruption in the CAVLC data. This exception will be caught by the top-level funciton H264VideoDecoder::GetFrame() and m_pTaskSupplier->AfterErrorRestore() will be called to process this exception.
In the AfterErrorRestore method, all the data including reference pics will be cleared and the decoder will be in fact restored to the intial status.
This is very catastropic since a small corruption in a frame may cause an exception and the decoder will be cleared and the subsequent frames will be lost.
I dont think this is a good way to handle errors. A robust decoder should be able to recover from where the error happens and do the best to continue to decode even though the output may be corrupted. This is much better than losing frames.

In the contructor ofclass h264_exception, ASSERT should be added, so that we can at least have the change to break into the call stack when the exception occurs in the right place.

[Emmanuel_W_]

Quoting - shyaki

If you have studied the IntelH264Decoder sample code, you will find h264_exception(UMC_ERR_INVALID_STREAM)in many places which throw the exception whenever the decoding process detects some error, for example, corruption in the CAVLC data. This exception will be caught by the top-level funciton H264VideoDecoder::GetFrame() and m_pTaskSupplier->AfterErrorRestore() will be called to process this exception.
In the AfterErrorRestore method, all the data including reference pics will be cleared and the decoder will be in fact restored to the intial status.
This is very catastropic since a small corruption in a frame may cause an exception and the decoder will be cleared and the subsequent frames will be lost.
I dont think this is a good way to handle errors. A robust decoder should be able to recover from where the error happens and do the best to continue to decode even though the output may be corrupted. This is much better than losing frames.

I believe most of the time the exception are caught in the ProcessSegment function which turns the exception into an error code, and do not end up callingAfterErrorRestore().

If I am not mistaking, the exception handler in TaskSupplier that do end up callingAfterErrorRestore is mainly triggered by decoding errors when parsing the headers (SPS/PPS) which are usually not recoverable (although I see some exception thrown by the SEI parser).

An issue I am having with the current implementation is that sometimes the CAVLC functions actually crash. Those crash are caught by the catch(...) statement but I wonder how much trust I can put into the integrity of the application after such incident.

Emmanuel