[FFmpeg-devel] [PATCH] DVBSUBTILES, fixes to decoding/encoding and a new way of burning the subtitles onto the screen

JULIAN GARDNER joolzg at btinternet.com
Tue Sep 27 00:43:10 CEST 2011






>________________________________
>From: Michael Niedermayer <michaelni at gmx.at>
>To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
>Sent: Monday, 26 September 2011, 23:25
>Subject: Re: [FFmpeg-devel] [PATCH] DVBSUBTILES, fixes to decoding/encoding and a new way of burning the subtitles onto the screen
>
>On Sun, Sep 25, 2011 at 12:18:20AM +0200, Clément Bœsch wrote:
>> From: joolzg <joolzg at btinternet.com>
>> 
>> ---
>> Here is the second patch from Julian Gardner.
>> 
>> I did my best to remove the unrelated changes, fix the style and various other
>> things, but it's still far from perfect. Some real reviews are needed.
>
>Before i can review this, i first need an explanation of what these
>changes are supposed to do.
>like:
>The changes in file1 are needed to fix X implement Y and you can test
>this with file Z command line C
>... file2 ...
>...
>
>A properly split patchset would be better than such list of course
>
>
>[...]
>> @@ -162,6 +162,21 @@ static uint8_t *audio_out;
>>  static unsigned int allocated_audio_out_size, allocated_audio_buf_size;
>>  
>>  static short *samples;
>> +
>> +static AVBitStreamFilterContext *video_bitstream_filters;
>> +static AVBitStreamFilterContext *audio_bitstream_filters;
>> +static AVBitStreamFilterContext *subtitle_bitstream_filters;
>
>unused
>
>
>[...]
>> @@ -1024,17 +1069,22 @@ static void dvbsub_parse_region_segment(AVCodecContext *avctx,
>>          region = av_mallocz(sizeof(DVBSubRegion));
>>  
>>          region->id = region_id;
>> +        region->version = -1;
>>  
>>          region->next = ctx->region_list;
>>          ctx->region_list = region;
>>      }
>>  
>> +    version = ((*buf)>>4) & 15;
>>      fill = ((*buf++) >> 3) & 1;
>> +    buf_size--;
>>  
>>      region->width = AV_RB16(buf);
>>      buf += 2;
>> +    buf_size -= 2;
>>      region->height = AV_RB16(buf);
>>      buf += 2;
>> +    buf_size -= 2;
>
>using buf_end is simpler and more robust than updating buf_size
>synchronly with every buf update. buf_end is already there so this
>seems unneeded
>


Michael, i have never liked using the convention of using a buffer end as being the correct way to process a structure, I prefer to process the buffer as the spec says as keep the counters decrementing as per the spec, and i have done a lot of work in the DVB field, as in my last 10 years ive been writing mainly settop box firmware.


Also a good compiler will join all these subtractions into a single subtract.

Changes made to dvbsub*.c are to fix to the encoding and decoding, not real much to say but if you use the old source against the TS file i posted to the bug system months ago you will see that it did not work, it does now.

Changes to ffmpeg were made to add a fifo system to the subtitle decode so we can properly show the correct subtitle at the correct time as dvb subtitles can be sent early, and i have seen channels sending subtitles 5 seconds early, and to do the "hard subtitles"

The FIFO should be made to be dynamic as other subtitle systems, .srt, etc are processed once and this produces a flood of subtitle structures. Is there a FIFO system available in the utils system.

Hacks present are there to stop the production of a dvb subtitle stream at the same time as doing hard subs, i could not find a way of getting the output subtitle stream removed when choosing hard subtitles.

joolz
ps as i am still learning git it may take time to produce different patch files for each.


More information about the ffmpeg-devel mailing list