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

🔥 [Story videocache] Apply hasAudio flag behind experiment #38285

Merged
merged 4 commits into from
Jun 9, 2022

Conversation

mszylkowski
Copy link
Contributor

@mszylkowski mszylkowski commented Jun 9, 2022

Currently many stories are being muted by the hasAudio flag that the video cache returns, which is not accurate. We want to disable this feature and only enable it through a client-side experiment when the backend is fixed and the correct value is returned. This mutes videos on origin and cache (and there's no way to unmute them since the unmute icon is missing from the page).

The experiment story-video-cache-apply-audio when enabled will apply the audio flag that it receives from the cache.

@amp-owners-bot amp-owners-bot bot requested a review from alanorozco June 9, 2022 20:58
@mszylkowski mszylkowski requested review from gmajoulet and removed request for alanorozco June 9, 2022 21:00
@mszylkowski mszylkowski self-assigned this Jun 9, 2022
Copy link
Contributor

@gmajoulet gmajoulet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @danielrozenberg about experiment change

@mszylkowski mszylkowski merged commit ed74a46 into ampproject:main Jun 9, 2022
jridgewell pushed a commit that referenced this pull request Jun 10, 2022
* Apply hasAudio flag behind experiment

* Added spec onto experiment config

* Fixed unit tests

(cherry picked from commit ed74a46)
ampprojectbot pushed a commit that referenced this pull request Jun 10, 2022
* Apply hasAudio flag behind experiment

* Added spec onto experiment config

* Fixed unit tests

(cherry picked from commit ed74a46)
ampprojectbot pushed a commit that referenced this pull request Jun 10, 2022
* Apply hasAudio flag behind experiment

* Added spec onto experiment config

* Fixed unit tests

(cherry picked from commit ed74a46)
ampprojectbot pushed a commit that referenced this pull request Jun 10, 2022
* Apply hasAudio flag behind experiment

* Added spec onto experiment config

* Fixed unit tests

(cherry picked from commit ed74a46)
@mszylkowski mszylkowski deleted the disable_audio_videocache branch June 10, 2022 20:07
ampprojectbot pushed a commit that referenced this pull request Jun 10, 2022
* Apply hasAudio flag behind experiment

* Added spec onto experiment config

* Fixed unit tests

(cherry picked from commit ed74a46)
ampprojectbot pushed a commit that referenced this pull request Jun 10, 2022
* Apply hasAudio flag behind experiment

* Added spec onto experiment config

* Fixed unit tests

(cherry picked from commit ed74a46)
ampprojectbot pushed a commit that referenced this pull request Jun 10, 2022
* Apply hasAudio flag behind experiment

* Added spec onto experiment config

* Fixed unit tests

(cherry picked from commit ed74a46)
ampprojectbot pushed a commit that referenced this pull request Jun 10, 2022
* Apply hasAudio flag behind experiment

* Added spec onto experiment config

* Fixed unit tests

(cherry picked from commit ed74a46)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants