[FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec

[James Almer]

On 6/10/2019 12:32 AM, Amir Z wrote:
> As far as I can tell  FF_DECODE_ERROR_MISSING_REFERENCE is not used
> anywhere.
> 
> Since the new flag DECODE_ERROR covers also missing reference case  i
> thought it will be more appropriate to replace it instead of adding a new
> enum value.

Removing a define or a symbol from a public header is an API break, and
doing so requires a deprecation period. That flag may not be currently
set by any decoder, but API users may be checking for it on their code
as it could start being used anytime.

You could just set FF_DECODE_ERROR_INVALID_BITSTREAM,
FF_DECODE_ERROR_MISSING_REFERENCE, or both, since as you mentioned they
describe the errors that could take place in
ff_h264_execute_decode_slices() just fine.

> 
> Thanks
> 
> 
> On Sun, Jun 9, 2019 at 10:15 PM James Almer <jamrial at gmail.com> wrote:
> 
>> On 6/10/2019 12:03 AM, Amir Pauker wrote:
>>> set AVFrame decode_error_flags in case h->slice_ctx->er.error_occurred
>> is set
>>> after the call to ff_h264_execute_decode_slices. This allows the user to
>> detect
>>> concealed decoding errors in the call to avcodec_receive_frame
>>>
>>> Signed-off-by: Amir Pauker <amir at livelyvideo.tv>
>>> ---
>>>  libavcodec/h264dec.c | 7 +++++++
>>>  libavutil/frame.h    | 2 +-
>>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
>>> index 00d922f..9f038e9 100644
>>> --- a/libavcodec/h264dec.c
>>> +++ b/libavcodec/h264dec.c
>>> @@ -758,6 +758,13 @@ static int decode_nal_units(H264Context *h, const
>> uint8_t *buf, int buf_size)
>>>      }
>>>
>>>      ret = ff_h264_execute_decode_slices(h);
>>> +
>>> +    // set decode_error_flags to allow users to detect concealed
>> decoding errors
>>> +    if( (ret < 0 || h->slice_ctx->er.error_occurred) && h->cur_pic_ptr){
>>> +        h->cur_pic_ptr->f->decode_error_flags |=
>> FF_DECODE_ERROR_DECODE_ERROR;
>>> +    }
>>> +
>>> +
>>>      if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE))
>>>          goto end;
>>>
>>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>>> index e2a2929..ef1ff6b 100644
>>> --- a/libavutil/frame.h
>>> +++ b/libavutil/frame.h
>>> @@ -521,7 +521,7 @@ typedef struct AVFrame {
>>>       */
>>>      int decode_error_flags;
>>>  #define FF_DECODE_ERROR_INVALID_BITSTREAM   1
>>> -#define FF_DECODE_ERROR_MISSING_REFERENCE   2
>>> +#define FF_DECODE_ERROR_DECODE_ERROR        2
>>
>> This is an API breaking change. Why are you removing
>> FF_DECODE_ERROR_MISSING_REFERENCE if what you want is adding a new flag?
>>
>>>
>>>      /**
>>>       * number of audio channels, only used for audio.
>>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>