[FFmpeg-devel] [PATCH] fix issue434

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Mon, Apr 21, 2008 at 02:04:08AM +0200, Michael Niedermayer wrote:
>> On Mon, Apr 21, 2008 at 12:21:07AM +0100, M?ns Rullg?rd wrote:
>> > Michael Niedermayer <michaelni at gmx.at> writes:
>> > 
>> > > Hi
>> > >
>> > > The patch below fixes issue434, i will commit it in 24h unless
>> > > mans objects
>> > >
>> > > Index: libavformat/mpeg.c
>> > > ===================================================================
>> > > --- libavformat/mpeg.c	(revision 12867)
>> > > +++ libavformat/mpeg.c	(working copy)
>> > > @@ -341,12 +341,13 @@
>> > >          if (flags & 0x01) { /* PES extension */
>> > >              pes_ext = get_byte(s->pb);
>> > >              header_len--;
>> > > -            if (pes_ext & 0x40) { /* pack header - should be zero in PS */
>> > > -                goto error_redo;
>> > > -            }
>> > >              /* Skip PES private data, program packet sequence counter and P-STD buffer */
>> > >              skip = (pes_ext >> 4) & 0xb;
>> > >              skip += skip & 0x9;
>> > > +            if (pes_ext & 0x40 || skip > header_len){
>> > > +                av_log(s, AV_LOG_WARNING, "pes_ext %X is invalid\n", pes_ext);
>> > > +                pes_ext=skip=0;
>> > > +            }
>> > >              url_fskip(s->pb, skip);
>> > >              header_len -= skip;
>> > 
>> > Workarounds for out-of-spec streams are only acceptable if explicitly
>> > requested.  Does lavf have something equivalent to lavc's
>> > AVCodecContext.strict_std_compliance?
>> 
>> No, also strict_std_compliance is a pure encoder side thing. Used for
>> preventing users to shoot themselfs in their foot (encoding expermental
>> codecs like snow, storing non jpeg YUV in jpeg, ...).
>> 
>> The variable with a matching purpose is error_resilience (bad name, yes ...)
>> It does select between how much spec non compliance should be ignored and
>> how much should be considered an error. Thus its a tradeoff between early
>> detection of errors in damaged streams vs. supporting streams which dont
>> conform to the specs.
>> 
>> We certainly could add such a variable to AVFormatContext as well.
>
> ping
> What shall be done about this issue?
> * This does not affect valid files
> * It does print a big and nasty "blah is invalid"
> * Its ffmpegs "policy" to support broken files by default
>
> I can add a error_resilience variable to AVFormatContext if you want, but
> its default would be what has the highest chance to result in correct
> playaback.

You may apply this if you give me the head of a vdr author on a silver
plate first.  There is no excuse for knowingly creating invalid files.

-- 
M?ns Rullg?rd
mans at mansr.com