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

[Michael Niedermayer]

On Mon, May 24, 2010 at 03:46:08PM +0300, Martin Storsj? wrote:
> On Mon, 24 May 2010, Michael Niedermayer wrote:
> 
> > 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?
> 
> I don't, I just hope it is.
> 
> > > 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
> 
> That's why I always refilled the buffer in the previous suggestion.

its quite inefficient if you have a 4096 buffer and 40 byte packets
and as this is example code such hacks are obviously something we cant
do

you could allocate a 100k buffer and keep filling it so there is at least
4k there, and once you hit the end memmove the <=4k left to the begin
this still is a hack without us knowing for sure 4k is enough in the
first place but its better than what we have now
Ideally we might even write several examples (the above and the AVParser
variant)


> 
> > codecs have a maximum packet size in general
> 
> So, where can I get such a maximum packet size for MP2, in a generic way 
> suitable for api-example.c?

from the spec or source of the codec ;)
we could of course add a max_packet_size field to AVCodec but there probably
are codecs where its theoretiocally quite a bit larger than in reality.


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

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"
-------------- 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/121e0b6c/attachment.pgp>