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

Implemented advanced settings screen (buffering and HTTP settings) #928

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

Conversation

bitlinker
Copy link

Hi! Please take a look at this implementation of additional network settings screen. It allows to configure ExoPlayer's streaming buffers size and to add aditional parameters to HTTP requests: User-Agent and other header values (some streaming hosts require specific values)

Also please note the fixes at PlayerViewModel:46 and PlayerViewModel:52. The settings repo is "preheated" here, but it seems not enough: When the player is created PlayerActivity.kt:288, the settings are not available yet and default ones are used! I know that runBlocking() is ugly, but can't find better solution here. It shouldn't actually block the thread since the preferences flow should already have value available.

@anilbeesetti
Copy link
Owner

Hey @bitlinker can you fix linting by running the following cmd in terminal ./gradlew ktlintFormat

@bitlinker
Copy link
Author

Hey @bitlinker can you fix linting by running the following cmd in terminal ./gradlew ktlintFormat

Done

@anilbeesetti
Copy link
Owner

Hey @bitlinker, thanks for the pull request. I couldn't play local videos now. I believe it's because we're setting DefaultHttpDataSource as the default data source for ExoPlayer. We should only set the DefaultHttpDataSource if the URI is a network URI. And I think we should rename network settings screen to advanced settings screen, What do you think?

@bitlinker
Copy link
Author

bitlinker commented May 1, 2024

Hey @bitlinker, thanks for the pull request. I couldn't play local videos now. I believe it's because we're setting DefaultHttpDataSource as the default data source for ExoPlayer. We should only set the DefaultHttpDataSource if the URI is a network URI. And I think we should rename network settings screen to advanced settings screen, What do you think?

Hey @anilbeesetti, sorry I didn't checked the local videos last time. Yes, the issue was with the data source configuration - the DefaultDataSource.Factory is responsible for selecting required data sources based on URI scheme, but DefaultHttpDataSource.Factory was used instead. It is working now for both local and network streams.

I agree, advanced settings is more suitable name here, since buffer configurations are applied to local videos too. Renamed it.

@bitlinker
Copy link
Author

Fixed ktlint =)

@bitlinker bitlinker changed the title Implemented network settings screen Implemented advanced settings screen (buffering and HTTP settings) May 1, 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.

2 participants