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

[Tomas Härdin]

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.

/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121116/82b50fd7/attachment.asc>