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

Add switch to disable progressive support #4564

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

osagie98
Copy link
Contributor

@osagie98 osagie98 commented Dec 11, 2024

  1. Adds a runtime switch "--disable_progressive_playback" that will prevent progressive media playback. When set, Cobalt will reject CanPlayType() queries for progressive formats, or will report an error before a ProgressiveDemuxer is constructed.
  2. Adds an H5vcc setting "DisableProgressivePlayback". The web app can call window.h5vcc.settings.set("Media.DisableProgressivePlayback", true) to disable progressive playback, and window.h5vcc.settings.set("Media.DisableProgressivePlayback", false) to reenable it.
  3. Rearranges methods in CanPlayTypeHandlerStarboard to match the declaration order in CanPlayTypeHandler.

b/382791540

1. Adds a runtime switch "--disable_progressive_playback" that will
   prevent progressive media playback. When set, Cobalt will reject
   CanPlayType() queries for progressive formats, or will report an
   error before a ProgressiveDemuxer is constructed.
2. Rearranges methods in `CanPlayTypeHandlerStarboard` to match the
   declaration order in `CanPlayTypeHandler`.

   b/382791540
@jasonzhangxx
Copy link
Contributor

My understanding of the "runtime switch" is to enable/disable progressive support via h5vcc, but the pr uses command line switch. @xiaomings to correct me if that's not correct.

@borongc
Copy link
Contributor

borongc commented Dec 12, 2024

We may consider to use h5vcc switch --> base_feature --> enable/disable media feature, as base_feature is the way Chromium uses to enable/disable feature. (cc @yell0wd0g)

We should define how we are going to add switch for Cobalt.

@osagie98
Copy link
Contributor Author

I can add an H5vcc switch to this PR

@osagie98
Copy link
Contributor Author

osagie98 commented Dec 12, 2024

The H5vcc switch is in. @borongc are you suggesting we use BASE_FEATURE for 25.lts?

@borongc
Copy link
Contributor

borongc commented Dec 12, 2024

I got confused with this PR #4547.

For this PR for C25, we should use h5vcc flag for this.

@xiaomings
Copy link
Contributor

Maybe we don't need this switch in C26, given that we are going to experiment on C25 first. We can keep progressive support in C26 (without the disable switch), and remove the support completely when the C25 metrics is in?

@osagie98
Copy link
Contributor Author

That should be fine - the C26 change is small and can be easily merged anytime if we need to

@osagie98
Copy link
Contributor Author

cc @yell0wd0g shall we add the progressive switch in C25 first in case we decide to remove support entirely in C26?

@osagie98 osagie98 requested a review from kaidokert December 17, 2024 21:49
@osagie98 osagie98 merged commit a082466 into youtube:25.lts.1+ Dec 18, 2024
300 of 301 checks passed
@osagie98 osagie98 deleted the add-progressive-switch branch December 18, 2024 23:19
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.

5 participants