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

Bandwidth rate restriction cannot be applied after playback started. #1533

Closed
SemihGk opened this issue Aug 9, 2018 · 8 comments
Closed
Assignees
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@SemihGk
Copy link
Contributor

SemihGk commented Aug 9, 2018

Have you read the FAQ and checked for duplicate open issues?:
Yes
What version of Shaka Player are you using?:
latest master or 2.4.3
Can you reproduce the issue with our latest release version?:
yes
Can you reproduce the issue with the latest code from master?:
yes
Are you using the demo app or your own custom app?:
demo app
If custom app, can you reproduce the issue using our demo app?:

What browser and OS are you using?:
chrome and macOS
What are the manifest and license server URIs?:
Does not matter. You may check tears of steel(widevine) sample.

What did you do?
I did set max bandwidth rate after playback started. However, the restrictions cannot be applied to abrManager even though restrictions applied to configuration object.

For instance, call before playback.
shakaDemo.localPlayer_.configure({ restrictions: { maxBandwidth: 500000 } });

Then play the content and call:
shakaDemo.localPlayer_.configure({ restrictions: { maxBandwidth: Infinity } });

What did you expect to happen?
The restriction should be applied to abrManager.

What actually happened?
The restriction could not be applied and abrManager only played variants by restricted of the previous value.

Note: The issue is fixed if I add this change at this block

    if (!activeVariant || !activeVariant.allowedByApplication ||
        !activeVariant.allowedByKeySystem) {
      shaka.log.debug('Choosing new streams after changing configuration');
      this.chooseStreamsAndSwitch_(period);
    } else {
      // If we change the configuration during the playback, the changes cannot
      // be applied due to abrManager is not updated. So, we force to update it.
      let variants = shaka.util.StreamUtils.filterVariantsByConfig(
          period.variants, this.currentAudioLanguage_, this.currentVariantRole_,
          this.currentAudioChannelCount_);
      this.abrManager_.setVariants(variants);
    }

I don't mind to create a PR if this change is appropriate for this fix. Thank you.

@theodab theodab added type: enhancement New feature or request and removed needs triage labels Aug 9, 2018
@shaka-bot shaka-bot added this to the Backlog milestone Aug 9, 2018
@theodab
Copy link
Collaborator

theodab commented Aug 11, 2018

"Hard" restrictions (the type you are setting with those example configuration calls) are applied at the following times:

  1. when adding a new period
  2. when first loading a manifest
  3. when the manifest is updated (like in a livestream)

They are only use by the player, and not by the AbrManager. AbrManager has its own separate set of "soft" restrictions (located in the configs as abr.restrictions).

They aren't, as far as I can tell, applied by default when configuration changes. If I am reading player.applyConfig_ correctly, we do apply "soft" restrictions at that time, though.
Manually applying "hard" restrictions on config would be possible, I think. It's possible we aren't doing that for a reason? I'm not sure.

On another note, I'm not sure why the sample code you provided helps with the problem? shaka.utils.StreamUtils. filterVariantsByConfig doesn't apply the "hard" restrictions, it just locates the variants with the given language, role, and audio channel count.

@SemihGk
Copy link
Contributor Author

SemihGk commented Aug 13, 2018

Thank you for providing very detailed feedback. I really appreciated.

To be honest, I did not really take too much attention on abr.restrictions since I was mainly using 'hard' restrictions to configure the player. However, it sounds a bit code redundancy for me. If the abrManager already restricts the variants, I am curious why do you keep 'hard' restrictions?

First I thought it may be a reason that shaka player cannot revert restrictions due to the the periods are already filtered. However, this should not be the case. Otherwise, my workaround would not have worked. Then, I checked some comments. Based on this explanation and this one , I may see that you do not want to interrupt playback even though the restrictions filtered out all variants. For this case, shaka player can still filtered out all restricted variants and keep one variants which is playable. Or you can only keep one restriction configuration and you may add a 'restrictionType: 'soft' or 'hard' . However, I can see this solution's disadvantage which it may break users configurations when they upgrade the shaka player. So, it is not desirable.

