Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Codecs
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.4
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by jch - 2009-12-05
Last edited by saratoga - 2010-01-02

FS#10832 - Grok unstreamable AAC (mp4) files

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.

–Juliusz

Closed by  saratoga
2010-01-02 21:05
Reason for closing:  Accepted
Additional comments about closing:  

Accepted in r24147 and r24148.

jch commented on 2009-12-05 18:53

The crash turned to be unrelated – see  FS#10833 . With the latter patch applied, this code works fine on the iPod too.

ajb commented on 2009-12-05 21:20

I shall test your patch out, this looks cleaner. I think the playback stuff has changed since I wrote #fs1060

jch commented on 2009-12-05 21:54

Some AAC files appear to contain empty mdat chunks. The following patch ignores them.

jch commented on 2009-12-05 21:55

Patch to ignore empty mdat chunks.

jch commented on 2009-12-05 22:02

Alex,

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.

ajb commented on 2009-12-06 14:33

What baseline where these patches made against? I can’t seem to apply them directly to the tree.

ajb commented on 2009-12-06 14:36

Ignore last comment, finger trouble…

ajb commented on 2009-12-06 15:23

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.

jch commented on 2009-12-06 20:36

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.

jch commented on 2009-12-28 16:17

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.

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.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing