State Ended is postponed if the content has incorrect duration set

[zsmatyas]

I am experiencing problems with mp3 files listed on the BBC World Service - Hourly Bulletin (http://www.bbc.com/news/world-17434527) The content changes every few minutes, so the issue is not always reproducible.

The content is:
Type: Audio
Bitrate: 32 kb/s
Sample rate: 22050 Hz
Channels: Mono
Codec: MPEG Audio layer 1/2/3 (mpga)

The content has variable bitrate, every player has different methods to find out the real duration. For example in a bad case (I send you the mp3 file separately as well, as the one on the BBC server keeps changing), ExoPlayer thinks the file length is 3:08. And although the AudioTrackRenderer reaches the end of content at about 2:00, no more data can be fed, ExoPlayer is keept in the Playing state. The player will transition to State Ended only when the position reaches 3:08. The result is 68 additional seconds of silence played.

in ExoPlayerImplInternal.java line 464:

if (allRenderersEnded
        && (durationUs == TrackRenderer.UNKNOWN_TIME_US || durationUs <= positionUs)) {
setState(ExoPlayer.STATE_ENDED);
...

According to the git log that additional condition of the position was introduced to handle the case when all renderers are disabled:
0ee8c07

mp3info:
Media Type: MPEG 2.0 Layer III
Audio: Variable kbps, 22 kHz (mono)
Emphasis: none
CRC: No
Copyright: No
Original: No
Padding: No
Length: 1:59

Reproducible:

• Every android device (like my nexus 5x) using any ExoPlayer I tested (1.5.0, 1.5.6)
• It is a 100% repro with the appropriate content

bugreport.txt

Yep, I know, the content is kind a invalid, but as I cannot influence the content, I would like to handle this case in the player.

[andrewlewis]

The problem is that this variable bitrate file has no variable bitrate header, so we assume it is constant bitrate and calculate the wrong duration. If you play the file using the platform's MediaPlayer (using e.g. adb shell am start -a android.intent.action.VIEW -d "file://path/to/file.mp3" -t "audio/*") you can see that the seek bar jumps to the end suddenly after two minutes, for the same reason.

To guarantee getting the correct duration in general, we would need to read the whole stream, which is not feasible. Alternatively, we could use 'unknown' duration for MP3 files without a header, but this would lose useful information when playing valid CBR files.

If your app is playing audio streams only, one possible workaround is to override MediaCodecAudioTrackRenderer.onOutputStreamEnded() and use that method to signal to your app that playback has ended. That won't help you provide correct player seeking controls, but perhaps you know the duration of the stream already from some other source.

We could take into account the duration of disabled renderers only in the condition to transition to STATE_ENDED in ExoPlayerImplInternal, so that when all enabled renderers have ended we would immediately transition to the ended state if there are no disabled renderers with a known duration after the current position. (Perhaps that is the fix you are hinting at in your comment.) Then ExoPlayer's behavior would match the platform. I am not convinced that it's worth the added complexity to handle input that is unsuitable for streaming, though.

[zsmatyas]

Agreed. But 0ee8c07 seems to be slightly incorrect.

The condition

if (allRenderersEnded
        && (durationUs == TrackRenderer.UNKNOWN_TIME_US || durationUs <= positionUs)) {
setState(ExoPlayer.STATE_ENDED);
...

is missing a case when there are renderers enabled. Based on the history, it was added to handle the case when all renderers are disabled or duration is known. What about

if (allRenderersEnded
        && (durationUs == TrackRenderer.UNKNOWN_TIME_US || durationUs <= positionUs || enabledRenderers.size() > 0)) {
setState(ExoPlayer.STATE_ENDED);
...

[andrewlewis]

If the duration is known and there is a disabled renderer with a duration that is longer than the enabled renderers' durations, I think we want playback to continue after the enabled renderers have ended (in case the app enables that renderer, which would still have media to play).

[zsmatyas]

I would think that is usually not intended by the users, and the commit messages say that the change is only intended to handle the case when all renderers are disabled.

In ideal case, this will never happen. But with broken content, as we meet them on a weekly bases, it will. For example, if the user disables subtitles, do we want to show the last frame of the video without any audio till we reach the incorrect (arbitrarily distant) timestamp of the disabled text?

I think we can even enhance the additional condition I suggested above to handle these kind of issues.

[ojw28]

As author of the change I can state that the intention was more general than you believe. Fundamentally, if a piece of media has a defined duration, the desired behavior is that the player does not transition to the ended state before that duration has been reached just because one or more of the renderers happens to be disabled.

It should be noted that the vast majority of ExoPlayer usage (in terms of hours of playback) takes place in applications that are provided by providers who generate their content correctly and aren't affected by this kind of issue. Out of the relatively small fraction of usage that's left, only a small fraction of that is trying to play broken content. So in the bigger picture it doesn't make sense for us to start implementing less well defined player behaviors to try and better accommodate broken playbacks. If individual applications that happen to deal with a lot of arbitrary/broken side-loaded content want to patch the version of ExoPlayer that they include to be more permissive, it's not that huge of a technical challenge for them to do this.