[FFmpeg-devel] [PATCH 2/2] mxfdec: set audio packets pts

[Michael Niedermayer]

On Fri, Nov 16, 2012 at 09:47:33AM +0100, Tomas Härdin wrote:
> On Mon, 2012-11-12 at 19:58 +0100, Matthieu Bouron wrote:
> > On Fri, Nov 09, 2012 at 01:36:39PM +0100, Tomas Härdin wrote:
> > > > Note: audio ptses are broken in case of seeking if the mxf has no
> > > > index
> > > > table; the current_edit_unit is not computed in that case in the
> > > > mxf_read_seek function.
> > > 
> > > Hm, fair enough. Broken files are less of a priority.
> > > 
> > > > +static int mxf_compute_sample_count(MXFContext *mxf, int stream_index, uint64_t *sample_count)
> > > > +{
> > > > +    int i, total = 0, size = 0;
> > > > +    AVStream *st = mxf->fc->streams[stream_index];
> > > > +    MXFTrack *track = st->priv_data;
> > > > +    AVRational time_base = av_inv_q(track->edit_rate);
> > > > +    AVRational sample_rate = av_inv_q(st->time_base);
> > > > +    const MXFSamplesPerFrame *spf = NULL;
> > > > +
> > > > +    if (av_q2d(sample_rate) == 48000)
> > > 
> > > I don't like checking for equality between doubles.. Consider casting to
> > > int (or dividing num/den manually so you get an int).
> > > 
> > > > +        spf = ff_mxf_get_samples_per_frame(mxf->fc, time_base);
> > > > +    if (!spf) {
> > > > +        int remainder = (sample_rate.num * time_base.num) % (time_base.den * sample_rate.den);
> > > 
> > > edit_rate should be tested somewhere before being used here. A malicious
> > > file could have a 0/0 EditRate.
> > > 
> > > > +        *sample_count = av_q2d(av_mul_q((AVRational){mxf->current_edit_unit, 1},
> > > > +                                        av_mul_q(sample_rate, time_base)));
> > > 
> > > OK.
> > > 
> > > > diff --git a/tests/ref/seek/lavf_mxf b/tests/ref/seek/lavf_mxf
> > > > index f7d1f67..34dddc3 100644
> > > > --- a/tests/ref/seek/lavf_mxf
> > > > +++ b/tests/ref/seek/lavf_mxf
> > > > @@ -7,8 +7,8 @@ ret: 0         st: 0 flags:0  ts: 0.800000
> > > >  ret: 0         st: 0 flags:1 dts: 0.840000 pts: 0.960000 pos: 460288 size: 24711
> > > >  ret: 0         st: 0 flags:1  ts:-0.320000
> > > >  ret: 0         st: 0 flags:1 dts:-0.040000 pts: 0.000000 pos:   6144 size: 24801
> > > > -ret:-1         st: 1 flags:0  ts: 2.560000
> > > > -ret: 0         st: 1 flags:1  ts: 1.480000
> > > > +ret:-1         st: 1 flags:0  ts: 2.576667
> > > > +ret: 0         st: 1 flags:1  ts: 1.470833
> > > 
> > > Are these timestamp changes expected? The difference is around 800
> > > samples, which feels like a relevant number. NTSC?
> > > 
> > > As long as no PAL samples changed behavior then I'm OK with the FATE
> > > changes. It's hard to tell.
> > > 
> > 
> > New patch attached.
> 
> Looks OK. Sorry for the delay.

applied, thanks

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

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121116/5dbb3258/attachment.asc>