This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#10160 - Clean up MP4 metadata parsing to handle files with tags after 'mdat' atom
Attached to Project:
Rockbox
Opened by Alex Bennee (ajb) - Wednesday, 22 April 2009, 14:22 GMT+2
Last edited by Thomas Martitz (kugel.) - Sunday, 18 April 2010, 14:31 GMT+2
Opened by Alex Bennee (ajb) - Wednesday, 22 April 2009, 14:22 GMT+2
Last edited by Thomas Martitz (kugel.) - Sunday, 18 April 2010, 14:31 GMT+2
|
DetailsFrom 240b4eb19c0e167030bdbfe498f15596ca09316f Mon Sep 17 00:00:00 2001
From: Alex Bennee <alex@bennee.com> Date: Wed, 22 Apr 2009 13:17:00 +0100 Subject: [PATCH] Clean up the MP4 metadata parsing. The parser used to finish as soon as it saw an "mdat" atom and has set the filesize. There is nothing that says all you metadata will have been seen at this point. I've tweaked the parser to ensure it goes all the way through the file scanning for metadata. If tested this with an m4a of iTunes which mp4info parsed OK: 09:44 alex@danny/x86_64 [simdisk] >mp4info 03\ -\ This\ House\ Is\ Not\ for\ Sale.m4a mp4info version 1.5.0.1 03 - This House Is Not for Sale.m4a: Track Type Info 1 audio MPEG-4 AAC LC, 233.732 secs, 128 kbps, 44100 Hz Metadata Name: This House Is Not for Sale Metadata Artist: Ryan Adams Metadata Year: 2003 Metadata Album: Love Is Hell Part 1 Metadata track: 3 of 10 Metadata Disk: 1 of 1 Metadata Genre: Alt.Country Metadata Cover Art pieces: 1 However the file still won't play on Rockbox, I assume because the codec that does the actual playing cannot deal with the unusual structure of the m4a. Interestingly it plays the other songs of the album fine! --- apps/metadata/mp4.c | 116 +++++++++++++++++++++++++++++++++++---------------- 1 files changed, 80 insertions(+), 36 deletions(-) |
This task depends upon
Closed by Thomas Martitz (kugel.)
Sunday, 18 April 2010, 14:31 GMT+2
Reason for closing: Out of Date
Additional comments about closing: Closure requested by task creator. FS#10832 supersedes this.
Sunday, 18 April 2010, 14:31 GMT+2
Reason for closing: Out of Date
Additional comments about closing: Closure requested by task creator.
To really fix this, the metadata reader should first extract (and somehow buffer) the information needed, then let the playback code buffer the rest of the file.
How much information does the codec need from the metadata. I assume anything not already covered by mp3data (bitrate and length) can be covered by giving a list of mdat pointers and sizes describing the blocks of media data that make up the file?
What the codec needs is this:
1) The data to pass to NeAACDecInit2 (in apps/codecs/aac.c; read from the esds atom).
2) Seek information (from the atoms stts, stsz, stsc and stco). Keeping the size of each sample (or frame) isn't necessary, as far as I know, so it should be enough to keep a number of seek point, as you suggest. For each seek point, the number of frames is needed (between this point and the next), as well as a time offset (as a sound sample count, I think). That should be enough to be able to decode a file, and seek in it based on file offset or time.
When creating a seek table, remember that there can be gaps between chunks, so a seek point can't always "begin" in one chunk and "end" in the next.
is one of the mov data being after the mdat data. The second mdat atom
in my test track is a phantom (0 length) as can be seen from this log:
audio_current_track: no id3 data
res = 0
read_mp4_container(16, 0x991680, 3933035)
un-handled case: type=0x17809
un-handled case: type=0x38c
second MP4_mdat (0), this file may not play
read_mp4_container(16, 0x991680, 96294)
un-handled case: type=0x64
read_mp4_container(16, 0x991680, 56705)
un-handled case: type=0x54
un-handled case: type=0x1c
read_mp4_container(16, 0x991680, 56569)
un-handled case: type=0x18
read_mp4_container(16, 0x991680, 56471)
un-handled case: type=0x8
un-handled case: type=0x1c
read_mp4_container(16, 0x991680, 56411)
read_mp4_container(16, 0x991680, 983)
un-handled case: type=0x2c30
un-handled case: type=0x9d54
un-handled case: type=0xec0
read_mp4_container(16, 0x991680, 39423)
un-handled case: type=0x22
MP4 bitrate 127, frequency 44100 Hz, length 233732 ms
audio_finish_load_track
Looking for album art for /03 - This House Is Not for Sale.m4a
audio_finish_load_track: elap=0, offset=0, codectype=12
foo!
audio_finish_load_track done
aac:codec_main
qtmovie_read: Found a chunk ftyp, length=32
qtmovie_read: Found a chunk free, length=96273
qtmovie_read: Found a chunk free, length=916
qtmovie_read: Found a chunk mdat, length=3739504
read_chunk_mdat: off=97229, len=3739496
qtmovie_read: Found a chunk mdat, length=8
qtmovie_read: Found a chunk moov, length=96302
format: mp4a
audioType=64, maxBitrate=136288, avgBitrate=127993
curpos=3837248, j=3837254 - Skipping 6 bytes
entry_remaining=896
NeAACDecInit2
codec_main: entering main decoding loop
sdl_audio_callback: No Data.
This presents me with a number of possibilities. One is simply to make
the MP4 decoder handle such simple cases or find some more MP4s with
proper multiple mdat assignments.
commit 35bfaf95659727a5fd06d1c34b85851f04c0dbc3
Author: Alex Bennee <alex@bennee.com>
Date: Wed May 6 07:52:44 2009 +0100
Wind stream position back to first mdat atom before starting playback.
This also leaves a FIXME for dealing with seeking in libm4a which needs fixing
apps/codecs/aac.c | 6 +++++-
apps/codecs/libm4a/demux.c | 3 ++-
apps/codecs/libm4a/m4a.c | 5 +++--
3 files changed, 10 insertions(+), 4 deletions(-)
commit 17a54ad890d52d4ce523333e79aa184eb84d89ac
Author: Alex Bennee <alex@bennee.com>
Date: Wed May 6 07:51:17 2009 +0100
Commentry for playback.c
apps/playback.c | 29 +++++++++++++++++------------
1 files changed, 17 insertions(+), 12 deletions(-)
commit e67cdc0582b9c80c392596ac15183911b66f3b6d
Author: Alex Bennee <alex@bennee.com>
Date: Tue May 5 18:37:35 2009 +0100
Parse MP4 files for multiple mdat chunks
apps/codecs/libm4a/demux.c | 97 ++++++++++++++++++++++++++++++++-----------
apps/codecs/libm4a/m4a.h | 21 +++++++--
2 files changed, 88 insertions(+), 30 deletions(-)
commit b4686e71a5fdeac751cba61feb391d8e78bec87b
Author: Alex Bennee <alex@bennee.com>
Date: Tue May 5 18:35:29 2009 +0100
Bit more info in DEBUGF
apps/metadata/mp4.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
commit 11c4a69120d72a10c593b237f864e9a5ea26723f
Author: Alex Bennee <alex@bennee.com>
Date: Wed Apr 22 13:17:00 2009 +0100
Clean up the MP4 metadata parsing.
The parser used to finish as soon as it saw an "mdat" atom and has set
the filesize. There is nothing that says all you metadata will have
been seen at this point. I've tweaked the parser to ensure it goes all
the way through the file scanning for metadata.
If tested this with an m4a of iTunes which mp4info parsed OK:
09:44 alex@danny/x86_64 [simdisk] >mp4info 03\ -\ This\ House\ Is\ Not\ for\ Sale.m4a
mp4info version 1.5.0.1
03 - This House Is Not for Sale.m4a:
Track Type Info
1 audio MPEG-4 AAC LC, 233.732 secs, 128 kbps, 44100 Hz
Metadata Name: This House Is Not for Sale
Metadata Artist: Ryan Adams
Metadata Year: 2003
Metadata Album: Love Is Hell Part 1
Metadata track: 3 of 10
Metadata Disk: 1 of 1
Metadata Genre: Alt.Country
Metadata Cover Art pieces: 1
However the file still won't play on Rockbox, I assume because the
codec that does the actual playing cannot deal with the unusual
structure of the m4a. Interestingly it plays the other songs of the
album fine!
apps/metadata/mp4.c | 116 +++++++++++++++++++++++++++++++++++----------------
1 files changed, 80 insertions(+), 36 deletions(-)
FS#10832