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

Add support for building a SeekMap based on ID3 MLLT tag #3241

Closed
rustyshelf opened this issue Sep 6, 2017 · 9 comments
Closed

Add support for building a SeekMap based on ID3 MLLT tag #3241

rustyshelf opened this issue Sep 6, 2017 · 9 comments
Assignees

Comments

@rustyshelf
Copy link

Issue description

When playing back the file linked to below ExoPlayer determines the duration to be 1 hour and 18 seconds. Every other playback framework I could find (Chrome Browser, iOS AVPlayer, Quicktime, VLC, Logic) all report the length of 29 minutes, 20 seconds. Interestingly VLC shows 1:00:18 before you hit play on the file, then changes it to 29 minutes.

Oddly ExoPlayer seems to play through all 1 hour of what it thinks it has, but it repeats content from other parts of the file as it does. If I had to guess I'd say it's taken the two stereo tracks and is playing them back to back? That's just a guess though.

I'm not entirely sure what causes this is but I thought it would be worth reporting since ExoPlayer seems to be the odd one out here.

Reproduction steps

  • Play this file in ExoPlayer
  • Duration reported is over an hour, should be 29 minutes, 20 seconds

Link to test content

http://traffic.libsyn.com/macintoshfm/emoji1_final1.mp3

Version of ExoPlayer being used

2.5.1

Device(s) and version(s) of Android being used

Google Pixel, running Android 8. Nexus 6P running Android 7. Samsung S7 Running Android 7. Seems to be non-device specific

@andrewlewis
Copy link
Collaborator

This stream is being treated as constant bitrate 80 kbit/s but the bitrate actually varies throughout the file. I guess the repetition only happens if you seek?

It looks like there is a VBRI header in the file so I'll look into why that isn't getting parsed.

@rustyshelf
Copy link
Author

Oh interesting. To be honest I didn't try just letting it play without ever seeking it. I might do that and report back.

@andrewlewis
Copy link
Collaborator

The VBRI header gets parsed correctly if I increase the frame size of the first audio frame in the file by nine bytes: just before this line add:

if (input.getPeekPosition() == 466191) {
  frameSize += 9;
  synchronizedHeader.frameSize += 9;
}

Then the VBRI header is picked up and the media appears to play/seek correctly.

I had a quick look at our frame size parsing logic in MpegAudioHeader and it looks like we do match the platform. It could be a bug or it could just be that the media is not valid.

@rustyshelf
Copy link
Author

I guess that's entirely up to you, but it does seem to work on everything from browsers to iOS to desktop applications. I haven't yet been able to find a player that does what ExoPlayer does in this case. Could just be an odd edge case though.

@andrewlewis
Copy link
Collaborator

I stepped through the audio header parsing logic and found that frame size matched what FFmpeg would calculate. Maybe other players have more accepting logic than ExoPlayer: we require four consecutive valid MPEG audio frames before treating the position as 'synchronized', which matches the platform.

At this point I'm inclined to say this media is bad but please reopen if you find there's a bug or discover that there are lots of streams like this one, and we'll investigate implementing a fix/workaround. Thanks!

@rustyshelf
Copy link
Author

rustyshelf commented Oct 13, 2017

So we're getting the same issue on a popular podcast that has hundreds of thousands of subscribers link to file

This is with ExoPlayer 2.5.3.

Just like in the other case the duration is twice as long as it should be, and seeking works, but not well, it goes to the wrong part of the file.

@andrewlewis I don't think I am able to re-open this issue but if you could take a look that would be amazing.

@andrewlewis andrewlewis reopened this Oct 13, 2017
@rustyshelf
Copy link
Author

Also a related query (but not the actual bug above): does ExoPlayer support the MLLT ID3 tag detailed here: http://id3.org/id3v2.3.0#sec4.7

This file contains that tag but not sure if it's used in ExoPlayer or not.

@andrewlewis
Copy link
Collaborator

We don't parse that tag at the moment. If it's commonly used and provides enough information to get the right duration and seek map, we should probably add support for it. Since both problematic streams on this issue appear to have the tag, I'll mark this as an enhancement.

@andrewlewis andrewlewis changed the title ExoPlayer Duration Issue Add support for building a SeekMap based on ID3 MLLT tag Oct 27, 2017
@rustyshelf
Copy link
Author

Nice one. In terms of precision I believe the order should be MLLT > VBRI > XING (from most precise to least)

ojw28 pushed a commit that referenced this issue Oct 18, 2018
Issue: #3241

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=217252254
ojw28 pushed a commit that referenced this issue Oct 20, 2018
Issue: #3241

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=217252254
@ojw28 ojw28 closed this as completed Oct 20, 2018
@google google locked and limited conversation to collaborators May 16, 2019
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

3 participants