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

new TrackSelection not updated in HlsChunkSource #6256

Closed
stevemayhew opened this issue Aug 1, 2019 · 6 comments
Closed

new TrackSelection not updated in HlsChunkSource #6256

stevemayhew opened this issue Aug 1, 2019 · 6 comments
Assignees
Labels

Comments

@stevemayhew
Copy link
Contributor

stevemayhew commented Aug 1, 2019

Issue description

When playing back adaptive HLS content if you change the playback speed the AdaptiveTrackSelection should level down to a lower bitrate track to avoid stalls, it does not because the playbackSpeed update is missed since the HlsChunkSource that calls this method references a stale TrackSelection.

The code that handles this is:

  @SuppressWarnings("unused")
  protected boolean canSelectFormat(
      Format format, int trackBitrate, float playbackSpeed, long effectiveBitrate) {
    return Math.round(trackBitrate * playbackSpeed) <= effectiveBitrate;
  }

Reproduction steps

Playback using any HLS content (I used the Mango Open Source video: https://mango.blender.org, I'll email an S3 bucket with an HLS version of this). Then:

  1. Disable the audio track
  2. Print or observer with a debugger the TrackSelection object reference in HlsChunkSource, it should change to match that in MediaPeriodHolder

Triage

This is easy to reproduce by just disabling the audio track. This forces a invalidate (TrackSelector.invalidate() called). The invalidate creates a new AdaptiveTrackSelection object, via TrackSelection.Factory.createTrackSelection()

The updated TrackSelection instance is stored in the MediaPeriodHolder but not updated in the HlsChunkSource. This is because of logic to avoid re-creating the SampleStream (a good thing) in HlsSampleStreamWrapper

Link to test content

Mango Open Source - Tears of Steel (I'll send a link)

A full bug report captured from the device

I'll send it, probably useless though, the issue is a logic issue not platform specific.

Version of ExoPlayer being used

dev-v2-r2.10.3 - 1275217

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

Evolution IPA-1104HDH-01. SDK 26

@tonihei
Copy link
Collaborator

tonihei commented Aug 2, 2019

Thanks for reporting. It looks this bug also affects other streaming types where we save a reference to the TrackSelection (e.g. DASH).

@stevemayhew
Copy link
Contributor Author

You're more than welcome! Here's a proposed fix that is working for me locally: stevemayhew@ddfaa97

I am working from dev-v2-r2.10.3 so it will not directly merge with a pull.

I looked to write a test case, but that would entail building out test cases for the whole of HlsSampleStreamWrapper. We can talk about that project later if it's a worthy undertaking (lots of big methods would need to be split up to make the logic more testable, a bigger job then I would attempt with my experience current experience with the class).

If this is good, I'll move the change to base off dev-v2 and submit it as a pull.

@tonihei
Copy link
Collaborator

tonihei commented Aug 5, 2019

Thanks for the proposal. I re-implemented this in a slightly different way to move the update further down where we keep the streams.

@stevemayhew
Copy link
Contributor Author

stevemayhew commented Aug 5, 2019 via email

ojw28 pushed a commit that referenced this issue Aug 5, 2019
If we keep streams in chunk sources after selecting new tracks, we also keep
a reference to a stale disabled TrackSelection object. Fix this by updating
the TrackSelection object when keeping the stream. The static part of the
selection (i.e. the subset of selected tracks) stays the same in all cases.

Issue:#6256
PiperOrigin-RevId: 261696082
ojw28 pushed a commit that referenced this issue Aug 5, 2019
If we keep streams in chunk sources after selecting new tracks, we also keep
a reference to a stale disabled TrackSelection object. Fix this by updating
the TrackSelection object when keeping the stream. The static part of the
selection (i.e. the subset of selected tracks) stays the same in all cases.

Issue:#6256
PiperOrigin-RevId: 261696082
@ojw28
Copy link
Contributor

ojw28 commented Aug 5, 2019

Fix merged into dev-v2 and dev-v2-r2.10.4. If you could give it a quick test, that would be great.

@ojw28 ojw28 closed this as completed Aug 5, 2019
@stevemayhew
Copy link
Contributor Author

stevemayhew commented Aug 6, 2019 via email

@google google locked and limited conversation to collaborators Oct 5, 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

3 participants