FS#10832 - Grok unstreamable AAC (mp4) files

Attached to Project: Rockbox
Opened by Juliusz Chroboczek (jch) - Saturday, 05 December 2009, 17:49 GMT
Last edited by MichaelGiacomelli (saratoga) - Saturday, 02 January 2010, 21:05 GMT
Task Type Patches
Category Codecs
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.4
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


Rockbox doesn't currently grok mp4 files in which the mdat chunk comes before the moov chunk. While I don't have the mp4 specs (please contact me if you can give me a copy), such files are properly played by all the Linux players I've tested, as well as Apple's OF.

In order to generate such a file, simply do "mplayer -dumpaudio" on an mp4 movie.

The attached patches add support for such files to rockbox. They work fine in the simulator, but unfortunately crash my iPod. Since I don't know how to debug code running on the iPod, I'll be grateful if anyone can help me working out what the problem is.

This task depends upon

Closed by  MichaelGiacomelli (saratoga)
Saturday, 02 January 2010, 21:05 GMT
Reason for closing:  Accepted
Additional comments about closing:  Accepted in r24147 and r24148.
Comment by Juliusz Chroboczek (jch) - Saturday, 05 December 2009, 18:53 GMT
The crash turned to be unrelated -- see  FS#10833 . With the latter patch applied, this code works fine on the iPod too.
Comment by Alex Bennee (ajb) - Saturday, 05 December 2009, 21:20 GMT
I shall test your patch out, this looks cleaner. I think the playback stuff has changed since I wrote #fs1060
Comment by Juliusz Chroboczek (jch) - Saturday, 05 December 2009, 21:54 GMT
Some AAC files appear to contain empty mdat chunks. The following patch ignores them.
Comment by Juliusz Chroboczek (jch) - Saturday, 05 December 2009, 21:55 GMT
Patch to ignore empty mdat chunks.
Comment by Juliusz Chroboczek (jch) - Saturday, 05 December 2009, 22:02 GMT

The main advantage of my patch over the one in  FS#10160  is that I only do the seeking forwards and backwards if we didn't find the moov chunk before the mdat chunk. in the normal case (moov before mdat), the behaviour is unchanged from the current one -- we stream the file from the buffer without any seeking.
Comment by Alex Bennee (ajb) - Sunday, 06 December 2009, 14:33 GMT
What baseline where these patches made against? I can't seem to apply them directly to the tree.
Comment by Alex Bennee (ajb) - Sunday, 06 December 2009, 14:36 GMT
Ignore last comment, finger trouble...
Comment by Alex Bennee (ajb) - Sunday, 06 December 2009, 15:23 GMT
OK I've tested with these patches (and the two from  FS#10833 ) and my "broken" m4a's all seem to be playing fine. From my point of view this patch seems good to go.

Your right the excessive seeking caused by FS #10160 was one of the reasons it never got merged.

If you want to get the following applied to the tree your best bet is to get onto IRC and see if anyone is willing to review and commit the changes to the source tree.
Comment by Juliusz Chroboczek (jch) - Sunday, 06 December 2009, 20:36 GMT
After discussion on IRC, a few clarifications:

1. These are perfectly correct mp4 files. The spec allows the metadata to precede the data (streamable files), or to follow it (unstreamable files, for one-pass writing). In other words, this attempts to fix an undocumented limitation in Rockbox, not to add a new feature. All players known to man (mplayer, vlc, iTunes, the iPod's OF) grok such files quite fine.

2. No performance hit is taken for streamable files (the only ones that we support right now). There is no regression.

3. For unstreamable files, we call bufadvance, which in turn calls bufseek. In the case where the whole track has been buffered (which should normally be the case), this does not result in actual disk I/O, just some pointer arithmetic inside the buffer. So unless you're really short on memory, or playing huge files, there will be no noticeable performance hit even for unstreamable files.

4. Yes, such files can be "fixed" by reordering the chunks. That is quite besides the point.
Comment by Juliusz Chroboczek (jch) - Monday, 28 December 2009, 16:17 GMT
After yet another round on IRC (sigh):

- the first two patches are for unstreamable files;
- the third patch adds support for skipping empty mdat chunks, which is a separate issue.
Comment by MichaelGiacomelli (saratoga) - Wednesday, 30 December 2009, 01:03 GMT
The third patch is simple and can go in as it (although I'm too lazy to commit it now since it doesn't apply cleanly without the first 2).

The first two look good to me, although I was hoping lear would comment since he actually knows something about mp4. I think they should go in regardless after the branch for 3.5. If no one beats me to it, I'll commit them all together after the branch.