Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ID3 Chapter Support #2316

Closed
rustyshelf opened this issue Jan 12, 2017 · 17 comments
Closed

ID3 Chapter Support #2316

rustyshelf opened this issue Jan 12, 2017 · 17 comments

Comments

@rustyshelf
Copy link

I notice the 2.1.0 release added support for ID3 meta data which is great! There's an additional set of tags people use to describe chapter information in an MP3 file which would be really handy to have access to as well (more info here: http://id3.org/id3v2-chapters-1.0). These are reasonably common in podcasts and other long-form audio content.

@ojw28
Copy link
Contributor

ojw28 commented Jan 12, 2017

It should be pretty easy to implement this yourself, if you (or anyone else) feels like sending us a pull request ;)...

@rustyshelf
Copy link
Author

@ojw28 Philip was kind enough to create a pull request with the changes I was talking about. Would love it if you could take a look and see if it's suitable for inclusion, we'd love for this to be part of ExoPlayer itself :)

@ojw28
Copy link
Contributor

ojw28 commented Jan 16, 2017

Yeah, I was just looking. Looks good. I've added a few comments. Once addressed we'll get it merged.

@ojw28
Copy link
Contributor

ojw28 commented Jan 17, 2017

The change has been merged. I made some further changes in 5aff31c and 48099ee. These changes are completely untested as I don't have any media containing ID3 chapter information, so it's highly likely I've broken something. Please could you take a look, test and fix any issues I've introduced. If you could also provide some test content for us, that would be great. You can either attach it here, or email it to dev.exoplayer@gmail.com.

Thanks!

@ojw28
Copy link
Contributor

ojw28 commented Jan 17, 2017

Obvious bugs fixed in c828d9b.

@ojw28
Copy link
Contributor

ojw28 commented Jan 17, 2017

It would be cool to add support for MP4/M4A chapter metadata too, if you fancy giving it a go. It's probably just a matter of adding support in extractor.mp4.MetadataUtil to parse out the ID3 frames. I found some test media here: https://auphonic.com/blog/2013/07/03/chapter-marks-and-enhanced-podcasts/

@geekygecko
Copy link
Contributor

@ojw28 wow thanks for taking the time to go through my change. I really like the way you just used sub frames. I will give those changes a test with the files I am using. I am happy to try adding some tests? Would I just create another pull request for that?
One minor thing I noticed, are the subframes meant to be private as I can't access them?

Also I will take a look at the MP4/M4A chapters to see if I can parse them.

@rustyshelf
Copy link
Author

rustyshelf commented Jan 18, 2017

I had assumed that chapters in M4A files that most podcasters use aren't in ID3 tags of any kind. I'm not even sure there's a specification for that format that's available? Maybe it would be best to track that in another issue and leave this one specific to MP3 ones?

Here's an example of an Enhanced AAC/M4A chaptered file: http://www.maccast.com/media/eMC20170113.m4a

vs an MP3 with ID3 chapters:
http://www.podtrac.com/pts/redirect.mp3/traffic.libsyn.com/upgrade/Upgrade_124.mp3

@ojw28
Copy link
Contributor

ojw28 commented Jan 18, 2017

@rustyshelf - I think you're correct that MP4 files don't use ID3 tags, however the approach we've taken for MP4 metadata has been to convert it into the equivalent ID3 tags as far as is possible. It usually is possible since the metadata atoms in MP4 are in most cases equivalent to ID3 tags. See extractor.mp4.MetadataUtil. For chapter metadata, my guess is that the chpl atom is used, as defined in this spec. It looks like such an atom could probably be converted into a ChapterTOCFrame and N ChapterFrames. Each ChapterFrame would have a single TIT2 sub-frame holding the title.

@geekygecko - No worries! Thanks for the contribution. Please feel free to add additional tests via a new pull request. Sub-frames are indeed not accessible currently, heh! I intended for them to be private and for there to be public getSubframeCount and getSubframe(index) methods (since just making the array public directly doesn't enforce immutability), but somehow forgot to actually add those methods. I'll fix that shortly. Thanks!

@rustyshelf
Copy link
Author

@ojw28 oh interesting, thanks for the link to that spec, I'll take a look tomorrow and see if I can figure it out. Would be a great addition if it's as easy as it sounds.

ojw28 pushed a commit that referenced this issue Jan 18, 2017
Issue: #2316

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=144815010
@ojw28
Copy link
Contributor

ojw28 commented Jan 18, 2017

That would be great, thanks!

@rustyshelf
Copy link
Author

@ojw28 we spent some time looking into this it turns out that m4a files with chapters have a 'chap' LeafAtom (stored in a tref ContainerAtom) which points to the Track that contains the chapter list.

According to Apple's doc (https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html#//apple_ref/doc/uid/TP40000939-CH205-57863):

To create a chapter list, you must create a text track with one sample for each chapter. The display time for each sample corresponds to the point in the movie that marks the beginning of that chapter. You must also create a track reference of type 'chap' from an enabled track of the movie to the text track. It is the 'chap' track reference that makes the text track into a chapter list. The track containing the reference can be of any type (audio, video, MPEG, and so on), but it must be enabled for the chapter list to be recognized.

By wrangling the Mp4Extractor code we were able to find this chap reference that pointed to the Track with the sample information in it. The track had 23 samples in it for the m4a file linked above, which is a file with 23 chapters so that definitely seems to be where the information is stored. The problem though was that we're both unfamiliar with the m4a file format and also the code that's reading it in ExoPlayer, so finding the right way to extract this data and expose it via an API is beyond us. It seems like someone more familiar with the file structure would be able to extract the text information that is the titles, as well as the timing information associated with it.

I'm not expecting you to do it, more just wanted to list all the info here for completeness and for anyone who might need it in the future. Also along the way we found a C++ library on GitHub that shows how to extract the titles and times for chapters out of the samples here: https://github.com/pcwalton/mp4v2/blob/e8d9272cf98bb573264a72c4c462c841f8900857/src/mp4file.cpp#L2512

Thanks again for merging the mp3 chapter code in, really appreciate it. Sorry that we couldn't get there with the m4a data.

@ojw28
Copy link
Contributor

ojw28 commented Jan 20, 2017

Interesting. We currently just expose the chapter data as a text track for that case. Which we probably shouldn't be doing!

@rustyshelf
Copy link
Author

@ojw28 any idea when this might end up in a production release? I'm new to ExoPlayer so don't really know how your release schedules etc work, but we are hoping to ship this library for the first time in our next app update. Naturally I'd prefer to ship a release version vs a development one.

@ojw28
Copy link
Contributor

ojw28 commented Jan 30, 2017

We're planning a release for some time this week.

@rustyshelf
Copy link
Author

@ojw28 awesome!

@ojw28
Copy link
Contributor

ojw28 commented Mar 13, 2017

Closing this as fixed. The MP4/M4A change looks like it could be quite difficult. I propose we file a separate enhancement request for that as and when someone actually wants it, as was also proposed by @rustyshelf further up this issue.

@ojw28 ojw28 closed this as completed Mar 13, 2017
@google google locked and limited conversation to collaborators Jul 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants