[FFmpeg-devel] [PATCH 1/10] GXF Encoder Fixes and HD support (patch broken up)

[Baptiste Coudurier]

On 9/13/10 7:38 PM, Reuben Martin wrote:
> Yo, back on Monday, September 13, 2010 Baptiste Coudurier was all like:
>> Hi,
>>
>> On 09/13/2010 04:23 PM, Reuben Martin wrote:
>>> Yo, back on Thursday, September 09, 2010 Baptiste Coudurier was all like:
>>>> Hi,
>>>>
>>>> On 09/08/2010 06:34 PM, Reuben Martin wrote:
>>>>> Yo, back on Wednesday, September 08, 2010 Reuben Martin was all like:
>>>>>> Here's the breakdown:
>>>>>>
>>>>>> 01-gxf__flt_offset_error.patch
>>>>>>
>>>>>> BUG: The offset values for the FLT packets were getting attached to the packet preceding the packet they were supposed to be attached to. This was causing all kinds of playback and seeking issues on GrassValley's Turbo.
>>>>>>
>>>>>>
>>>>>>
>>>>>> 01-gxf__flt_offset_error.patch
>>>>>>
>>>>>>
>>>>>> --- ffmpeg-old/libavformat/gxfenc.c	2010-09-04 11:41:27.000000000 -0500
>>>>>> +++ ffmpeg-new/libavformat/gxfenc.c	2010-09-08 16:39:20.405000084 -0500
>>>>>> @@ -878,7 +878,11 @@
>>>>>>                     return -1;
>>>>>>                 }
>>>>>>             }
>>>>>> -        gxf->flt_entries[gxf->flt_entries_nb++] = url_ftell(pb) / 1024;
>>>>>> +
>>>>>> +        if (!(gxf->flt_entries_nb))
>>>>>> +            gxf->flt_entries[gxf->flt_entries_nb] = 4; /* First offset seems to be pretty close to this. May vary. */
>>>>>> +        gxf->flt_entries_nb++;
>>>>>> +        gxf->flt_entries[gxf->flt_entries_nb] = url_ftell(pb) / 1024;
>>>>>>             gxf->nb_fields += 2; // count fields
>>>>>>         }
>>>>>>
>>>>
>>>> Looks hackish to me.
>>>>
>>>>
>>>
>>> Changes made:
>>>
>>> Instead of checking the offset after writing the packet and using that values as the next FLT's offset value, it checks the offset before writing the packet and uses the value as the current FLT's offset.
>>>
>>> This fixes the bug as before, while also correctly calculating the proper offset for FLT[0]. The previous solution assumed 4k for FLT[0] which I found to be incorrect in cases where the first packet is not a video packet.
>>>
>>> Also less hackish. :)
>>>
>>> -Reuben
>>>
>>>
>>> 01-gxf__flt_offset_error.patch
>>>
>>>
>>> --- ffmpeg-old/libavformat/gxfenc.c	2010-09-04 11:41:27.000000000 -0500
>>> +++ ffmpeg-new/libavformat/gxfenc.c	2010-09-13 18:02:43.094000094 -0500
>>> @@ -859,7 +859,9 @@
>>>        AVStream *st = s->streams[pkt->stream_index];
>>>        int64_t pos = url_ftell(pb);
>>>        int padding = 0;
>>> +    int packet_start_offset = 0;
>>>
>>> +    packet_start_offset = url_ftell(pb) / 1024;
>>>        gxf_write_packet_header(pb, PKT_MEDIA);
>>>        if (st->codec->codec_id == CODEC_ID_MPEG2VIDEO&&   pkt->size % 4) /* MPEG-2 frames must be padded */
>>>            padding = 4 - pkt->size % 4;
>>> @@ -878,7 +880,8 @@
>>>                    return -1;
>>>                }
>>>            }
>>> -        gxf->flt_entries[gxf->flt_entries_nb++] = url_ftell(pb) / 1024;
>>> +        gxf->flt_entries[gxf->flt_entries_nb] = packet_start_offset;
>>> +        gxf->flt_entries_nb++;
>>
>> Looks good to me except the nb++ change which is not necessary.
>>
>>
>
> That was left over from my previous change. I've attatched updated version with the nb++ moved back the way it was before.
>
>
>
>
> 01-gxf__flt_offset_error.patch
>
>
> --- ffmpeg-old/libavformat/gxfenc.c	2010-09-04 11:41:27.000000000 -0500
> +++ ffmpeg-new/libavformat/gxfenc.c	2010-09-13 18:02:43.094000094 -0500
> @@ -859,7 +859,9 @@
>       AVStream *st = s->streams[pkt->stream_index];
>       int64_t pos = url_ftell(pb);
>       int padding = 0;
> +    int packet_start_offset = 0;
>
> +    packet_start_offset = url_ftell(pb) / 1024;
>       gxf_write_packet_header(pb, PKT_MEDIA);
>       if (st->codec->codec_id == CODEC_ID_MPEG2VIDEO&&  pkt->size % 4) /* MPEG-2 frames must be padded */
>           padding = 4 - pkt->size % 4;
> @@ -878,7 +880,7 @@
>                   return -1;
>               }
>           }
> -        gxf->flt_entries[gxf->flt_entries_nb++] = url_ftell(pb) / 1024;
> +        gxf->flt_entries[gxf->flt_entries_nb++] = packet_start_offset;
>           gxf->nb_fields += 2; // count fields
>       }
>

Patch ok, I forgot to tell you to run the regression tests, this change 
should make them differ, attach the diff so it's easier to apply.

Thanks for the work btw.

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org