| Message ID | 1560135808-31961-1-git-send-email-amir@livelyvideo.tv |
|---|---|
| State | Superseded |
| Headers | show |
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. >
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".
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". >
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".
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.
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(-)