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

[Michael Niedermayer]

On Wed, Oct 06, 2010 at 06:09:19PM -0700, Baptiste Coudurier wrote:
> On 10/06/2010 06:04 PM, Michael Niedermayer wrote:
>> On Thu, Oct 07, 2010 at 02:33:27AM +0200, Michael Niedermayer wrote:
>>> On Wed, Oct 06, 2010 at 04:09:30PM -0700, Baptiste Coudurier wrote:
>>>> 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
>>>
>>> i found and fixed one issue, there possibly are more.
>>> especially the VBV buffer size used could plain be too small
>>
>> and interrestingly the mpeg ts muxer doesnt even read the vbv buf size
>
> Of course it should, but the TS muxer in CBR mode is only activated by  
> the user specifying a muxrate, so it's up to the user to specify a  
> correct one :)
>
>> either way, if you still have a failing file (with mpegps), i need a sample
>> as my 2 that i tried work but they are lower bitrate.
>
> You can create a file that fails using -vb 50Mb -minrate 50Mb -maxrate  
> 50Mb -bufsize <pick optimal value here, btw ffmpeg could do that for you> 
> :)

done and fixed, ffmpeg wasnt passing the vbv buffer size

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101007/6dc3c8fe/attachment.pgp>