[FFmpeg-devel] [bug/patch] MPEG-TS muxer: PCR not in sync with PTS/DTS
Niobos
niobos.be
Wed Jul 22 09:52:40 CEST 2009
On 22 Jul 2009, at 01:52, Baptiste Coudurier wrote:
>> @@ -429,9 +429,10 @@
>> /* PES header size */
>> if (st->codec->codec_type == CODEC_TYPE_VIDEO ||
>> st->codec->codec_type == CODEC_TYPE_SUBTITLE)
>> - total_bit_rate += 25 * 8 / av_q2d(st->codec->time_base);
>> + total_bit_rate += (19*8 + 92*8) / av_q2d(st->codec-
>> >time_base); /* 19B PES header every frame */
>> +
>> /* on
>> average 92B of padding at the end of every packet */
>> else
>> - total_bit_rate += total_bit_rate * 25 /
>> DEFAULT_PES_PAYLOAD_SIZE;
>> + total_bit_rate += (14*8 + 92*8) * 7; /* 14B PES header
>> 6.7 times a second */
>> }
>
> What's the rationale behind the avrage padding value ?
> Is it necessary if we increase the bitrate by 2% ?
The average padding is added to compensate for the last packet: Even
if only 1 byte is needed, a full 184+4 bytes will be written/sent.
Statistics tell us that, on average, the last packet will be half
filled, hence the 184/2 bytes of padding.
It's still needed when we add the 2%. Especially if the bitrate is
relatively low compared to the framerate, the 92B/frame might become
more than 2%. I'm asuming 1 frame = 1 PES. Example for 50fps at
100kbps (which is probably as low as anyone ever wants to go): 100kbps
= 12.5kB/s; 50fps => 4.6kB/s padding (on average). 4.6kB/s is more
than 2% of 12kB/s
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.
>>
>> /* if no video stream, use the first stream as PCR */
>> @@ -465,9 +466,9 @@
>>
>> total_bit_rate +=
>> total_bit_rate * 4 / TS_PACKET_SIZE + /* TS
>> header size */
>> - SDT_RETRANS_TIME * 8 * sdt_size / 1000 + /* SDT
>> size */
>> - PAT_RETRANS_TIME * 8 * pat_pmt_size / 1000 + /* PAT
>> +PMT size */
>> - PCR_RETRANS_TIME * 8 * 8 / 1000; /* PCR
>> size */
>> + 1000 * 8 * sdt_size / SDT_RETRANS_TIME + /* SDT
>> size */
>> + 1000 * 8 * pat_pmt_size / PAT_RETRANS_TIME + /* PAT
>> +PMT size */
>> + 1000 * 8 * 8 / PCR_RETRANS_TIME; /* PCR
>> size */
>>
>> av_log(s, AV_LOG_DEBUG, "muxrate %d freq sdt %d pat %d\n",
>> total_bit_rate, ts->sdt_packet_freq, ts->pat_packet_freq);
>
> This hunk LGTM.
Well, I needed to change the first line as well (TS header): This is
calculated in Bytes instead of bits. The new version is in the
attached patch.
>> +/* Insert NULL packets to keep PTS near PCR */
>
> You mean DTS.
>> +static void instert_null_packets(AVFormatContext *s, int64_t dts)
>
> typo.
Indeed. Corrected.
>> + MpegTSWrite *ts = s->priv_data;
>> + uint8_t buf[TS_PACKET_SIZE];
>> + uint8_t *q;
>> +
>> + while( dts != AV_NOPTS_VALUE && dts > ts->cur_pcr && dts - ts-
>> >cur_pcr > 135000LL ) {
>> + q = buf;
>> + *q++ = 0x47;
>> + *q++ = 0x00 | 0x1f;
>> + *q++ = 0xff;
>> + *q++ = 0x10;
>> + memset(q, 0x00, TS_PACKET_SIZE-4);
>> + put_buffer(s->pb, buf, TS_PACKET_SIZE);
>> + ts->cur_pcr += TS_PACKET_SIZE*8*90000LL/ts->mux_rate;
>> + }
>
> I think the constraint delta PCR <= 0.1s might not be respected here.
> Btw coding style is:
> - no space after ( and before )
> - 4 spaces indentation
Code style adopted.
About the PCR interval < 100ms: True, I fixed this in the updated patch.
I also added a "cur_pcr += written_length" to the retransmit_si_info,
since this function writes bits without counting them.
What about the hard-coded 1.5seconds (135000LL)?
>> +
>> static void write_pts(uint8_t *q, int fourbits, int64_t pts)
>> {
>> int val;
>> @@ -542,6 +563,7 @@
>> is_start = 1;
>> while (payload_size > 0) {
>> retransmit_si_info(s);
>> + instert_null_packets(s, dts);
>>
>> write_pcr = 0;
>> if (ts_st->pid == ts_st->service->pcr_pid) {
>
> The other solution is to only bump PCR and not
> writing these padding packets. However I remember reading,
> about some specs requiring these packets to be present.
Quote from the specs (ISO 13818-1, section 2.4.2.2):
> Data from the Transport Stream enters the T-STD at a piecewise
> constant rate
So just leaving out the packets violates this. We could dynamically
step the muxrate to a lower value, but should ensure that the new
muxrate is held "long enough" for the decoder to synchronize it's PLL
(although software decoders synchronize almost instantly, hardware
might need more time).
But as Alexandre Ferrieux pointed out:
> I confirm that the solution works in practice for several set-top-
> boxes. Even disgusting hacks like setting PCR to (DTS-offset) give a
> perfect playback, provided the timing of sending of the data (over
> UDP) doesn't drift from this synthetic PCR (I wrote an external
> spacer for that).
I'm ok to make the insert_null_packets() an option which defaults to off
Since you are considering all of my patches, I combined mpegts-
overhead-calc.diff and mpegts-overhead-null-packets.diff into one
patch. I also included a graph of {PCR,DTS,PTS} vs packet number for
the original and patched version, which illustrates the problem quite
well.
Niobos
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mpegts-overhead.diff
Type: application/octet-stream
Size: 4530 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090722/4d4bdc1b/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: original.png
Type: image/png
Size: 4643 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090722/4d4bdc1b/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: original.zoom.png
Type: image/png
Size: 5832 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090722/4d4bdc1b/attachment-0001.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patched.png
Type: image/png
Size: 4524 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090722/4d4bdc1b/attachment-0002.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patched.zoom.png
Type: image/png
Size: 5465 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090722/4d4bdc1b/attachment-0003.png>
More information about the ffmpeg-devel
mailing list