feat: Support default quality and media type in scene player - #7061
feat: Support default quality and media type in scene player#7061leclerc-leo wants to merge 1 commit into
Conversation
| const label = sources[i].label ?? ""; | ||
| const matchQuality = preferredQuality ? label.endsWith(preferredQuality) : true; | ||
| const matchType = preferredType ? label.startsWith(preferredType) : true; | ||
| if (matchQuality && matchType) return i; |
There was a problem hiding this comment.
If you select MKV plus a resolution such as 1080p in the settings, opening an MKV scene falls back to Direct stream instead of selecting MKV. The matching logic requires the label to match both the preferred type and quality, but MKV and Direct stream labels do not include resolution suffixes, so MKV can never match when a non-original quality is selected.
Additionally, I may be missing some context here, but parsing SceneStreamEndpoint.label for behavior feels fragile. It might be better to expose structured metadata on SceneStreamEndpoint such as stream type and resolution, and keep label display-only.
| } | ||
|
|
||
| setSources(sources: ISource[]) { | ||
| setSources( |
There was a problem hiding this comment.
Changing the default stream type/quality while the current scene remains mounted does not reliably update the selected source. ScenePlayer.tsx passes the new preferences to sourceSelector.setSources, but that code only runs past the scene.id === sceneId.current guard during scene initialization, and the new config fields are not part of a separate reselection path. This means that if you change the setting and refresh a scene page, it breaks and will not load the video to play. It can be resolved by leaving the page and reopening the scene.
There was a problem hiding this comment.
Just wanted to note that the issue still exists. Steps to test:
- Have a tab playing a scene
- on another tab, change the default settings to another type
- refresh the first scene tab
- Note that it has updated to the new default but the video is black screened and doesnt play
973cb1b to
2035ed3
Compare
|
Thanks for the feedback. I probably forgot to re-check MKV and Direct Stream after making some changes, I've re-tested before pushing this change and it now work as I specified in the PR description. I added a useEffect to solve the issue you talked about with the settings that could change (this seemed for me this easiest and cleanest solution). In the initial useEffect that call Please let me know what you think is best. |
| preferredType: string | undefined, | ||
| preferredResolution: string | undefined | ||
| ): number { | ||
| for (let i = sources.length - 1; i >= 0; i--) { |
There was a problem hiding this comment.
Because this cycles through the list from the bottom up, it will match with the bottom type if no preference is selected. You can test this by 1) ensure the settings are written to config with No Preference/Original 2) Load a scene 3) Notice that it defaults to Dash instead of Direct Stream
An additional note on this is the fact that when you first load this build, you don't see this issue because it treats the unset "" and set "ORIGINAL" as different values
| } | ||
|
|
||
| setSources(sources: ISource[]) { | ||
| setSources( |
There was a problem hiding this comment.
Just wanted to note that the issue still exists. Steps to test:
- Have a tab playing a scene
- on another tab, change the default settings to another type
- refresh the first scene tab
- Note that it has updated to the new default but the video is black screened and doesnt play
bb6f832 to
ded6c21
Compare
|
Thanks again. I've finally been able to reproduce the issue (I'm using something similar to https://github.com/nerethos/docker-stash, which edit the transcode a bit, I'm now using a more basic docker image) The fix I applied is to call However, this seems to be due to the setting "FFmpeg hardware encoding" as when it's off, the issue doesn't happen. |
ded6c21 to
f82ba54
Compare
If Direct stream or MKV is selected, since they do not support any quality, the quality will have no effect. If No preference is selected as a media type, the first media type that has an available quality will be selected. Otherwise, the media type with its specifc quality will be selected. For example, MP4 Full HD (1080p).
f82ba54 to
4ef3432
Compare
|
Hi 👋 The issue appears in this PR due to Before, when a player was ready, it would trigger Now, since the When the user tries to play the video To resolve this issue, I'm catching the failed If you have any question about this issue (which I know I'm not very good at explaining things) or anything else, please let me know. |
Description
If Direct stream or MKV is selected, since they do not support any resolution, the resolution will have no effect.
If No preference is selected as a stream type, the last stream type that has an available resolution will be selected.
Otherwise, the stream type with it's specific resolution will be selected. For example, MP4 Full HD (1080p).
In this PR, I'm adding (what I think is) a easy way to select a default source in the ScenePlayer.
I've split it in two, with the possibility to choose a preferred: Stream Type and Resolution.
This allows with only a few options to select one of the 26 possible options.
Here's the full table of what source is selected based on the Stream type on top and Resolution on the left.
However, since some source are not always present like MKV or on Safari browser, I've added a "No preference" Stream Type to fallback on the first Stream Type with the selected Resolution.
P.S.
I did think about making the Stream Type an Ordered List of Preference.
However, this complicated the logic a lot. And (In my opinion), It didn't solve much more than what the "No preference" already does.
But since I'm not at all an expect on every exception that can occur, let me know if you think it would be worth the complexity.
Related Issue
Resolves #2523
Resolves #4175
Testing
I've checked that the table I've given on top was correct by testing a few options like:
I did not test on Safari browser since I do not own one.
Screenshots
Checklist
AI Usage Disclosure
I have used Claude inside VSCode to make a draft/PoC so I was faster for me to set it up.
Afterward, I just did everything manually and changed every line of code (except some of en-GB.json)
Additional Context
Since the Resoltuion (such as Standard, Full HD, ...) aren't translated in the source selector inside the ScenePlayer, I didn't translate them, but would you prefer to have a translation for them in the settings ?
If any test needs to be added, let me know I'll check how to add them
And Thank You for taking the time to read this 😄