Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
Quoting James Almer (2021-05-24 19:20:19)
> On 7/7/2019 5:15 PM, Michael Niedermayer wrote:
> > On Fri, Jun 21, 2019 at 07:15:17AM -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
> >> ---
> >> libavcodec/error_resilience.c | 2 ++
> >> libavcodec/h264dec.c | 5 +
> >> 2 files changed, 7 insertions(+)
> >>
> >> diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
> >> index 35d0c60..ca22871 100644
> >> --- a/libavcodec/error_resilience.c
> >> +++ b/libavcodec/error_resilience.c
> >> @@ -1121,6 +1121,8 @@ void ff_er_frame_end(ERContext *s)
> >> av_log(s->avctx, AV_LOG_INFO, "concealing %d DC, %d AC, %d MV errors
> >> in %c frame\n",
> >> dc_error, ac_error, mv_error,
> >> av_get_picture_type_char(s->cur_pic.f->pict_type));
> >>
> >> +s->cur_pic.f->decode_error_flags |=
> >> FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> >> +
> >> is_intra_likely = is_intra_more_likely(s);
> >>
> >> /* set unknown mb-type to most likely */
> >> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> >> index 837c3b7..8d1bd16 100644
> >> --- a/libavcodec/h264dec.c
> >> +++ b/libavcodec/h264dec.c
> >> @@ -761,6 +761,11 @@ static int decode_nal_units(H264Context *h, const
> >> uint8_t *buf, int buf_size)
> >> if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE))
> >> goto end;
> >>
> >> +// 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_SLICES;
> >> +}
> >> +
> >> ret = 0;
> >> end:
> >
> > will split and apply
> >
> > thx
>
> This change seems to have produced FATE failures when run under tsan.
> http://fate.ffmpeg.org/report.cgi?time=20210524021410=x86_64-archlinux-gcc-tsan
>
> That being said, TSAN runs show a lot of issues that have remained
> unresolved for a long while. No idea if they are false positives, or if
> they are benign and rarely (if at all) have any effect on real world
> usage (Like this one, apparently), but what i can say is that they are
> masking new and potentially bad threading bugs that nobody will notice
> in a timely manner.
Modifying frames in the DPB is a race, this flag should be applied on
the output frame rather than the DPB frame.
--
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
On 7/7/2019 5:15 PM, Michael Niedermayer wrote:
On Fri, Jun 21, 2019 at 07:15:17AM -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
---
libavcodec/error_resilience.c | 2 ++
libavcodec/h264dec.c | 5 +
2 files changed, 7 insertions(+)
diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
index 35d0c60..ca22871 100644
--- a/libavcodec/error_resilience.c
+++ b/libavcodec/error_resilience.c
@@ -1121,6 +1121,8 @@ void ff_er_frame_end(ERContext *s)
av_log(s->avctx, AV_LOG_INFO, "concealing %d DC, %d AC, %d MV errors in %c
frame\n",
dc_error, ac_error, mv_error,
av_get_picture_type_char(s->cur_pic.f->pict_type));
+s->cur_pic.f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
+
is_intra_likely = is_intra_more_likely(s);
/* set unknown mb-type to most likely */
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 837c3b7..8d1bd16 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -761,6 +761,11 @@ static int decode_nal_units(H264Context *h, const uint8_t
*buf, int buf_size)
if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE))
goto end;
+// 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_SLICES;
+}
+
ret = 0;
end:
will split and apply
thx
This change seems to have produced FATE failures when run under tsan.
http://fate.ffmpeg.org/report.cgi?time=20210524021410=x86_64-archlinux-gcc-tsan
That being said, TSAN runs show a lot of issues that have remained
unresolved for a long while. No idea if they are false positives, or if
they are benign and rarely (if at all) have any effect on real world
usage (Like this one, apparently), but what i can say is that they are
masking new and potentially bad threading bugs that nobody will notice
in a timely manner.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
Thanks
On Sun, Jul 7, 2019 at 3:38 PM Michael Niedermayer
wrote:
> On Fri, Jun 21, 2019 at 07:15:17AM -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
> > ---
> > libavcodec/error_resilience.c | 2 ++
> > libavcodec/h264dec.c | 5 +
> > 2 files changed, 7 insertions(+)
> >
> > diff --git a/libavcodec/error_resilience.c
> b/libavcodec/error_resilience.c
> > index 35d0c60..ca22871 100644
> > --- a/libavcodec/error_resilience.c
> > +++ b/libavcodec/error_resilience.c
> > @@ -1121,6 +1121,8 @@ void ff_er_frame_end(ERContext *s)
> > av_log(s->avctx, AV_LOG_INFO, "concealing %d DC, %d AC, %d MV
> errors in %c frame\n",
> > dc_error, ac_error, mv_error,
> av_get_picture_type_char(s->cur_pic.f->pict_type));
> >
> > +s->cur_pic.f->decode_error_flags |=
> FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> > +
> > is_intra_likely = is_intra_more_likely(s);
> >
> > /* set unknown mb-type to most likely */
> > diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> > index 837c3b7..8d1bd16 100644
> > --- a/libavcodec/h264dec.c
> > +++ b/libavcodec/h264dec.c
> > @@ -761,6 +761,11 @@ static int decode_nal_units(H264Context *h, const
> uint8_t *buf, int buf_size)
> > if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE))
> > goto end;
> >
> > +// 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_SLICES;
> > +}
> > +
> > ret = 0;
> > end:
>
> will split and apply
>
> thx
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Whats the most studid thing your enemy could do ? Blow himself up
> Whats the most studid thing you could do ? Give up your rights and
> freedom because your enemy blew himself up.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
On Fri, Jun 21, 2019 at 07:15:17AM -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
> ---
> libavcodec/error_resilience.c | 2 ++
> libavcodec/h264dec.c | 5 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
> index 35d0c60..ca22871 100644
> --- a/libavcodec/error_resilience.c
> +++ b/libavcodec/error_resilience.c
> @@ -1121,6 +1121,8 @@ void ff_er_frame_end(ERContext *s)
> av_log(s->avctx, AV_LOG_INFO, "concealing %d DC, %d AC, %d MV errors in
> %c frame\n",
> dc_error, ac_error, mv_error,
> av_get_picture_type_char(s->cur_pic.f->pict_type));
>
> +s->cur_pic.f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +
> is_intra_likely = is_intra_more_likely(s);
>
> /* set unknown mb-type to most likely */
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 837c3b7..8d1bd16 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -761,6 +761,11 @@ static int decode_nal_units(H264Context *h, const
> uint8_t *buf, int buf_size)
> if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE))
> goto end;
>
> +// 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_SLICES;
> +}
> +
> ret = 0;
> end:
will split and apply
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Whats the most studid thing your enemy could do ? Blow himself up
Whats the most studid thing you could do ? Give up your rights and
freedom because your enemy blew himself up.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
Michael hey,
Could you please apply this patch as well.
Thanks
On Fri, Jun 21, 2019 at 9:15 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
> ---
> libavcodec/error_resilience.c | 2 ++
> libavcodec/h264dec.c | 5 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
> index 35d0c60..ca22871 100644
> --- a/libavcodec/error_resilience.c
> +++ b/libavcodec/error_resilience.c
> @@ -1121,6 +1121,8 @@ void ff_er_frame_end(ERContext *s)
> av_log(s->avctx, AV_LOG_INFO, "concealing %d DC, %d AC, %d MV errors
> in %c frame\n",
> dc_error, ac_error, mv_error,
> av_get_picture_type_char(s->cur_pic.f->pict_type));
>
> +s->cur_pic.f->decode_error_flags |=
> FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +
> is_intra_likely = is_intra_more_likely(s);
>
> /* set unknown mb-type to most likely */
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 837c3b7..8d1bd16 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -761,6 +761,11 @@ static int decode_nal_units(H264Context *h, const
> uint8_t *buf, int buf_size)
> if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE))
> goto end;
>
> +// 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_SLICES;
> +}
> +
> ret = 0;
> end:
>
> --
> 2.1.4
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
thanks Moritz
i submitted two new patches
On Fri, Jun 14, 2019 at 2:43 AM Moritz Barsnick wrote:
> On Thu, Jun 13, 2019 at 08:52:22 -0700, Amir Pauker wrote:
> > doc/APIchanges | 3 +++
> > libavcodec/h264dec.c | 5 +
> > libavutil/frame.h| 1 +
> > libavutil/version.h | 2 +-
>
> You should split the introduction of the API (libavutil and doc) and
> the use of the API (libavcodec) into two separate patches.
>
> > +if( (ret < 0 || h->slice_ctx->er.error_occurred) && h->cur_pic_ptr){
>
> Please fix the bracket/whitespace style:
> https://ffmpeg.org/developer.html#Code-formatting-conventions
>
> ->
> if ((ret < 0 || h->slice_ctx->er.error_occurred) && h->cur_pic_ptr) {
>
> Moritz
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
On Thu, Jun 13, 2019 at 08:52:22 -0700, Amir Pauker wrote:
> doc/APIchanges | 3 +++
> libavcodec/h264dec.c | 5 +
> libavutil/frame.h| 1 +
> libavutil/version.h | 2 +-
You should split the introduction of the API (libavutil and doc) and
the use of the API (libavcodec) into two separate patches.
> +if( (ret < 0 || h->slice_ctx->er.error_occurred) && h->cur_pic_ptr){
Please fix the bracket/whitespace style:
https://ffmpeg.org/developer.html#Code-formatting-conventions
->
if ((ret < 0 || h->slice_ctx->er.error_occurred) && h->cur_pic_ptr) {
Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
thanks, I will submit a new patch with the requested changes On Wed, Jun 12, 2019 at 3:00 PM Marton Balint 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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
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 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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
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 signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
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 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Avoid a single point of failure, be that a person or equipment. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
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
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
> > ---
> > 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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
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
> ---
> 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
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
Sounds good, thanks I will submit a new patch
On Sun, Jun 9, 2019 at 10:53 PM James Almer 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 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
> >>> ---
> >>> 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_ERROR2
> >>
> >> 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-requ...@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-requ...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
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 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
>>> ---
>>> 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_ERROR2
>>
>> 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-requ...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
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 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
> > ---
> > 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_ERROR2
>
> 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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
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
> ---
> 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_ERROR2
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-requ...@ffmpeg.org with subject "unsubscribe".
