| Message ID | 1560145513-2130-1-git-send-email-amir@livelyvideo.tv |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, Jun 09, 2019 at 10:45:13PM -0700, 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 | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c > index 00d922f..67dee11 100644 > --- a/libavcodec/h264dec.c > +++ b/libavcodec/h264dec.c > @@ -758,6 +758,12 @@ 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_INVALID_BITSTREAM|FF_DECODE_ERROR_MISSING_REFERENCE; > + } This is not correct. error_occurred does not imply the 2 specific errors [...]
Thanks Michael Niedermayer for looking into this What I am trying to solve is having a way to detect concealed decoding errors by the caller to avcodec_receive_frame. Should I add a general value e.g. #define FF_DECODE_ERROR_DECODE_ERROR_OCCURRED 4 ? Thanks Amir On Tue, Jun 11, 2019 at 11:39 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sun, Jun 09, 2019 at 10:45:13PM -0700, 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 | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c > > index 00d922f..67dee11 100644 > > --- a/libavcodec/h264dec.c > > +++ b/libavcodec/h264dec.c > > @@ -758,6 +758,12 @@ 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_INVALID_BITSTREAM|FF_DECODE_ERROR_MISSING_REFERENCE; > > + } > > This is not correct. > error_occurred does not imply the 2 specific errors > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > What does censorship reveal? It reveals fear. -- Julian Assange > _______________________________________________ > 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 Tue, Jun 11, 2019 at 03:21:41PM -0500, Amir Z wrote: > Thanks Michael Niedermayer for looking into this > > What I am trying to solve is having a way to detect concealed decoding > errors by the caller to avcodec_receive_frame. > > Should I add a general value e.g. #define > FF_DECODE_ERROR_DECODE_ERROR_OCCURRED 4 ? I suggest FF_DECODE_ERROR_CONCEALMENT_ACTIVE or something similar and then always set this for all cases of error concealment Its more informative than just knowing there was an error also dont forget to increase minor version and add a entry to APICHanges thx [...]
On Wed, 12 Jun 2019, Michael Niedermayer wrote: > On Tue, Jun 11, 2019 at 03:21:41PM -0500, Amir Z wrote: >> Thanks Michael Niedermayer for looking into this >> >> What I am trying to solve is having a way to detect concealed decoding >> errors by the caller to avcodec_receive_frame. >> >> Should I add a general value e.g. #define >> FF_DECODE_ERROR_DECODE_ERROR_OCCURRED 4 ? > > I suggest > FF_DECODE_ERROR_CONCEALMENT_ACTIVE or something similar and then always > set this for all cases of error concealment > Its more informative than just knowing there was an error Concealment is a consequence. Error_flags should refer to the cause. A generic UNKNOWN error seems much better to me if it is not feasible to determine the cause. Regards, Marton
On Wed, Jun 12, 2019 at 10:09:08AM +0200, Marton Balint wrote: > > > On Wed, 12 Jun 2019, Michael Niedermayer wrote: > > >On Tue, Jun 11, 2019 at 03:21:41PM -0500, Amir Z wrote: > >>Thanks Michael Niedermayer for looking into this > >> > >>What I am trying to solve is having a way to detect concealed decoding > >>errors by the caller to avcodec_receive_frame. > >> > >>Should I add a general value e.g. #define > >>FF_DECODE_ERROR_DECODE_ERROR_OCCURRED 4 ? > > > >I suggest > >FF_DECODE_ERROR_CONCEALMENT_ACTIVE or something similar and then always > >set this for all cases of error concealment > >Its more informative than just knowing there was an error > > Concealment is a consequence. Error_flags should refer to the cause. A > generic UNKNOWN error seems much better to me if it is not feasible to > determine the cause. Concealment is the consequence generally, still the error in the frames differs between concealment or no concealment. A user application may want to treat these differently. concealemnt is not supported by all codecs currently and also not by all variants, for example interlaced material tends to be less supported in concealment. A user app might choose to discard a frame that contains errors but no concealemnt if the following frame is fine. Also in a very pedantic view, concealemnt itself is an error too. Its rarly known exactly where the damage starts so concealment often needs to cover more and by doing so adds errors in a minority of locations that is if you just want a formal argument why this would fit in here. Not an argument against the principle that concealemnt differs here in what it is, you are certainly correct about that. [...]
FF_DECODE_ERROR_CONCEALMENT_ACTIVE sounds right for the case that the
ret variable is set to zero (i.e. indicate the fact that there was an
error and it is concealed)
ret = ff_h264_execute_decode_slices(h);
if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE))
goto end;
// set h->cur_pic_ptr->f->decode_error_flags here
ret = 0;
end:
Thank
On Wed, Jun 12, 2019 at 4:35 AM Michael Niedermayer <michael@niedermayer.cc>
wrote:
> On Wed, Jun 12, 2019 at 10:09:08AM +0200, Marton Balint wrote:
> >
> >
> > On Wed, 12 Jun 2019, Michael Niedermayer wrote:
> >
> > >On Tue, Jun 11, 2019 at 03:21:41PM -0500, Amir Z wrote:
> > >>Thanks Michael Niedermayer for looking into this
> > >>
> > >>What I am trying to solve is having a way to detect concealed decoding
> > >>errors by the caller to avcodec_receive_frame.
> > >>
> > >>Should I add a general value e.g. #define
> > >>FF_DECODE_ERROR_DECODE_ERROR_OCCURRED 4 ?
> > >
> > >I suggest
> > >FF_DECODE_ERROR_CONCEALMENT_ACTIVE or something similar and then always
> > >set this for all cases of error concealment
> > >Its more informative than just knowing there was an error
> >
> > Concealment is a consequence. Error_flags should refer to the cause. A
> > generic UNKNOWN error seems much better to me if it is not feasible to
> > determine the cause.
>
> Concealment is the consequence generally, still the error in the frames
> differs between concealment or no concealment. A user application may
> want to treat these differently.
> concealemnt is not supported by all codecs currently and also not by
> all variants, for example interlaced material tends to be less supported
> in concealment. A user app might choose to discard a frame that
> contains errors but no concealemnt if the following frame is fine.
>
> Also in a very pedantic view, concealemnt itself is an error too.
> Its rarly known exactly where the damage starts so concealment often needs
> to cover more and by doing so adds errors in a minority of locations
> that is if you just want a formal argument why this would fit in here.
> Not an argument against the principle that concealemnt differs here in what
> it is, you are certainly correct about that.
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Modern terrorism, a quick summary: Need oil, start war with country that
> has oil, kill hundread thousand in war. Let country fall into chaos,
> be surprised about raise of fundamantalists. Drop more bombs, kill more
> people, be surprised about them taking revenge and drop even more bombs
> and strip your own citizens of their rights and freedoms. to be continued
> _______________________________________________
> 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 Wed, 12 Jun 2019, Michael Niedermayer wrote: > On Wed, Jun 12, 2019 at 10:09:08AM +0200, Marton Balint wrote: >> >> >> On Wed, 12 Jun 2019, Michael Niedermayer wrote: >> >>> On Tue, Jun 11, 2019 at 03:21:41PM -0500, Amir Z wrote: >>>> Thanks Michael Niedermayer for looking into this >>>> >>>> What I am trying to solve is having a way to detect concealed decoding >>>> errors by the caller to avcodec_receive_frame. >>>> >>>> Should I add a general value e.g. #define >>>> FF_DECODE_ERROR_DECODE_ERROR_OCCURRED 4 ? >>> >>> I suggest >>> FF_DECODE_ERROR_CONCEALMENT_ACTIVE or something similar and then always >>> set this for all cases of error concealment >>> Its more informative than just knowing there was an error >> >> Concealment is a consequence. Error_flags should refer to the cause. A >> generic UNKNOWN error seems much better to me if it is not feasible to >> determine the cause. > > Concealment is the consequence generally, still the error in the frames > differs between concealment or no concealment. A user application may > want to treat these differently. > concealemnt is not supported by all codecs currently and also not by > all variants, for example interlaced material tends to be less supported > in concealment. A user app might choose to discard a frame that > contains errors but no concealemnt if the following frame is fine. > > Also in a very pedantic view, concealemnt itself is an error too. > Its rarly known exactly where the damage starts so concealment often needs > to cover more and by doing so adds errors in a minority of locations > that is if you just want a formal argument why this would fit in here. > Not an argument against the principle that concealemnt differs here in what > it is, you are certainly correct about that. OK, FF_DECODE_ERROR_CONCEALMENT_ACTIVE is fine then. Thanks, Marton
thanks, I will submit a new patch with the requested changes On Wed, Jun 12, 2019 at 3:00 PM Marton Balint <cus@passwd.hu> wrote: > > > On Wed, 12 Jun 2019, Michael Niedermayer wrote: > > > On Wed, Jun 12, 2019 at 10:09:08AM +0200, Marton Balint wrote: > >> > >> > >> On Wed, 12 Jun 2019, Michael Niedermayer wrote: > >> > >>> On Tue, Jun 11, 2019 at 03:21:41PM -0500, Amir Z wrote: > >>>> Thanks Michael Niedermayer for looking into this > >>>> > >>>> What I am trying to solve is having a way to detect concealed decoding > >>>> errors by the caller to avcodec_receive_frame. > >>>> > >>>> Should I add a general value e.g. #define > >>>> FF_DECODE_ERROR_DECODE_ERROR_OCCURRED 4 ? > >>> > >>> I suggest > >>> FF_DECODE_ERROR_CONCEALMENT_ACTIVE or something similar and then always > >>> set this for all cases of error concealment > >>> Its more informative than just knowing there was an error > >> > >> Concealment is a consequence. Error_flags should refer to the cause. A > >> generic UNKNOWN error seems much better to me if it is not feasible to > >> determine the cause. > > > > Concealment is the consequence generally, still the error in the frames > > differs between concealment or no concealment. A user application may > > want to treat these differently. > > concealemnt is not supported by all codecs currently and also not by > > all variants, for example interlaced material tends to be less supported > > in concealment. A user app might choose to discard a frame that > > contains errors but no concealemnt if the following frame is fine. > > > > Also in a very pedantic view, concealemnt itself is an error too. > > Its rarly known exactly where the damage starts so concealment often > needs > > to cover more and by doing so adds errors in a minority of locations > > that is if you just want a formal argument why this would fit in here. > > Not an argument against the principle that concealemnt differs here in > what > > it is, you are certainly correct about that. > > OK, FF_DECODE_ERROR_CONCEALMENT_ACTIVE is fine then. > > Thanks, > Marton > _______________________________________________ > 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..67dee11 100644 --- a/libavcodec/h264dec.c +++ b/libavcodec/h264dec.c @@ -758,6 +758,12 @@ 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_INVALID_BITSTREAM|FF_DECODE_ERROR_MISSING_REFERENCE; + } + if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE)) goto end;
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 | 6 ++++++ 1 file changed, 6 insertions(+)