[FFmpeg-devel] [PATCH]: Modify decode_frame() in mpegaudiodec.c to only consume s->frame_size bytes max

[Michael Niedermayer]

Hi

On Sat, May 26, 2007 at 11:52:06PM -0400, Chris Pinkham wrote:
> Hi,
> 
> I'm one of the MythTV developers and recently ran into an issue while
> working on a patch.  Currently we use lame directly in our
> NuppelVideoRecorder and NuppelDecoder classes to encode and decode
> MP3 audio.  As part of my current mods, I am converting over to using
> libavcodec for this so that we can support things like AC3 inside
> our .nuv files.
> 
> When I converted NuppelDecoder over from using lame_decode() to
> use avcodec_decode_audio(), I started getting lots of "incorrect frame
> size" warnings from mpegaudiodec.c.  I tracked this down and found
> out that it is because for some reason, sometimes we have 2 frames
> in one buffer.  mpegaudiodec.c's decode_frame() would kick out a
> "incorrect frame size" warning about this and then return the
> total size of the passed-in buffer as if it had consumed all the data
> when in fact it hadn't, it had only consumed the first frame
> (s->frame_size bytes).
> 
> I know that there is probably some other issue as to why we get these
> buffers with two frames in them in our nuv files, but it didn't make
> sense to me that decode_frame() would just ignore the extra data
> in the buffer when it was another frame, so I modified decode_frame()
> to only consume s->frame-size bytes and to squelch the warning
> about incorrect frame size if the buf_size passed in is a multiple
> of s->frame_size (and hence probably a buffer containing multiple
> frames).

i agree that s->frame_size should be returned instead of buf_size
besides that your solution isnt correct (in the sense that it hides
a bug rather then fixing it, not that it would neccessarily not work)

the issue is that libav* expects a single mp3 frame in a AVPacket
and not multiple ones, later would break stream copy into other
container formats which per specification need 1 mp3 frame within
their packets. i dunno if .nuv really was supposed to contain multiple
mp3 frames per packet or if that was rather some encoder bug ...
also mp3 frames can differ in their size (even for CBR mp3 when the
ideal frame size is not an exact integer)

ive fixed the multiple mp3 frames in one AVPacket bug, a patch which makes
decode_frame() return s->frame_size and properly decodes the first frame
is welcome though (but the warning should be left there ...)

[...]

>          av_log(avctx, AV_LOG_ERROR, "incorrect frame size\n");
>      }
>  
> -    out_size = mp_decode_frame(s, out_samples, buf, buf_size);
> +    out_size = mp_decode_frame(s, out_samples, buf, s->frame_size);

why?


[...]
> +
> +    int ret = s->frame_size;
>      s->frame_size = 0;
> -    return buf_size;
> +    return ret;

mixing declarations and statements will break gcc 2.95


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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070527/0d4bda13/attachment.pgp>