-
Notifications
You must be signed in to change notification settings - Fork 20
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
Implement inputstream.adaptive.max_bandwidth #189
Conversation
d8909df
to
87bc38a
Compare
This does not work as I expected. The settings do not show the pull-down menu as the main settings page. And whatever value inserted does not appear to work. PS It does not look like DRM is working either on my system, live streams are 540p at all times (even before this change). |
Thanks, this can work for |
@mediaminister I am not sure if this is related to DRM only. AFAICT it works for inputstream.adaptive in general. In fact, I think it's even better to always add this property so it also works if somehow Kodi decides to use inputstream.adaptive. |
Attempt to use max_bandwidth for inputstream.adaptive. This will select the lowest value of both the global network max_bandwidth as well as our own max_bandwidth setting.
And I am now also setting network.bandwidth for every stream, and it simply works. So with this PR we successfully limit the bandwidth on all the streams. The only thing not limited is our own HTTP requests, and I am not sure if we really have to. Maybe we ought to provide a drop-down list of values like the global setting. PS It now no longer belongs to the DRM settings tab, so again where do we put this ? I think we have to reduce Credentials, Subtitles, DRM, Advanced into Credentials, Interface, Video and then move Subtitles and DRM to Video. |
For now, it definitely doesn't work for hls streams, because these streams are handled by I'm okay with your suggested changes to the settings tabs. |
@mediaminister I understand now. Our best hope for the future is to have both HLS and DASH be handled by Kodi itself. In the meantime we have to handle bandwidth limitation ourselves as best as we can for HLS. So I'll merge both PRs, and change the settings in a follow-up PR. |
@dagwieers Are you sure this really works, for example when you disable DRM and play a livestream? It doesn't work for me. |
@mediaminister I will test. Using network.bandwidth did work for a scenario where inputstream.adaptive.max_bandwidth did not work though. But it would help if the logs would indicate which scenario I am effectively testing (now I simply try a few things that I assume are different scenarios, but can't know for sure...) For debugging, we may want to force MPEG-DASH or HSL ourselves, maybe ? Maybe a new option in settings ? |
And maybe we should Mock ListItem so we can actually test if we get the expected ListItem entries for each of the scenarios based on settings/bandwidth. |
Disable the DRM user setting and play Eén or Canvas livestream and you get always HLS. The streaming protocol is already displayed in the log. I don't think we need other settings. |
Wait until you see the implementation I have in my head... :-) |
@mediaminister I can confirm that "network.bandwidth" does not work. So it means for streams played by Kodi/HLS, only the global setting takes effect. So probably the best way forward is to remove the bandwidth setting from our plugin, and only take the global setting into account. Unless we specify that this setting only affects true inputstream.adaptive-induced streams. |
I finally found why 720p HD streams were not working. By default InputStream Adaptive has a configuration setting Ignore Display Resolution which is disabled. I guess since my display resolution is 1080p, which could not be matched, it did not select 720p. I updated our documentation, and add a HD quality section. |
Attempt to use max_bandwidth for inputstream.adaptive.
This will select the lowest value of both the global network max_bandwidth as well as our own max_bandwidth setting.
This fixes #172