-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
More stream selection refactors #10733
base: dev
Are you sure you want to change the base?
More stream selection refactors #10733
Commits on Jan 5, 2024
-
Simplify selection of video stream
I was trying to understand the logic here, and noticed the indirection via a QualityResolver interfaces is pretty unnecessary. Just branching directly makes the logic a lot easier to follow. The `-999` sentinel value is a bit dumb, but java does not recognize that videoIndex is always initialized. Nice side-effect, the `Resolver` interface was completely unused and can be dropped.
Configuration menu - View commit details
-
Copy full SHA for 76eb751 - Browse repository at this point
Copy the full SHA 76eb751View commit details
Commits on Jan 6, 2024
-
Configuration menu - View commit details
-
Copy full SHA for 0148d65 - Browse repository at this point
Copy the full SHA 0148d65View commit details -
Configuration menu - View commit details
-
Copy full SHA for be4e0cb - Browse repository at this point
Copy the full SHA be4e0cbView commit details -
Remove null check on getDefaultSharedPreferences
The function never returns `null`.
Configuration menu - View commit details
-
Copy full SHA for e5ac682 - Browse repository at this point
Copy the full SHA e5ac682View commit details -
ListHelper: rename functions & variables and add documentation
This should make the purpose of these functions more clear.
Configuration menu - View commit details
-
Copy full SHA for 71d88d0 - Browse repository at this point
Copy the full SHA 71d88d0View commit details -
VideoPlaybackResolver: implicit -1 videoIndex
All functions that set videoIndex return `-1` if the list is empty, so we don’t have to check manually for that case. I’m somewhat more questioning why `StreamInfoTag.of` allows the index to be out-of-bounds in the constructor, that should be an error.
Configuration menu - View commit details
-
Copy full SHA for 64c68cf - Browse repository at this point
Copy the full SHA 64c68cfView commit details -
Configuration menu - View commit details
-
Copy full SHA for e0cbddf - Browse repository at this point
Copy the full SHA e0cbddfView commit details -
Quality/AudioTrack: ensure correct indices at construction
Instead of allowing to pass arbitrary out-of-bounds indexes to these bean classes, ensure that the index is always valid for the list. This is always true for our filter functions, except they all return `-1` if the list was empty. We have to check/assert that beforehand. This improves the logic somewhat, because fetching the stream always returns something now. Ideally, we wouldn’t be filtering stuff and then passing indices around everywhere, but that’s the current state of things. ~~~ I took the liberty to remove the `.of`-wrappers, because they don’t really add much compared to just calling the constructor here.
Configuration menu - View commit details
-
Copy full SHA for 743e16a - Browse repository at this point
Copy the full SHA 743e16aView commit details -
getFilteredAudioStreams: make trackId more obvious and document bug
Why would you serialize objects to avoid a null check, that’s just weird.
Configuration menu - View commit details
-
Copy full SHA for 24ac6d5 - Browse repository at this point
Copy the full SHA 24ac6d5View commit details -
Configuration menu - View commit details
-
Copy full SHA for 982d236 - Browse repository at this point
Copy the full SHA 982d236View commit details -
Configuration menu - View commit details
-
Copy full SHA for 2f52991 - Browse repository at this point
Copy the full SHA 2f52991View commit details
Commits on Jan 7, 2024
-
Configuration menu - View commit details
-
Copy full SHA for 189c35e - Browse repository at this point
Copy the full SHA 189c35eView commit details -
getFilteredAudioStreams: Use language and tracktype for grouping
Instead of the `trackId`, which is only ever set for the `youtube` backend, we use the audio stream language and tracktype. These are `null` for youtube videos without language selector, leading to a single audio stream being selected. For media.ccc.de, the language on audio tracks is always set correctly, meaning for a video with German and English tracks we get two groups, which are then sorted according to the preferences of users. I verified that youtube still works by playing https://www.youtube.com/watch?v=8bDRVP9xSfc and selecting the different audio streams, going to background etc. and also tried with a video without multiple audio tracks. For media.ccc.de, it will select the right audio track as well. Unfortunately, media.ccc.de returns videos with multiple audio track muxed into the video itself, which is still buggy because the wrong track is selected by default. This is gonna be fixed/worked around in the next commit.
Configuration menu - View commit details
-
Copy full SHA for e242ad4 - Browse repository at this point
Copy the full SHA e242ad4View commit details -
Configuration menu - View commit details
-
Copy full SHA for d134219 - Browse repository at this point
Copy the full SHA d134219View commit details