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

[Michael Niedermayer]

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. Thats the least if you dont
replace the buffering code by the working code from mpeg-ps

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

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- 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/20101006/d5cb4647/attachment.pgp>