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

[Michael Niedermayer]

On Thu, Oct 07, 2010 at 12:48:10AM +0200, 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
> 
> 
> >
> >> 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
> 
> 
> > 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.

It appears there are differences i was unaware of from a quick look at the spec

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

It is not what we do, but why we do it that matters.
-------------- 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/5876ea3e/attachment.pgp>