----- Forwarded message from Måns Rullgård <mans.mansr.com> -----

Date: Tue, 29 Apr 2008 00:57:56 +0100
From: Måns Rullgård <mans.mansr.com>
To: FFmpeg development discussions and patches <ffmpeg-devel.mplayerhq.hu>
Subject: Re: [FFmpeg-devel] [PATCH] fix issue434
User-Agent: Gnus/5.1008 (Gnus v5.10.8) XEmacs/21.4.21 (Educational Television,
	linux)

Michael Niedermayer <michaelni.gmx.at> writes:

> On Mon, Apr 28, 2008 at 11:28:23PM +0100, Måns Rullgård wrote:
>> Michael Niedermayer <michaelni.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.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.
>
> No problem
>
>              _||||||_
>             / VDR DEV\
>            |  X    X  |
>           O|    |     |O
>            |   /_     |
>          ---\ -===-  /----
>      ----    |   U  |      ----
>  -==         |______|          ==-
>      ----   SILVER PLATE   ----
>          ------------------

What can I say?  Go ahead and commit.

-- 
Måns Rullgård
mans.mansr.com
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@mplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel

----- End forwarded message -----

-- 
Do your part to help preserve life on Earth -- by trying to preserve your own.