WIP: Work around and demo for stuck buffering tunneling issue #6400
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…oads PiperOrigin-RevId: 243954989
ImaAdsLoader gets the player position after the app releases the player to support resuming ads at their current position if the same ads loader is reused. PiperOrigin-RevId: 243969916
One goal we forgot about a little bit was to allow applications to provide their own index implementation. This requires the writable side to also be defined by an interface. PiperOrigin-RevId: 243979660
Issue: google#5772 PiperOrigin-RevId: 243987497
PiperOrigin-RevId: 243988105
PiperOrigin-RevId: 244003817
Currently SimpleCache will touch cache spans whenever it reads from them. With legacy SimpleCache setups this involves a potentially expensive file rename. With new SimpleCache setups it involves a more efficient but still non-free database write. For offline use cases, and more generally any use case where the eviction policy doesn't use last access timestamps, touching is not useful. This change allows the evictor to specify whether it needs cache spans to be touched or not. SimpleCache will only touch spans if the evictor requires it. Note: There is a potential change in behavior in cases where a cache uses an evictor that doesn't need cache spans to be touched, but then later switches to an evictor that does. The new evictor may temporarily make sub-optimal eviction decisions as a result. I think this is a very fair trade-off, since this scenario is unlikely to occur much, if at all, in practice, and even if it does occur the result isn't that bad. PiperOrigin-RevId: 244005682
PiperOrigin-RevId: 244009343
1. customCacheKey for DASH/HLS/SS is now asserted against in DownloadRequest 2. Merging of event delivery in DownloadManager is very tricky to get right and probably not a good idea PiperOrigin-RevId: 244048392
PiperOrigin-RevId: 244094942
PiperOrigin-RevId: 244139959
PiperOrigin-RevId: 244168713
PiperOrigin-RevId: 244170179
…ction Assuming there is no text language preference. PiperOrigin-RevId: 244176667
PiperOrigin-RevId: 244196081
PiperOrigin-RevId: 244210737
PiperOrigin-RevId: 244216620
PiperOrigin-RevId: 244223870
PiperOrigin-RevId: 244226680
- Listener based reporting of progress allows the content length to be persisted into the download index (and notified via a download state change) as soon as it's available. - Moved contentLength back into Download proper. It should only ever change once, so I'm not sure it belongs in the mutable part of Download. - Made a DownloadProgress class, for naming sanity. PiperOrigin-RevId: 244242487
PiperOrigin-RevId: 244267255
PiperOrigin-RevId: 244268855
- We had buildAddRequest and sendNewDownload. Converged to buildAddDownload and sendAddDownload. - Also fixed a few more inconsistencies, and brought the action constants into line as well. PiperOrigin-RevId: 244274041
This ensures we keep the loading period in sync with the the playing period in PlybackInfo, when the latter changes to something new. PiperOrigin-RevId: 244838123
Disabling stack trackes currently logs messages twice, once with and once without stack trace. PiperOrigin-RevId: 244853127
PiperOrigin-RevId: 244864742
PiperOrigin-RevId: 245014381
PiperOrigin-RevId: 261102180
Android considers ALAC initialization data to consider of the magic cookie only, where-as FFmpeg requires a full atom. Standardize around the Android definition, since it makes more sense (the magic cookie being contained within an atom is container specific, where-as the decoder shouldn't care what container the media stream is carried in) Issue: google#5938 PiperOrigin-RevId: 261124155
Issue: google#5938 PiperOrigin-RevId: 261150349
PiperOrigin-RevId: 244173887
They're not unexpected! PiperOrigin-RevId: 260907687
PiperOrigin-RevId: 260970865
Issue: google#6241 PiperOrigin-RevId: 261126968
A previous change switched to calculation of the bitrate based on the first MPEG audio header in the stream. This had the effect of fixing seeking to be consistent with playing from the start for streams where every frame has the same padding value, but broke streams where the encoder (correctly) modifies the padding value to match the declared bitrate in the header. Issue: google#6238 PiperOrigin-RevId: 261163904
- When in STATE_SEEK with targetGranule==0, seeking would exit without checking that the input was positioned at the correct place. - Seeking could fail due to trying to read beyond the end of the stream. - Seeking was not robust against IO errors during the skip phase that occurs after the binary search has sufficiently converged. PiperOrigin-RevId: 261317035
- Test seeking to (timeUs=0, position=0), which should always work and produce the same output as initially reading from the start of the stream. - Reset the input when testing seeking, to ensure IO errors are simulated for this case. PiperOrigin-RevId: 261317898
PiperOrigin-RevId: 261320318
PiperOrigin-RevId: 261328701
PiperOrigin-RevId: 261340526
PiperOrigin-RevId: 261341256
PiperOrigin-RevId: 261353271
Issue:google#6260 PiperOrigin-RevId: 261647858
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:google#6256 PiperOrigin-RevId: 261696082
To diagnose the MusicChoice playback issues.
Add logging to show tunnel session id
See issue google#6366 for more blow by blow of the issue. The work around is to keep the video renderer ready, even if it is out of upstream samples, as long as it has samples in the codec that will eventually hit their render time. A more "correct fix" might be to understand and implement the logic for AudioTrack.write() when the audio track is paused.
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
stevemayhew
commented
Sep 5, 2019
| // This "works around" the problem, even if it is not time to render a frame, and we don't have samples | ||
| // (because the entire LoadControl limited buffered sample queue is in the decoder), stay 'ready' to render the frame | ||
| // when it is due. | ||
| if ((super.isReady() || buffersInCodecCount > 0) && (renderedFirstFrame || (dummySurface != null && surface == dummySurface) | ||
| || getCodec() == null || tunneling)) { |
There was a problem hiding this comment.
The 'fix' is to keep reporting ready, even if we have fully buffered samples, as long as buffers are in the decoder that have not been presented to the output.
There was a problem hiding this comment.
The obvious drawback is the main doSomeWork() loop will not transition to buffering playback state in the event there are decoded buffers that have not been rendered.
|
I've cleaned the fix up and opened a new pull request. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull is just to expose the issue, logging and one workaround/fix for discussion.
The fix is for the "Playback stalls forever in buffering with "Music Choice" content tunneling. Issue #6366
Look at the change in MediaCodecVideoRenderer for the 'workaround/fix'
I will close and re-open a new pull request with a proper base to
dev-v2and just the final fix.