Anyway, I still think 'hard' restrictions should be applied during the playback. However, it is not a big deal since I am able to apply 'soft' restrictions, but I believe this might be confusing for other users in the future.

As for my solution, I thought some variables which arethis.currentAudioLanguage_, this.currentVariantRole_, this.currentAudioChannelCount_ might have changed during the playback. However, I noticed that this.abrManager_.setVariants(variants) api is called whenever those variables are changed. So, you are right. I do not need to call filterVariantsByConfig . Calling this.abrManager_.setVariants(period.variants) would be efficent since 'hard' restrictions are appled at this line for a possible fix if you consider.

Thank you.

@theodab
Copy link
Collaborator

theodab commented Aug 13, 2018

For more information on restrictions, you can see the docs for player configuration. The short version is that the "soft" restrictions aren't as absolute; for instance, if you apply a "soft" restriction to bandwidth that is below the bandwidth of every variant, the abr manager will just default to picking the lowest-bandwidth variant, even though it's technically restricted. If that was a "hard" restriction, on the other hand, the player would be completely unable to pick a stream, and would error. Sometimes that behavior is desirable, sometimes it's not, so we retain both types of restrictions.

Maybe I should add an FAQ entry about how restrictions work? It's a pretty confusing distinction.

One interesting thing to note is that the allowedByApplication flag is checked in player.applyConfig_, but that flag is only set by StreamUtils.applyRestrictions, in the situations I mentioned earlier. I'm not sure how if we can ever end up in a situation where that check is meaningful, if we avoid playing streams with allowedByApplication set to false and don't change the flag in player.applyConfig_. That might suggest that we are supposed to be applying "hard" restrictions in player.applyConfig_, and us not doing so is a bug. I'll want to look into that more.

@chrisfillmore
Copy link
Contributor

I'm also curious about the purpose of soft restrictions in ed0f37b . Why allow ABR to pick a variant if all are restricted?

@ismena
Copy link
Contributor

ismena commented Aug 21, 2018

@chrisfillmore We should, probably, rename them to preferences or something.
@joeyparrish, please correct me, if I'm wrong (I was not actively involved with this change), but, to the best my knowledge, the idea is that:

  • top-level restrictions mean "we can/shall not play the variants that don't comply with the restrictrictions, treat them like they don't exist"
  • abr restrictions (in the new world, the ones that I think should be called preferences) mean "we can/shall play these, but we'd rather not. If there is any other variant we can play instead, let's do it; if not, so be it".

The data savers thing references in the commit msg is basically "if the data saving mode is on, make every attempt to play lower bandwidth video before going for higher res and wasting precious data".

@joeyparrish
Copy link
Member

It used to be the case (I think) that restrictions would be reapplied when the config changed. So this may be a regression.

Also, to clarify an earlier discussion, "soft" ABR restrictions only apply to ABR logic. Tracks restricted at that level can still be selected manually via selectVariantTrack(), so it is not redundant with "hard" restrictions. The top-level "hard" restrictions are meant to communicate something that should not be played, and have a primary purpose of communicating out-of-band DRM restrictions. For example, on IE11, where key statuses are not available, restrictions and out-of-band DRM hints are the only way to avoid playing tracks that can't be decrypted with a given license.

@joeyparrish joeyparrish added type: bug Something isn't working correctly and removed type: enhancement New feature or request labels Sep 6, 2018
@joeyparrish joeyparrish modified the milestones: Backlog, v2.5 Sep 6, 2018
@TheModMaker TheModMaker self-assigned this Sep 10, 2018
@joeyparrish
Copy link
Member

I attempted to cherry-pick this bug fix to v2.4.5, but the merge conflicts were too complex to fix up. So this bug fix will not appear in a release until v2.5.0-beta2. We apologize for the inconvenience.

@shaka-project shaka-project locked and limited conversation to collaborators Nov 9, 2018
@joeyparrish
Copy link
Member

Released in v2.5.0-beta2.

@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

7 participants