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

Wrong progress is reported when using ConcatenatingMediaSource with playback speed that is not 1 #6117

Closed
schyau opened this issue Jul 2, 2019 · 3 comments
Assignees
Labels

Comments

@schyau
Copy link

schyau commented Jul 2, 2019

Issue description

When using ConcatenatingMediaSource and playback speed is not 1, player.getCurrentPosition() reports the wrong value. It happens when transitioning into the next audio. It starts by first reporting a nonzero number at the start of audio, then repeating this same number until the real-life audio progress catches up to that number. When that happens getCurrentPosition will start reporting correctly.

for example:
If track 1 finishes playing, track 2 will start playing and it should start by reporting 0 when player.getCurrentPosition() is called, but instead it reports 2000. 2000 is repeatedly reported until 2000 ms of playback has elapsed in real life for track 2, and then the player will start reporting correctly every time it gets polled again (until the next track).

Other notes:

  • this is an audio only app
  • the problem exists if playback rate is not 1, (I tested using .75, 1.1, and 1.5)

Reproduction steps

Clone this repo, build, install, launch. Click the button to play. Then pay close attention to the TextView that reports progress. Specifically notice when the index increases, you'll see the bug very clearly when the progress stalls before picking back up. I see this bug (barely) when transitioning from index 0 to 1 (there's a ~1 sec stall) - it's really easy to miss. However, it is very apparent from index 1 to 2 (there's a ~5-45 sec stall). As more track transitions happen, the problems range from none to a lot. When the errors do happen, they are pretty consistent - ie: I am able to reproduce the errors at roughly the same track transitions.

btw, the content is slliiiightly NSFW, so I hope you are wearing earphones. (some context on why this content was chosen: I work for Audm, we hire professional voice actors to read articles. This was one of the more problematic articles that we ran into, unfortunately it is also one of the more provocative. To be fair, it's a pretty interesting read.)

My impl uses a SimpleExoPlayer instance and prepares a ConcatenatingMediaSource with 20 ProgressiveMediaSources (m4a audio from assets). The playback rate is set to 2.

Link to test content

Content is in assets

A full bug report captured from the device

bugreport-NYC-2019-07-01-22-45-55.zip

Version of ExoPlayer being used

2.10.2
interesting to note: this did not happen in 2.9.x

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

physical devices - Pixel 3a api28, Galaxy S9 api28
emulator - Pixel XL api28 and 25

reproducible all the time.

@tonihei
Copy link
Collaborator

tonihei commented Jul 3, 2019

Thanks for reporting. I can reproduce in our demo app and will investigate.

@tonihei tonihei self-assigned this Jul 3, 2019
@tonihei
Copy link
Collaborator

tonihei commented Jul 5, 2019

We'll push a fix soon. It was basically a very severe rounding error.

@schyau
Copy link
Author

schyau commented Jul 5, 2019

you are amazing! thanks for the quick turnaround :)

tonihei added a commit that referenced this issue Jul 5, 2019
Currently, we sometimes apply new playback parameters directly and sometimes
through the list of playbackParameterCheckpoints. Only when using the checkpoints,
we also reset the offset and corresponding position for speedup position
calculation. However, these offsets need to be changed in all cases to prevent
calculation errors during speedup calculation[1].

This change channels all playback parameters changes through the checkpoints to
ensure the offsets get updated accordingly. This fixes an issue introduced in
31911ca.

[1] - The speed up is calculated using the ratio of input and output bytes in
SonicAudioProcessor.scaleDurationForSpeedUp. Whenever we set new playback
parameters to the audio processor these two counts are reset. If we don't reset
the offsets too, the scaled timestamp can be a large value compared to the input
and output bytes causing massive inaccuracies (like the +20 seconds in the
linked issue).

Issue:#6117
PiperOrigin-RevId: 256533780
@tonihei tonihei closed this as completed Jul 5, 2019
ojw28 pushed a commit that referenced this issue Jul 9, 2019
Currently, we sometimes apply new playback parameters directly and sometimes
through the list of playbackParameterCheckpoints. Only when using the checkpoints,
we also reset the offset and corresponding position for speedup position
calculation. However, these offsets need to be changed in all cases to prevent
calculation errors during speedup calculation[1].

This change channels all playback parameters changes through the checkpoints to
ensure the offsets get updated accordingly. This fixes an issue introduced in
31911ca.

[1] - The speed up is calculated using the ratio of input and output bytes in
SonicAudioProcessor.scaleDurationForSpeedUp. Whenever we set new playback
parameters to the audio processor these two counts are reset. If we don't reset
the offsets too, the scaled timestamp can be a large value compared to the input
and output bytes causing massive inaccuracies (like the +20 seconds in the
linked issue).

Issue:#6117
PiperOrigin-RevId: 256533780
@google google locked and limited conversation to collaborators Oct 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants