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

Low power mode causes story videos to not play #12786

Closed
propbuster opened this issue Jan 10, 2018 · 11 comments
Closed

Low power mode causes story videos to not play #12786

propbuster opened this issue Jan 10, 2018 · 11 comments

Comments

@propbuster
Copy link

With Low Power mode enabled (on iPhone6s) video does not play correctly. it either doesn't play at all, or it seems to play it really fast on tap the next page, before the next page is rendered.

@newmuis

@newmuis newmuis changed the title STAMP video does not play correctly on in LOW POWER mode on iOS amp-story video does not play correctly on in LOW POWER mode on iOS Jan 11, 2018
@newmuis newmuis added this to Developer Preview (Available) in Stories - By Type via automation Jan 11, 2018
@newmuis newmuis added this to the Active Non-Sprint Tracked milestone Jan 11, 2018
@newmuis
Copy link
Contributor

newmuis commented Jan 11, 2018

My first guess is that the videos that don't play are because might be throwing MEDIA_ERR_DECODE. If they do, then MediaPool (#12604) is a step in the right direction for fixing this, as we can detect this when filling the pool and just lower the cap. iOS already limits to 16 videos even in the regular (non-low-power) case, so it would make sense to me to limit to fewer than that in the low battery case.

This is all speculation; I'll debug and see if I can find out what's happening.

@newmuis newmuis self-assigned this Jan 11, 2018
@newmuis newmuis moved this from Developer Preview (Available) to Developer Preview (Pending) in Stories - By Type Jan 11, 2018
@newmuis
Copy link
Contributor

newmuis commented Jan 11, 2018

Nope, my guess was off target. Turns out iOS 11+ just doesn't autoplay videos in low power mode, and there's nothing you can do about it. Android Chrome doesn't autoplay videos served from http hosts.

After #12604 goes in, we should change it to also bless all media in the MediaPool on navigation (currently, we only bless on unmute)

@newmuis
Copy link
Contributor

newmuis commented Jan 17, 2018

We could not reproduce the fast video playback. I'm not sure if there's anything we could do about it anyway, especially if it's a decoding issue.

I will leave this open to track blessing media on navigation.

@newmuis newmuis changed the title amp-story video does not play correctly on in LOW POWER mode on iOS MediaPool media should be blessed on amp-story navigation Jan 17, 2018
@newmuis newmuis moved this from Developer Preview (Pending) to Developer Preview (Available) in Stories - By Type Jan 17, 2018
@newmuis newmuis moved this from Developer Preview (Available) to Developer Preview (Pending) in Stories - By Type Jan 18, 2018
Stories - By Type automation moved this from Developer Preview (Pending) to Done Jan 18, 2018
@newmuis newmuis changed the title MediaPool media should be blessed on amp-story navigation Low power mode causes story videos to not play Nov 20, 2018
@newmuis newmuis reopened this Nov 20, 2018
Stories - By Type automation moved this from Done to Incoming (Untriaged) Nov 20, 2018
@newmuis
Copy link
Contributor

newmuis commented Nov 20, 2018

There are two problems we're seeing currently:

  1. Media on the first page the user is on does not play.
    There is nothing that we can do about this directly. When the device is in low-power mode, media playback requires a user gesture. We will need to introduce a new user experience that explicitly asks the user to perform a user gesture in the case that we detect that playback has failed.

  2. Media on subsequent pages do not play.
    We currently only bless media on unmute; we removed blessing on navigation for performance. @gmajoulet now that bless performs better and we also have the ability to defer expensive tasks until after the navigation has taken place, do you think we could bless in a low-priority task at the end of switchTo_?

@gmajoulet
Copy link
Contributor

gmajoulet commented Nov 20, 2018

Edit: please disregard this comment. Our bless operation was not working because it was not running in the same frame as a user interaction.

I've been playing with this today, and even if blessAll performance as a low-priority task at the end of the page navigation code is fine, it doesn't fix the bug...
Our current bless operation unmutes and mutes back the media elements.
I updated it to also play and then pause back the media, hoping it'd work as a "play bless", but... the video still doesn't play after a auto-advance :(

I couldn't find any way to autoplay a video without a user interaction while on iOS low energy mode.

I uploaded my test demo here

To summarize:

  • Videos won't play on the first page
  • Videos can play if user taps to navigate to the next page
  • Videos won't play if auto-advance

The good thing is that the play promise is rejected with a NotAllowedError, allowing us to display a message to play the video, like @newmuis just suggested in (1).
However, I believe we should think about making the experience similar for all the pages with video(s): if we know some pages won't play, then maybe we should consider requiring that user interaction on all the pages, even if navigation was triggered by a user tap gesture.

@newmuis
Copy link
Contributor

newmuis commented Nov 20, 2018

@gmajoulet, out of curiosity, does the bless with a play() call fix the bug if done in the first step of the navigation?

@gmajoulet
Copy link
Contributor

Wow, you're right, moving the bless in the first step of the navigation does work. Even with the BlessTask we have right now, without the play().

@newmuis
Copy link
Contributor

newmuis commented Nov 20, 2018

Okay. My theory is that the fact that the latter steps of switchTo_ are deferred is throwing it off, i.e. the browser doesn't think that code is being executed in response to a user gesture.

@newmuis newmuis moved this from Incoming (Untriaged) to Media Playback in Stories - By Type Nov 21, 2018
@newmuis
Copy link
Contributor

newmuis commented Nov 21, 2018

@gmajoulet found that putting the bless in the critical path is too slow, so we can try to attempt this by trying to resolve point (1) from my comment above, ensuring that we bless all videos in the pool.

We need to verify that blessing with a play() call will actually allow autoplay videos on auto-advance pages.

@gmajoulet
Copy link
Contributor

gmajoulet commented Nov 21, 2018

Blessing with mediaEl.muted = true; mediaEl.muted = false is actually enough to autoplay videos on auto advance in this context.
(please disregard my previous message, I'll edit it to make this thread less confusing)

@newmuis
Copy link
Contributor

newmuis commented Nov 29, 2018

This also applies to Google Chrome's Data Saver mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

3 participants