bandwidth based quality selection #59
Conversation
this enables quality selection in cases where no image size is specified for the HLS variants. it also replaces the utilization of <image width> labels by <width x hight> notation.
|
i forgot to add the archive containing simple test examples to reproduce the issue: |
|
@matvp91 we are rather busy theses days to improvise video-solutions for our friends in the culture scene to enable the realization of remote presentations in the artistic field, which can not be handled otherwise because of the present CoV crisis, and it is/would be very helpful to use indigo for this efforts. although i can utilize my own fork of the player for this short-term activities, i still prefer to cooperate with upstream maintainers and a shared code base used by all interested user... |
| @@ -15,7 +15,10 @@ tabs[SettingsTabs.OPTIONS] = (props: SettingsProps) => ( | |||
| item: SettingsTabs.TRACKS, | |||
| label: props.data.getTranslation('Quality'), | |||
| info: `${ | |||
| props.data.activeTrack ? props.data.activeTrack.width : '' | |||
| props.data.activeTrack ? | |||
There was a problem hiding this comment.
Nested condition shorthands are a bit confusing, could this be better structured?
There was a problem hiding this comment.
as i already mentioned in a earlier reply, i'm not very familiar with TS or JS. if it would be python or rust code, i would definitely choose the most pleasant and idiomatic expression, but on the java script side i'm unfortunately hardly able to keep my head above water. ;)
if you don't mind, i would prefer, that you simply modify the suggested changes to the better.
i know, it's a little bit of annoying work, but at the end it's definitely better to keep the code base clean and in exemplary style, which i'm simply not capable to serve in this particular programming language without spending huge amounts of time to study and learn the most basic expressive possibilities.
| props.data.activeTrack ? | ||
| (props.data.activeTrack.width ? | ||
| (""+props.data.activeTrack.width+"x"+props.data.activeTrack.height+", "):"") | ||
| +(props.data.activeTrack.bandwidth/1e6).toPrecision(2)+"M" : '' |
There was a problem hiding this comment.
Not a big fan of this line, ES6 template literals would be a cleaner approach imo.
| @@ -456,7 +456,7 @@ export class StateStore | |||
| this.props.player.tracks.sort( | |||
| (a, b) => Number(b.height) - Number(a.height), | |||
| ), | |||
| 'height', | |||
| 'bandwidth', | |||
There was a problem hiding this comment.
I believe we can drop the uniqBy filter above, as this would select "bandwidth" to group by.
|
Hey @mash-graz, I've given this some thought and I ran into several conceptual issues that would require a bigger refactor. For this reason, I'd like to postpone this issue if you don't mind until I have the following factors figured out. I'll prioritize this over the weekend. Could you tell me which adaptive streaming format you're using (DASH / HLS)? These are several issues I'll have to deal with:
Making the change for the sake of having it work properly on HLS but not on DASH kinda defeats the purpose of indigo-player's strict API policy. :) Honestly, it seems indigo-player Once I expose a What are you thoughts on this approach? Meanwhile, I'll spec this out a bit on a separate issue. |
that's perfectly fine with me!
that's indeed a good question. right now we are using mostly HLS and x264 for live streaming, because that's the smalest common denominator, which works on all devices and we do not have enough available computing power to realize parallel x265 and VP9 delivery in realtime, too. but for static VOD content i prefer this alternative solution. in this case you simply have to use DASH and CMAF, because HLS still doesn't support VP9 or AV1. in the context of your player it's a rather interesting questions, because it can make sense to choose an alternative delivery format even in cases, where one other option gets prioritized as default source. i would therefore appreciate a manual choice option in this case as well, but i also have to say, that i didn't utilize your tools for this kind kind of delivery in practice until now, but i'm working on a project, where i'll try to test it soon. i'm rather familiar with troubles and fundamental incompatibilities of various video.js plugins, which i used for a long time, but are affected by very similar symptoms. in fact i would even describe your player as much more coherent and trustworthy in this concern. although i'm not able to participate in your actual work by contributing code (o.k -- maybe in the far future i'll perhaps send you some WASM modules written in rust to extend indigo ;) ), i still like your efforts and are always willing to help as beta tester or provide similar trivial support. thanks! |
|
This looks like a nice and useful update, however, not touched for a couple of months now. @mash-graz would you care to merge master to branch? :-) |
standard compliant HLS master playlists may only use the bandwidth field in the description of stream variants, because that's the only required entry, all other fields (e.g. image size) are optional (see: https://developer.apple.com/documentation/http_live_streaming/example_playlists_for_http_live_streaming/creating_a_master_playlist).
the quality selection should therefore mainly based on this this particular entry to work reliable.
this PR also replaces the reported options from the slightly irritating
image widthdisplay to more informativeimage width x height bandwidthentries. this could look overly verbose to some users, but very useful to others...Fixes: #58