[MPlayer-dev-eng] [PATCH v3] loosen requirements on error resilence flags

[Reimar Döffinger]

On Fri, Feb 10, 2012 at 09:48:36PM +0100, Reimar Döffinger wrote:
> On Fri, Feb 10, 2012 at 09:23:44PM +0100, Reinhard Tartler wrote:
> > On Fri, Feb 10, 2012 at 7:41 PM, Reimar Döffinger
> > <Reimar.Doeffinger at gmx.de> wrote:
> > >
> > >
> > > On 10 Feb 2012, at 19:19, Reinhard Tartler <siretart at gmail.com> wrote:
> > >
> > >> On Mon, Feb 6, 2012 at 7:32 AM, Reinhard Tartler <siretart at gmail.com> wrote:
> > >>> $subj
> > >>
> > >> Variant two.
> > >>
> > >> This patch effectively restores the behavior prior to svn r34634 if
> > >> those offending macros are not defined.
> > >> By this, compilation with older releases of FFmpeg become easier.
> > >
> > > That makes no sense, you did not restore the old behaviour. The MPlayer option takes values from 0 to 4 or so, err_recognition takes flags.
> > > If you want to restore the old behaviour you have to assign error_recognition, but if you do that by checking for the flags you'll break compilation with current Libav.
> > 
> > I see. Right, I clearly misunderstood the point of the FF_API_ER deprecation.
> > 
> > Variant 3 attached.
> > 
> > -- 
> > regards,
> >     Reinhard
> 
> > Index: libmpcodecs/vd_ffmpeg.c
> > ===================================================================
> > --- libmpcodecs/vd_ffmpeg.c	(Revision 34684)
> > +++ libmpcodecs/vd_ffmpeg.c	(Arbeitskopie)
> > @@ -266,6 +266,7 @@
> >      avctx->coded_width = sh->disp_w;
> >      avctx->coded_height= sh->disp_h;
> >      avctx->workaround_bugs= lavc_param_workaround_bugs;
> > +#if defined AV_EF_COMPLIANT && defined AV_EF_CAREFUL && defined AV_EF_AGGRESSIVE
> >      switch (lavc_param_error_resilience) {
> >      case 5:
> >          avctx->err_recognition |= AV_EF_EXPLODE | AV_EF_COMPLIANT | AV_EF_CAREFUL;
> > @@ -280,6 +281,11 @@
> >      case 1:
> >          avctx->err_recognition |= AV_EF_CAREFUL;
> >      }
> > +#else
> > +    /* FF_ER_CAREFUL   (==1) implies AV_EF_CRCCHECK (== 1<<1 - 1),
> > +       FF_ER_COMPLIANT (==2) implies AV_EF_{CRCCHECK,BITSTREAM} (== 1<<2 - 1), et cetera} */
> > +    avctx->err_recognition |= (1<<(lavc_param_error_resilience)) - 1;
> 
> This is rather obfuscated and depends on internals like the exact values
> of AV_EF.
> The bigger problem however is that these flags only go by _type_ of
> error, however the point of lavc_param_error_resilience is to allow
> selecting them by _severity_.
> E.g. AV_EF_BUFFER treats both the MP3 decoder running out of bits
> and bits being left over as an error, but running out of bits is
> clearly a far severe problem, the other is most likely just a sloppy
> encoder.
> I don't see the point of mapping that option if it can't be
> mapped to anything compatible with the old meaning.
> The err_recognition setting is still available via the generic
> o=... settings that allow setting any option through FFmpeg's
> option system.

Hm, I see you probably reacted to Alexander's comments.
Sorry if you feel pushed back and forth here...
I don't really consider that option all that important.
Thus at least erroring out seems a bit extreme, but whatever
people want...
I'd be in favour of when it is set print something like
"... option is not available with your libavcodec version,
similar functionality is available via o=err_detect=...".