[FFmpeg-devel] [PATCH] Fix empty G-VOP header decoding in MPEG-4

[Anatoly Nenashev]

On 10.02.2011 16:55, M?ns Rullg?rd wrote:
> Anatoly Nenashev<anatoly.nenashev at ovsoft.ru>  writes:
>
>    
>> On 10.02.2011 01:32, M?ns Rullg?rd wrote:
>>      
>>> Anatoly Nenashev<anatoly.nenashev at ovsoft.ru>   writes:
>>>
>>>
>>>        
>>>> On 08.02.2011 22:00, M?ns Rullg?rd wrote:
>>>>
>>>>          
>>>>> Anatoly Nenashev<anatoly.nenashev at ovsoft.ru>    writes:
>>>>>
>>>>>
>>>>>
>>>>>            
>>>>>> Hi all!
>>>>>> There are some cameras which send mpeg-4 streams with empty G-VOP header.
>>>>>> This part of stream looks like this:
>>>>>> ... 00 00 01 B3 00 00 00 01 B6 ...
>>>>>> Sample file uploaded in issue 2592.
>>>>>> FFmpeg reports "header damaged" and ignores first I-frame in G-VOP.
>>>>>> Attached patch fix this problem.
>>>>>>
>>>>>> Anatoly.
>>>>>>
>>>>>>
>>>>>>              
>>>>    [...]
>>>>
>>>>          
>>>>> Looking at this, I wonder why this is there at all.  The s->time_base
>>>>> field is only used to incorrectly set the pts field of the decoded
>>>>> frame (now I understand why those values always are wrong).  The
>>>>> actual PTS is passed from the input packet to the pkt_pts field of the
>>>>> output frame.
>>>>>
>>>>> IMO this nonsense should be removed.  The time_code field in the
>>>>> bitstream has nothing to do with PTS.
>>>>>
>>>>>
>>>>>
>>>>>            
>>>> Hmm... So, if I understand you clearly, we just can ignore GOP header
>>>> as done in attached patch.
>>>>
>>>>          
>>> The value seems to also be used in a convoluted calculation of direct
>>> mode MVs.  This calculation doesn't actually need the time_code value,
>>> only the distance (in frames) between reference frames and the predicted
>>> frame.  I'd be careful messing with it.
>>>
>>> The part setting the PTS of the output frame should of course be
>>> removed.  It is clearly wrong.
>>>
>>>
>>>        
>> I've changed  original patch to carefully check that time code
>> obtained from GOP header is monotone.
>> Also PTS setting for output frame is removed. See attachment.
>>
>> diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
>> index 5303da3..02e7853 100644
>> --- a/libavcodec/mpeg4videodec.c
>> +++ b/libavcodec/mpeg4videodec.c
>> @@ -1494,16 +1494,19 @@ end:
>>
>>   static int mpeg4_decode_gop_header(MpegEncContext * s, GetBitContext *gb){
>>       int hours, minutes, seconds;
>> +    int val, time_base;
>>
>> -    hours= get_bits(gb, 5);
>> -    minutes= get_bits(gb, 6);
>> -    skip_bits1(gb);
>> -    seconds= get_bits(gb, 6);
>> +    val= show_bits(gb, 18);
>> +    hours= (val>>  13)&  0x1F;
>> +    minutes= (val>>  7)&  0x3F;
>> +    seconds= val&  0x3F;
>>
>> -    s->time_base= seconds + 60*(minutes + 60*hours);
>> -
>> -    skip_bits1(gb);
>> -    skip_bits1(gb);
>> +    time_base= seconds + 60*(minutes + 60*hours);
>> +    if (time_base<  s->time_base) {
>> +        av_log(s->avctx, AV_LOG_WARNING, "time_code in GOP header is non monotone, skipping\n");
>> +    } else {
>> +        s->time_base= time_base;
>> +    }
>>
>>       return 0;
>>   }
>>      
> The time_code doesn't have to be monotonically increasing IIUC.  Better
> to just check the marker bit and do nothing if it's not there.  It won't
> be there in your broken streams.
>
>    

Ok. Fixed.



-------------- next part --------------
A non-text attachment was scrubbed...
Name: mpeg4_v5.patch
Type: text/x-patch
Size: 937 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110210/a0a83f9a/attachment.bin>