Mans Rullgard <[EMAIL PROTECTED]> added the comment:
Michael Niedermayer <[EMAIL PROTECTED]> writes:
> Michael Niedermayer <[EMAIL PROTECTED]> added the comment:
>
> On Sun, Mar 02, 2008 at 11:31:59PM -0000, Mans Rullgard wrote:
>>
>> Mans Rullgard <[EMAIL PROTECTED]> added the comment:
>>
>> Michael Niedermayer <[EMAIL PROTECTED]> writes:
>>
>> > Michael Niedermayer <[EMAIL PROTECTED]> added the comment:
>> >
>> > Suggested patch attached.
>> > I will commit in 48h if no objections.
>> >
>> > Index: mpeg.c
>> > ===================================================================
>> > --- mpeg.c (revision 12265)
>> > +++ mpeg.c (working copy)
>> > @@ -334,6 +334,10 @@
>> > header_len -= 5;
>> > }
>> > }
>> > + if (flags & 0x01 && header_len == 0){
>> > + flags &= ~0x01;
>> > + av_log(s, AV_LOG_WARNING, "PES Extension flag set but no
>> > bytes left\n");
>> > + }
>>
>> This could just as well cover all PES header parts signalled by flags,
>> even though we don't care about them.
>
> We already check that there was enough space available, and droping the
> offending flag like here isnt possible, as "too little space"
> doesnt say which flag, if any is wrong.
If the remaining length is zero, any set flag is wrong.
>>
>> > if (flags & 0x01) { /* PES extension */
>> > pes_ext = get_byte(s->pb);
>> > header_len--;
>>
>> This bit of code could do with a little more error checking...
>
> Such error checking would be redundant, the code below already catches
> all cases of too little space.
> if(header_len < 0)
> goto error_redo;
I am aware of this, but the suggested patch is evidence that it's not
good enough.
> If you want i could add a av_log() to annoy the user but besides this it
> seems the code is fine?
Users should be informed about broken files. Alternatively, users
with broken files deserved to be annoyed. Either way, printing a
message would be useful in diagnosing future problems of similar
nature.
______________________________________________________
FFmpeg issue tracker <[EMAIL PROTECTED]>
<https://roundup.mplayerhq.hu/roundup/ffmpeg/issue198>
______________________________________________________
