[FFmpeg-devel] [PATCH] mpeg4video: ignore broken GOP headers

Michael Niedermayer michaelni
Mon Feb 14 20:41:38 CET 2011


On Mon, Feb 14, 2011 at 06:40:43PM +0300, Anatoly Nenashev wrote:
> On 14.02.2011 15:18, Michael Niedermayer wrote:
>> On Sun, Feb 13, 2011 at 07:22:19PM +0100, Janne Grunau wrote:
>>    
>>> On Sun, Feb 13, 2011 at 03:57:44PM +0000, Mans Rullgard wrote:
>>>      
>>>> From: Anatoly Nenashev<anatoly.nenashev at ovsoft.ru>
>>>>
>>>> Some MPEG4 cameras produce files with empty GOP headers.
>>>> This patch makes the decoder ignore such broken headers and proceed
>>>> with the following I-frame.  Without this change, the following
>>>> start code is missed resulting in the entire I-frame being skipped.
>>>>
>>>> Signed-off-by: Mans Rullgard<mans at mansr.com>
>>>> ---
>>>>   libavcodec/mpeg4videodec.c |   21 +++++++++++----------
>>>>   1 files changed, 11 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
>>>> index 673c4e8..617dcb9 100644
>>>> --- a/libavcodec/mpeg4videodec.c
>>>> +++ b/libavcodec/mpeg4videodec.c
>>>> @@ -1494,16 +1494,17 @@ end:
>>>>
>>>>   static int mpeg4_decode_gop_header(MpegEncContext * s, GetBitContext *gb){
>>>>       int hours, minutes, seconds;
>>>> -
>>>> -    hours= get_bits(gb, 5);
>>>> -    minutes= get_bits(gb, 6);
>>>> -    skip_bits1(gb);
>>>> -    seconds= get_bits(gb, 6);
>>>> -
>>>> -    s->time_base= seconds + 60*(minutes + 60*hours);
>>>> -
>>>> -    skip_bits1(gb);
>>>> -    skip_bits1(gb);
>>>> +    unsigned time_code = show_bits(gb, 18);
>>>> +
>>>> +    if (time_code&  0x40) {     /* marker_bit */
>>>> +        hours   = time_code>>  13;
>>>> +        minutes = time_code>>   7&  0x3f;
>>>> +        seconds = time_code&  0x3f;
>>>> +        s->time_base = seconds + 60*(minutes + 60*hours);
>>>> +        skip_bits(gb, 20);      /* time_code, closed_gov, broken_link */
>>>> +    } else {
>>>> +        av_log(s->avctx, AV_LOG_WARNING, "GOP header missing marker_bit\n");
>>>> +    }
>>>>
>>>>       return 0;
>>>>   }
>>>>        
>>> ok
>>>      
>> rejected and implemented correctly in ffmpeg at videolan
>>
>>
>>    
>
> Why do you think that your version is better? You just check that  
> time_code bits is zero. Why do you think that this is more robust than  
> checking marker_bit?

the marker bit could be wrongly set to 0, we wouldnt want to discard data in
that case.
the bug that was described can only happen if an actual startcode that is
00 00 01 XX is there. So we should test for this ...


> And moreover why you've said nothing before? I don't follow the flame
> about current situation and leadership but I don't believe that you have  
> no rights for review.

right to review, ...
thats like right to do the work for free while others have the final say.
Thats ok if iam being payed but its not how free software works, its a mystery
to me why the new team doesnt get this.


> I very much appreciate your comments about my previous patches but now I
> can't understand what you are trying to do. It looks like you have your  
> own repository and push there your versions of other people's patches  
> without any type of discussion.

You should read the ffmpeg svn policy (that has actually been that way since
almost a decade).
Each part (like the mpeg4 decoder) has a maintainer and that guy (or girl) has
the final say of  what goes in and what does not. Thats exactly how
it was in ffmpeg and also how its in linux, just linux has many repos while
ffmpeg had 1.
Its up to the maintainer if he discusses his decission with others. Sometimes
this makes sense sometimes it just wastes time.



> It just increases a precipice between
> ffmpeg at ffmpeg and ffmpeg at videolan.

I dont see how. i maintain the mpeg4 decoder and anyone who thinks iam not doing
it good can try to do it better. Failing that its pull my changes or become
outdated. And if someone manages better iam happy to step back and pull from
them :)
its exactly like in linux.


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110214/54eebd31/attachment.pgp>



More information about the ffmpeg-devel mailing list