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

[Baptiste Coudurier]

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.

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