[FFmpeg-devel] [bug/patch] MPEG-TS muxer: PCR not in sync with PTS/DTS

Niobos niobos.be
Mon Jul 27 13:37:53 CEST 2009


On 26 Jul 2009, at 08:37, Baptiste Coudurier wrote:
> On Wed, Jul 22, 2009 at 09:52:40AM +0200, Niobos wrote:
>>
>> I agree that these calculations are not as clean as they should be.
>> Another thing to consider is using -maxrate instead of -b to do
>> these calculations.
>
> Well, in CBR, maxrate == bitrate so it wouldn't change much.
> We indeed to increase total bitrate reasonably to compensate VBR.

I'm not sure on this, but I think in VBR-mode it would be best to  
generate a VBR transport stream: set the muxrate "high enough" and  
step the PCR if needed; only inserting NULL-packets if the muxrate is  
specified to be constant on the command line. What do you think of  
this approach?

>> What about the hard-coded 1.5seconds (135000LL)?
>
> You can use AVFormatContext->max_delay I think, so user can configure
> it.

Done

> Please keep patches splited, because patches will be applied in
> separate commits.

Hmm, ok...

>> +    uint64_t sdt_packet_size;
>
> unsigned is sufficient.
>
> But I don't think you need these fields.

Actually I just copied uint64_t from one function into the structure,  
so...
But indeed, I don't need them anymore because:

>>    if (++ts->sdt_packet_count == ts->sdt_packet_freq) {
>>        ts->sdt_packet_count = 0;
>>        mpegts_write_sdt(s);
>> +        ts->cur_pcr += ts->sdt_packet_size *8*90000LL/ts->mux_rate;
>>    }
>
> This is unneeded, mpegts_write_sdt will update pcr accordingly through
> mpegts_write_section.

My bad, removed these lines again.

>> static void write_pts(uint8_t *q, int fourbits, int64_t pts)
>> {
>>    int val;
>> @@ -552,6 +580,10 @@
>>            }
>>        }
>>
>> +	/* Allow 1 payload packet if we need to send the PCR */
>> +        if( ! write_pcr && insert_null_packets(s, dts) )
>> +            	continue;
>
> Spaces before and after ( )

I'll never learn...

Attached the re-split and edited patches for the overhead and the  
(untouched) varname-change.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mpegts-overhead-calc-v2.diff
Type: application/octet-stream
Size: 2152 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090727/edc50cf6/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mpegts-overhead-null-packets-v2.diff
Type: application/octet-stream
Size: 1200 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090727/edc50cf6/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mpegts-varname-freq-period.diff
Type: application/octet-stream
Size: 2601 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090727/edc50cf6/attachment-0002.obj>
-------------- next part --------------


I tested the patched version with:
ffmpeg -i Source.avi -t 100 -acodec libfaac -ab 32k -ar 22050 -ac 1 -s  
160x88 -vcodec libx264 -vpre hq -bf 2 -vpre ipod320 -g 50 -b 400k - 
maxrate 400k -bt 400k -bufsize 800k -muxdelay 2 -muxpreload 2 -y  
output.ts

The output is created without dts < pcr warning.

Niobos





More information about the ffmpeg-devel mailing list