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

Michael Niedermayer michaelni
Thu Oct 7 03:04:14 CEST 2010


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

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.

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

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- 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/d921ce6a/attachment.pgp>



More information about the ffmpeg-devel mailing list