[FFmpeg-devel] mpegtsenc: Improve PCR generation and output

[Baptiste Coudurier]

On 10/06/2010 03:48 PM, Michael Niedermayer wrote:
> On Wed, Oct 06, 2010 at 03:19:25PM -0700, Baptiste Coudurier wrote:
>> On 10/06/2010 04:31 AM, Michael Niedermayer wrote:
>>> On Wed, Oct 06, 2010 at 11:07:05AM +0200, Tomas H?rdin wrote:
>>>> On Mon, 2010-10-04 at 16:48 +0200, Tomas H?rdin wrote:
>>>>> On Thu, 2010-09-30 at 11:33 -0700, Baptiste Coudurier wrote:
>>>>>> Hi Tomas,
>>>>>>
>>>>>> On 9/30/10 7:07 AM, Tomas H?rdin wrote:
>>>>>>> Hi
>>>>>>>
>>>>>>> While working some more on remuxing dvbsub in mpegts I
>>>>>>> noticed that the longer the sample I used was, the higher
>>>>>>> the muxdelay I needed in order to avoid the "dts<    pcr,
>>>>>>> TS is invalid" warning.
>>>>>>>
>>>>>>> This is due to how the current muxer calculates PCR. It
>>>>>>> simply accumulates TS_PACKET_SIZE*8*90000LL/ts->mux_rate
>>>>>>> in cur_pcr. Unless your mux_rate evenly divides 135360000
>>>>>>> (TS_PACKET_SIZE*8*90000) this will cause rounding errors
>>>>>>> which quickly lead to unacceptable PCR drift.
>>>>>>>
>>>>>>> A second problem is that it only uses 90 kHz precision,
>>>>>>> when it should use 27 MHz. 90 kHz is insufficient to stay
>>>>>>> within the allowed +- 500 ns jitter.
>>>>>>>
>>>>>>> The attached patch calculates PCR based on max_delay and
>>>>>>> the current size of the output, in 27 MHz. This method
>>>>>>> should eliminate any PCR drift caused by the rounding
>>>>>>> errors in the previous solution. It also outputs the full
>>>>>>> PCR.
>>>>>>>
>>>>>>> The patch passes the regtests, but that seems to be
>>>>>>> because there are none for mpegts. Maybe some should be
>>>>>>> added?
>>>>>>
>>>>>> The test is present, by default the muxer uses VBR, you
>>>>>> activate CBR muxing by specifying a muxrate. Did you test
>>>>>> specifying a muxrate ? :)
>>>>>>
>>>>>> Patch looks good, thanks for the work, I'll test it against
>>>>>> some tools.
>>>>>
>>>>> Actually, I made a mistake. The six reserved bits are
>>>>> between program_clock_reference_base and
>>>>> program_clock_reference_extension, not after. So the patch
>>>>> writes the program_clock_reference_extension bits in the
>>>>> wrong place. See table 2-7 on page 44 in ISO/IEC 13818-1.
>>>>>
>>>>> Updated patch attached.
>>>>>
>>>>> As for testing, I failed to find a file in tests/ that
>>>>> contains "mpegts" or whose name includes "mpegts", so how
>>>>> would I go about testing this?
>>>>>
>>>>> /Tomas
>>>>
>>>> I discovered another bug yesterday. If dts ever becomes less
>>>> than pcr
>>>
>>> if dts become less than pcr then you should do av_log("internal
>>> error in mpeg ts muxer") av_abort() or return -1 at appropriate
>>> point to end muxing
>>>
>>> why?
>>>
>>> because you create a broken file that isnot going to playback on
>>> at least some players and doing this silently is VERY wrong.
>>
>> Well, many players just don't care about the PCR, namely FFmpeg,
>> mplayer, even Quicktime player.
>
> yes, still its better to generate compliant files that play on all
> players

Agree but sometimes I just don't care myself, I'm sure I'm not the only one.

>>
>>> Thats the least if you dont replace the buffering code by the
>>> working code from mpeg-ps
>>
>> Ok, now tell me how the ps muxer works with very high bitrates ?
>> 50mb/s and higher. It doesn't.
>
> elaborate please, i was not aware of this problem

It appears when stream copying

ffmpeg -i <file> -vcodec copy -f vob test.mpg

[vob @ 0x1e9d1c0]packet too large, ignoring buffer limits to mux it
[vob @ 0x1e9d1c0]buffer underflow i=0 bufi=235483 size=250000
[vob @ 0x1e9d1c0]buffer underflow i=0 bufi=237507 size=250000
[vob @ 0x1e9d1c0]packet too large, ignoring buffer limits to mux it
[vob @ 0x1e9d1c0]buffer underflow i=0 bufi=237507 size=250000
[vob @ 0x1e9d1c0]buffer underflow i=0 bufi=239531 size=250000
[vob @ 0x1e9d1c0]packet too large, ignoring buffer limits to mux it
[vob @ 0x1e9d1c0]buffer underflow i=0 bufi=239531 size=250000
[vob @ 0x1e9d1c0]buffer underflow i=0 bufi=241555 size=250000
[vob @ 0x1e9d1c0]packet too large, ignoring buffer limits to mux it
[vob @ 0x1e9d1c0]buffer underflow i=0 bufi=241555 size=250000
[vob @ 0x1e9d1c0]buffer underflow i=0 bufi=243579 size=250000
[vob @ 0x1e9d1c0]packet too large, ignoring buffer limits to mux it

>> Now PS and TS buffering model is _different_, and by different I
>> mean as much as having a different model in the specs (T-STD).
>
> could you summarize the differences? Having things desribed by 2
> totally different pieces of text and still be completely identical is
> not uncommon for ISO specs.

Part 1 of ISO/IEC 13818 employs a "System Target Decoder" (STD), one for
Transport Streams (refer to 2.4.2) referred to as "Transport System
Target Decoder" (T-STD) and one for Program Streams (refer to 2.5.2)
referred to as "Program System Target Decoder" (P-STD), to provide a
formalism for timing and buffering relationships. Because the STD is
parameterized in terms of ITU-T Rec. H.222.0 | ISO/IEC 13818-1 fields
(for example, buffer sizes) each elementary

"In Transport Streams the intended rate is determined by the values of 
the PCR fields and the number of Transport Stream bytes between them. In 
Program Streams the intended rate is explicitly specified as the 
Program_mux_rate, although as specified in this Recommendation |
International Standard the rate may drop to zero at SCR locations, i.e.,
if the SCR arrives before the time expected when the data is delivered 
at the indicated rate."

T-STD have more buffers (transport buffers and multiplex buffers), also
the mux rate is not written in the stream like in PS (program_mux_rate).

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