[FFmpeg-devel] [PATCH] au: add duration info

[Stefano Sabatini]

On date Saturday 2011-07-16 17:36:55 +0200, Stefano Sabatini encoded:
> On date Thursday 2011-07-14 12:06:17 +0200, ami_stuff encoded:
> > Modified.
> > 
> > Regards
> 
> > diff --git a/libavformat/au.c b/libavformat/au.c
> > index 23365c4..f361a71 100644
> > --- a/libavformat/au.c
> > +++ b/libavformat/au.c
> > @@ -120,7 +120,7 @@ static int au_probe(AVProbeData *p)
> >  static int au_read_header(AVFormatContext *s,
> >                            AVFormatParameters *ap)
> >  {
> > -    int size;
> > +    int size, data_size = 0;
> >      unsigned int tag;
> >      AVIOContext *pb = s->pb;
> >      unsigned int id, channels, rate;
> > @@ -132,7 +132,12 @@ static int au_read_header(AVFormatContext *s,
> >      if (tag != MKTAG('.', 's', 'n', 'd'))
> >          return -1;
> >      size = avio_rb32(pb); /* header size */
> > -    avio_rb32(pb); /* data size */
> > +    data_size = avio_rb32(pb); /* size in bytes */
> > +
> > +    if (data_size < 0) {
> > +        av_log(s, AV_LOG_ERROR, "negative data_size\n");
> > +        return AVERROR_INVALIDDATA;
> > +    }
> >  
> >      id = avio_rb32(pb);
> >      rate = avio_rb32(pb);
> > @@ -159,6 +164,10 @@ static int au_read_header(AVFormatContext *s,
> >      st->codec->codec_id = codec;
> >      st->codec->channels = channels;
> >      st->codec->sample_rate = rate;
> > +
> > +    if (data_size)
> > +        st->duration = (int64_t)(data_size) / (st->codec->channels * (uint64_t)(av_get_bits_per_sample(st->codec->codec_id)>>3));

Re-reading the comment from Michael, I realived this was not what he
meant.

In case of coded samples, av_get_bits_per_sample(st->codec->codec_id)
can be < 8 so if you shift by >>3 you get a 0 in the denominator.

Also data_size << 3 may overflow, but if you cast the int to a
variable with a greater number of bits *before* to shift, then you're
sure it won't.

So you can write it as:

if (data_size) // data size expressed in bits
    st->duration = (((int64_t)data_size) << 3) / (st->codec->channels * (uint64_t)(av_get_bits_per_sample(st->codec->codec_id));

Also the "if (data_size)" check can be avoided since it will return
the same value of 0 if the expression is evaluated.

BTW I'm not totally sure that the int -> int64_t cast is to be
considered correct, e.g. it may happen that:
sizeof(int64_t) == sizeof(int)
in this case the two types have the same "numerability", (and in
general it could be even happen that sizeof(int64_t) < sizeof(int),
right?).

So a possibly safer alternative would be to use floating point
arithmetic:

bpp = av_get_bits_per_sample(st->codec->codec_id);
if (bpp) {
    double duration = (double)(data_size*8) / (st->codec->channels * bpp);
    st->duration = FFMAX((((int64_t)1)<<63)-1, duration); // trunc duration to its maximum admitted value
}

-- 
FFmpeg = Frightening and Fierce Multimedia Portentous Extensive Geisha