[FFmpeg-devel] Audio decoding in api-example.c broken?

[Michael Niedermayer]

On Mon, May 24, 2010 at 02:27:42PM +0300, Martin Storsj? wrote:
> On Mon, 24 May 2010, Michael Niedermayer wrote:
> 
> > On Mon, May 24, 2010 at 10:42:41AM +0300, Martin Storsj? wrote:
> > > On Mon, 24 May 2010, Michael Niedermayer wrote:
> > > 
> > > > a AVParser could be used or you could implement some buffer that is refilled
> > > > after each frame is decoded
> > > 
> > > An attempt at solving it by refilling the buffer is attached.
> > > 
> > > // Martin
> > >  api-example.c |   12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > f62159abacf81c6c15a0a4f310284b9f6d2859e4  0001-Refill-the-input-audio-buffer-after-decoding-each-pa.patch
> > > From 25f9c3322ebe7381623ddb546d61c67540267835 Mon Sep 17 00:00:00 2001
> > > From: Martin Storsjo <martin at martin.st>
> > > Date: Mon, 24 May 2010 10:23:25 +0300
> > > Subject: [PATCH] Refill the input audio buffer after decoding each packet
> > > 
> > > This avoids trying to decode incomplete frames.
> > > ---
> > >  libavcodec/api-example.c |   12 ++++++------
> > >  1 files changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/libavcodec/api-example.c b/libavcodec/api-example.c
> > > index fb48b1f..576826d 100644
> > > --- a/libavcodec/api-example.c
> > > +++ b/libavcodec/api-example.c
> > > @@ -155,12 +155,8 @@ static void audio_decode_example(const char *outfilename, const char *filename)
> > >  
> > >      /* decode until eof */
> > >      avpkt.data = inbuf;
> > > -    for(;;) {
> > >          avpkt.size = fread(inbuf, 1, INBUF_SIZE, f);
> > > -        if (avpkt.size == 0)
> > > -            break;
> > >  
> > > -        avpkt.data = inbuf;
> > >          while (avpkt.size > 0) {
> > >              out_size = AVCODEC_MAX_AUDIO_FRAME_SIZE;
> > >              len = avcodec_decode_audio3(c, (short *)outbuf, &out_size, &avpkt);
> > > @@ -173,9 +169,13 @@ static void audio_decode_example(const char *outfilename, const char *filename)
> > >                  fwrite(outbuf, 1, out_size, outfile);
> > >              }
> > >              avpkt.size -= len;
> > > -            avpkt.data += len;
> > > +            memmove(avpkt.data, avpkt.data + len, avpkt.size);
> > > +            /* Refill the input buffer, to avoid trying to decode
> > > +             * incomplete frames. */
> > > +            len = fread(avpkt.data + avpkt.size, 1, INBUF_SIZE - avpkt.size, f);
> > > +            if (len > 0)
> > > +                avpkt.size += len;
> > 
> > as this is an example people will use for their applications
> > it should mention parsers and demuxers as alternative solutions
> 
> Ok, I could add comments mentioning that.
> 
> > and the refill must be implemented so as to minimize memmove/cpy
> > moving is only needed once we get close to the end of the buffer ...
> 
> How do we know that we're "close to the end of the buffer"? 

How do you know your buffer is large enough to begin with?


> Something like 
> if (avpkt.size < 2*len)?

if len was the last packets len then no this is not good
sizes can surely vary by more

codecs have a maximum packet size in general

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

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100524/f2f9a018/attachment.pgp>