FS#5062 - libmad does not decode the last frame

Attached to Project: Rockbox
Opened by Mark Arigo (lowlight) - Friday, 07 April 2006, 19:53 GMT
Last edited by Dave Chapman (linuxstb) - Saturday, 08 April 2006, 08:56 GMT
Task Type Bugs
Category Codecs
Status Closed
Assigned To No-one
Operating System All players
Severity Medium
Priority Normal
Reported Version
Due in Version Version 3.0
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


See here:
"...MAD will not decode a frame unless it is followed by at least MAD_BUFFER_GUARD (8) additional bytes in the stream."

This means the last frame will not decode because the final call to "request_buffer" (in mpa.c) for a track will return the frame size of the final frame with no additional bytes.

I reported this sometime ago with one possible solution (i.e. padding an extra 8 bytes to mp3s when buffering):
Maybe one of the devs has a better solution.

Probably no one notices that the last frame is missing since it is generally at least partial silence, although I'm suprised gapless play isn't effected by this. Regardless, it would be preferrable if Rockbox does the right thing and decodes the whole file. With the 3.0 feature freeze, I hope that someone will look into this.
This task depends upon

Closed by  Linus Nielsen Feltzing (linusnielsen)
Wednesday, 26 April 2006, 14:01 GMT
Reason for closing:  Fixed
Comment by Dave Chapman (linuxstb) - Saturday, 08 April 2006, 08:56 GMT
I have a couple of questions:

1) Does this only apply to layer 3 decoding? (i.e. do MP1/MP2 also need the guard bytes?)

2) When playing an album which has been encoded as a single MP3 file which has then been split on frame boundaries, and maybe (I'm not sure) albums encoded with the lame --no-gap option, it is desirable to feed libmad with a continuous stream of data and not reset the decoder between tracks. Do you know if adding the 8 guard bytes to the final frame will break this behaviour?

I agree we should try and fix this for 3.0
Comment by Mark Arigo (lowlight) - Monday, 10 April 2006, 17:36 GMT
1) It appears to effect all layers...see libmad/frame.c lines 389-406.

2) I'm not sure what you mean by "reset the decoder". It appears that each new track "re-initializes" the decoder (init_mad in line 126) regardless of whether the last track ends with an error or not. However, you could just change line 182 of mpa.c to break when size <= MAD_BUFFER_GUARD and it will request the next track.
Comment by Mark Arigo (lowlight) - Friday, 21 April 2006, 15:47 GMT
I've made a simple mp3 decoder using libmad to test if the padding has to be zero or not. You need to compile the source files in the attached zip file and link to libmad. Usage is maddec [mp3file] [outfile]. On line 113 of mad_decode.c I memset the padding. I've randomly tried some different values and the md5sum of the decoded file did not change.
Comment by Mark Arigo (lowlight) - Monday, 24 April 2006, 13:23 GMT Comment by Thom Johansen (preglow) - Monday, 24 April 2006, 13:47 GMT
Have you done some tests to verify that it is indeed fixed? If so, this bug should be closed.
Comment by Mark Arigo (lowlight) - Wednesday, 26 April 2006, 13:55 GMT
I've verified that all the frames are decoded now.
You may close this bug.