Adding the ability for CapLevelController to be enabled/disabled dynamically #1986
Conversation
darrennolan
force-pushed
the
feature/dynamic-capping
branch
from
0140cd0
to
7c23614
Compare
|
Feel I might have to allow some more of the normal manifest startup capturing to happen as currently we seem to be getting too small of bitrates. I assume this is when on manifestParsed we capture levels and things, but only when the config option has been enabled. ^ Scratch the above. Going over the code again, I can see all the things are collected, even if we don't _startCapping(). Which is what I suspected might have been missed. But this PR is still looking okay. I just need to work out on my contentScaleFactor |
|
@johnBartos - sounds good to me. I'll get on that shortly, please don't wait for us. We're happy to go in the next release. |
|
@darrennolan Thanks, that helps a lot! Happy to schedule for 0.11.2 |
* capLevelToPlayerSize now calls start and stop capping accordingly from hls class. * Removed get against cap level as config can be checked for this state.
|
Now for some tests.... |
…rom the hls object itself.
|
@johnBartos - How's this? One thing I wasn't overly happy with myself, was putting the manually created CapLevelController back onto the HLS object, as that's the link needed for the new pub apis. What do you think? Edit: Also any further tests you think it needs, let me know. |
|
Will take a look today 👍
…On Mon, Jan 7, 2019 at 01:58 Darren Nolan ***@***.***> wrote:
@johnBartos <https://github.com/johnBartos> - How's this?
One thing I wasn't overly happy with myself, was putting the manually
created CapLevelController back onto the HLS object, as that's the link
needed for the new pub apis.
What do you think?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1986 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEzwZBvUhzZ2LUN1kWraaHimnrfhdeu2ks5vAvAYgaJpZM4YNL-g>
.
|
I think this is fine, since it's part of the established pattern with the hls instance |
* Removed unnecessary !! for setting config on update. * Reworded capLevelToPlayerSize tests to suit mocha assertions. * Added test to ensure stop capping called when cap level controller destroyed.
|
Should have all those things fixed up @johnBartos |
…apping * Resolving API documentation updates from master.
…level-controller test.
|
Just resolving new merge conflicts, keeping this little fella ready at a moment's notice..... :) |
|
@darrennolan Once the conflict is resolved I'll give it a merge 👍 |
This PR will...
Enable hls.js to toggle capping bitrates to media dimensions dynamically.
Why is this Pull Request needed?
Currently,
hls.config.capLevelToPlayerSizeis only checked when booting a new source. This allows the start and stop of level capping for the video element's size dynamically.Use case for us is switching two video positions around (main video and PiP) - where the PiP video downloading at full 1080 and competing for bandwidth against the main video is not ideal. When the videos are switched (PiP becomes the main video) - we'll release the cap, and start capping the previous main video.
Are there any points in the code the reviewer needs to double check?
I would love some advice on what docblock should read or state. I did try to take some of the formatting from other get/set blocks, but, totally unsure.
Additionally in the main hls constructor where I've capped the capLevelController to the class, not sure if that's the best way to do this, or if I should be looking to the
coreComponentsarray.Resolves issues:
Ours internally, but no listed issue that I could find. It is a bit of a continuation of #1541
Checklist
^ Let me update the API docs in API.md now.