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

Added a preferred stream quality option #266

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

e2fo2l
Copy link

@e2fo2l e2fo2l commented Feb 2, 2023

Added the option to select a preferred stream quality in the player settings. This works for both ExoPlayer and mpv.
You can either request the original file, or ask for a specific resolution (4K, 1080p, 720p, 480p, 360p).
If the asked resolution is lower than the original file's, it will require transcoding.

Ideally, the option to switch the quality should have been directly inside the player, however, I haven't found an easy way to set different stream URLs for different resolutions.

Please note that this option is only for video streaming, downloads still use the original file regardless.

@Ge082
Copy link

Ge082 commented Feb 4, 2023

So ... It basically means transcoding is now a thing on Findroid ?

@e2fo2l
Copy link
Author

e2fo2l commented Feb 7, 2023

Changed DropDownPreference to ListPreference to suit the new style, as per @Yash-Garg 's suggestion.

@e2fo2l
Copy link
Author

e2fo2l commented Feb 13, 2023

Fixed merge conflicts for imports, this should be good now

@e2fo2l
Copy link
Author

e2fo2l commented Feb 13, 2023

Still had a few errors left, this now passes the CI build

@WreckingBANG
Copy link

Is there a Test apk to download?

@Yash-Garg
Copy link
Contributor

Is there a Test apk to download?

You can check the artifacts for this PR here.

@WreckingBANG2
Copy link

WreckingBANG2 commented Feb 14, 2023

Does it consume less bandwidth or is it just a local resolution selector?

Will there be a setting to set it to different Values when using WIFI or mobile?

@e2fo2l
Copy link
Author

e2fo2l commented Feb 14, 2023

Does it consume less bandwidth or is it just a local resolution selector?

Will there be a setting to set it to different Values when using WIFI or mobile?

The transcoding is done server side, so that it consumes less bandwith.
Currently, the implementation is very basic, the resolution is selected manually by the user, there's no WiFI/mobile data specific setting.

@jarnedemeulemeester
Copy link
Owner

@e2fo2l I have yet to fully test this but is it normal for the Jellyfin dashboard to say the the file is direct playing while obviously transcoding? (High FFmpeg usage and lower video quality)

@e2fo2l
Copy link
Author

e2fo2l commented Feb 17, 2023

I don't know, the transcodeReason is properly set to "ContainerBitrateExceedsLimit" when requesting the HLS stream, perhaps there is another parameter missing so that jellyfin properly logs the transcoding

Copy link
Contributor

@OpenSourceSimon OpenSourceSimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works perfectly! Although the Jellyfin panel says it doesn't transcode and it's a direct play. And a suggestion: maybe add it per video like YouTube has? Thanks for the PR!

OpenSourceSimon

This comment was marked as duplicate.

@CakesTwix
Copy link
Contributor

Why don't they accept PR? I have a pretty weak tablet that can't play 1080p
Useful feature

@Yash-Garg
Copy link
Contributor

Yash-Garg commented Apr 8, 2023

Why don't they accept PR? I have a pretty weak tablet that can't play 1080p Useful feature

I think he's not getting much time, let's wait for a reply. I'll suggest to use the build that is generated or create one from this PR in the meantime.

@e2fo2l
Copy link
Author

e2fo2l commented Apr 11, 2023

Hello, I haven't had the time to thoroughly troubleshoot the issue with the web panel not showing the stream as being transcoded. To me, that seems like an issue on Jellyfin's end though, because the video IS being transcoded, and should be reported as such.

@OpenSourceSimon Ideally, I would also like the option to have the resolution selector directly in the player, however, I don't have that much experience working with ExoPlayer. This could maybe be implemented in a separate PR.

@jarnedemeulemeester
Copy link
Owner

Sorry for the long wait everyone. I have indeed been pretty busy lately.
I first want to merge #329, this is a big rework of how items are handles in Findroid (and will require some changes to this PR).
I would be happy to work on those changes once I have merged the rework.

I am also more a fan of a resolution selector directly in the player UI but that can indeed be implemented in a separate PR. I am also not sure yet how that would be implemented but I'm sure someone will figure it out 😄

@CommanderRedYT
Copy link
Contributor

Any update?

1 similar comment
@jiajiaxd
Copy link

jiajiaxd commented Jul 1, 2023

Any update?

@e2fo2l
Copy link
Author

e2fo2l commented Jul 3, 2023

Hello,
Sorry for not replying earlier, I haven't had any spare time to work on this. I'll try adapting the code to the new refactor this week, hopefully it shouldn't take too long.

@cesarblancg
Copy link
Contributor

There is any update?

@seang96 seang96 mentioned this pull request Oct 22, 2023
@sjiawjbssj sjiawjbssj mentioned this pull request Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.