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

Submitted by Amir Pauker on June 10, 2019, 3:03 a.m.

Details

Message ID 1560135808-31961-1-git-send-email-amir@livelyvideo.tv
State New
Headers show

Commit Message

Amir Pauker June 10, 2019, 3:03 a.m.
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@livelyvideo.tv>
---
 libavcodec/h264dec.c | 7 +++++++
 libavutil/frame.h    | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

Comments

James Almer June 10, 2019, 3:07 a.m.
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@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.
>
Amir Pauker June 10, 2019, 3:32 a.m.
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.

Thanks


On Sun, Jun 9, 2019 at 10:15 PM James Almer <jamrial@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@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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
James Almer June 10, 2019, 3:52 a.m.
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@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@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@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Amir Pauker June 10, 2019, 3:52 a.m.
Sounds good, thanks I will submit a new patch

On Sun, Jun 9, 2019 at 10:53 PM James Almer <jamrial@gmail.com> wrote:

> 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@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@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@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Patch hide | download patch | download mbox

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
 
     /**
      * number of audio channels, only used for audio.