-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Reuse decoder instances on re-preparation + when seeking to a different period #2826
Comments
We should also look at re-using decoder instances when a new MediaSource is passed via prepare(), where it's possible to do so. |
+1 😉 |
Is there any help we can provide in improving this aspect? Have you already thought about potential design changes, or already have an idea that we can try to implement? TA |
Work is in progress. There's a design proposal here. Feel free to post any comments you have. I don't think we need any help on the implementation side. |
More specifically, if the parts of the Format that are used for decoder configuration are unchanged. Issue: #2826 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=214791234
- It's a no-op for now - Renderers that want to support retaining resources will move some functionality from their disable() implementations into reset() - ExoPlayerImplInternal will be updated to not always call reset() immediately after disable(), which is what will enable resources to actually be retained. Issue: #2826 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=215729783
For decoder reuse, we want disable() to flush the decoder. However, if the flush needs to release the decoder for some reason, it seems non-ideal to immediately re-initialize it. Re-initialization can also throw ExoPlaybackException, which we don't want for disabling. This change allows a variant of flush that wont re-initialize the decoder if it has to be released, which will be used from disable(). Issue: #2826 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=216834862
Issue: #2826 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=216852679
Issue: #2826 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=217189082
*** Reason for rollback *** Broke photos (b/117818304) *** Original change description *** Add ExoPlayer.setForegroundMode Issue: #2826 *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=217356705
*** Reason for rollback *** Photos regression is resolved by [] *** Original change description *** Automated g4 rollback of changelist 217189082. *** Reason for rollback *** Broke photos (b/117818304) *** Original change description *** Add ExoPlayer.setForegroundMode Issue: #2826 *** *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=217773278
There are multiple subtle issues with the current implementation: 1. setOperatingRate can cause a codec initialization even if the renderer is disabled. This is not supposed to happen. 2. If the codec is released whilst the renderer is disabled, the renderer can instantiate a new codec using the old format when it's enabled again, only to immediately have to reconfigure or release it if the actual format to be played is different. 3. Codec reuse does not take into account renderer configuration. The specific case where this is problematic is if the video renderer is re-enabled with a different tunneling session id. The reused codec is then not configured correctly. Also moved availableCodecInfos reset into releaseCodec for sanity. Issue: #2826 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=217924592
Issue: #2826 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=218332277
Issue: #2826 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=218346327
Issue: #2826 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=218540967
This paves the way to cleanly fix the first two issues listed in [] onDisabled will null inputFormat, but nulling of codecFormat will remain tied to the codec being released. Issue: #2826 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=219125458
Issue: #2826 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=219130576
*** Original change description *** Re-enable codec re-use Issue: #2826 *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=219585084
*** Reason for rollback *** Rolling forward again as [] should fix issue that prompted the rollback *** Original change description *** Automated g4 rollback of changelist 219130576. *** Original change description *** Re-enable codec re-use Issue: #2826 *** *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=220124362
@ojw28 I read the linked design proposal. It's so great. Is there a similar public design documents? If have, please give me the links. I am reading the ExoPlayer source code.Thanks! |
Thanks for the feedback! There aren't, but I think it's a good idea for us to try and publish more of our design documents externally. This was a bit of a trial run for doing that. |
@ojw28 Reselect tracks manually when playing HLS stream ,and then decoder instances re-init. Whether reuse decoder instances at this situation is feasible? |
That doesn't happen when I try, except in the case where the re-selection is selecting a video stream bigger than anything that the decoder was previously configured to play (in this case release and re-initialization is necessary because the existing decoder wont have sufficiently large buffers). |
Dose this mean re-selection from low bit rate video stream to high bit rate video stream? @ojw28 ExoPlayer version: 2.9.0
|
@ojw28 Did you already had the time to look into this? Is there anything I can do to support you? |
Decoder re-use is not part of the 2.9.0 release, or any release to date. It's in the |
Closing because this is supported in |
ExoPlayer V2 re-uses decoder instances where possible as it plays through multiple periods of content (e.g. that have been concatenated using
ConcatenatingMediaSource
).However the decoders are always released and re-instantiated if the user explicitly seeks to a period other than the one currently being played, even in cases where it's possible that the instances could be retained and re-used. We should look at optimizing this case.
The text was updated successfully, but these errors were encountered: