sampotts / plyr

A simple HTML5, YouTube and Vimeo player
https://plyr.io
MIT License
26.52k stars 2.93k forks source link

YouTube autoplay broken in >= 3.3.8 #1185

Closed jryom closed 5 years ago

jryom commented 6 years ago

Expected behaviour

Plyr should autoplay when the correct options are passed to the constructor.

Actual behaviour

Video loads, but doesn't autoplay.

Steps to reproduce

Using the Codepen template, pass autoplay=true, muted=true, and volume=0 (links below)

Environment

Console errors (if any)

Link to where the bug is happening

Not working: https://codepen.io/anon/pen/KxZOqM (latest) Not working: https://codepen.io/anon/pen/GXyVmL (v 3.3.8) Working: https://codepen.io/anon/pen/aaEeWE (v 3.3.7)

friday commented 6 years ago

This was likely caused by #959. I'm not very involved any more, but if someone wants to have a look at it, the issue was that YouTube and Vimeo autoplays the first time you seek, and I had to add a workaround to make this consistent with HTML5.

HTML5 autoplay shouldn't be affected.

turshija commented 6 years ago

This is also an issue for us too. Downgrading to 3.3.7 doesn't work because it has some visual bugs with controls etc. Is there any workaround for this ?

ShaunLWM commented 5 years ago

@turshija The only workaround I can think of (and using right now) is using the .play method.

player.on('ready', () => {
  player.play();
})
dacostafilipe commented 5 years ago

We have the same issue.

@ShaunLWM 's solution works in IE11/Firefox and Chrome, but does not in Safari Mac and on iOS it makes the player not work at all. Not usable for us at the moment.

HectorLS commented 5 years ago

Same situation here, seems to be a Webkit stupid approach

So... what can we do about this, i have a ton of production websites that cannot work as expected anymore because of this 😭

friday commented 5 years ago

HTML5 works: https://codepen.io/anon/pen/gZKZbo, so anyone who finds this issue critical can self-host the video (or try to fix the issue).

Not going to get a ton of upvotes for this comment I suspect :smiley:

dacostafilipe commented 5 years ago

I've done some debugging, and it seems that the issue is that the media is played before it's ready:

screenshot 2019-01-08 at 15 20 20

If I add the following to the onReady method of youtube.js, it seems to solve the issue:

if(player.config.autoplay === true ) {
    player.media.play();
}
friday commented 5 years ago

That's probably a red herring. "Start autoplay!" isn't logged by Plyr or YouTube, so I'm guessing you added it in your fork or event listener? The order of the events logged doesn't mean they happen strictly in that order anyway.

If you send the autoplay param to YouTube it should set the HTML5 autoplay param accordingly. Once that param is enabled and the browser supports it, autoplay will start when it has enough data. It won't try until then. It's all handled by your browser with no JS involved.

As your earlier comment suggests, triggering .play() outside of user interaction events is not as well supported as the autoplay param, so I doubt YouTube does that.

This is also why it's not optimal to fix this way, even if it mostly works.

I would instead try to figure out which of the commits broke this (for examle f8c89e3e95cb01a621f59d66c60f0fa2d76c4d58), and try to work it from there. Perhaps unmute is triggered too soon or something.

Nice try anyway. Might be better than no fix at all for users, although from a maintenance perspective quickfixes that doesn't follow the original intention may not be better than no fix at all.

dacostafilipe commented 5 years ago

Just had a look at the seeking code in youtube.js

At line L231 you define paused as true. So, in L393 you will always pause the video.

Even in the case where autoplay was added to the youtube iframe, you will still pause.

We could change L231 to something like:

player.media.paused = !player.config.autoplay;
friday commented 5 years ago

I think you're right that L393 is the problem. L231 is just the initial setter before there's an event. It should be kept. paused has to be the current state, not the next state, and updated via the events and assurePlaybackState() as now. player.config.autoplay just means you want it to autoplay. It can't be trusted to happen.

L393 is meant for stopping YouTube's automatic play when seeking, not autoplay generally, but it looks like it does both.

Maybe try this:

if (player.media.paused && !player.embed.hasPlayed && !player.config.autoplay) {
dacostafilipe commented 5 years ago

Just tested it on chrome and it works.

Will do more testing on multiple browsers and create a PR for you.

dacostafilipe commented 5 years ago

With your solution, the poster flickers when autoplay is enabled: https://www.youtube.com/watch?v=hTW1Ntb6cCY

If paused is set accordingly to the autoplay configuration, there's no flicker: https://www.youtube.com/watch?v=rTiPxhzxX6E

I understand why you want to have a "clean" state at the start, but if it's not the expected state, I would maybe rethink that idea.

friday commented 5 years ago

I've actually stepped down from Plyr development, but I still answer questions. @sampotts is the one who should decide the direction here.

If autoplay fails (which is something to expect imo) with your solution, you'd end up with no play buttons though, since there is no event for autoplay failing (can't listen to YouTube's exceptions either).

The poster can be hidden with css at least, or perhaps a different fix could take care of this. For example adding a css class for autoplay that disables the poster.

dacostafilipe commented 5 years ago

I understand.

IMO, adding CSS classes for that case does not sound clean.

Anyway, both solutions are "okay" with me, at least the autoplay works. Let's wait for @sampotts feedback and I'll create a PR if needed.

abc983ft commented 5 years ago

@dacostafilipe

Did you end up creating a PR with the fix for this? If so, can you please post the link?

Thanks!

dacostafilipe commented 5 years ago

@tttmack123 Was waiting for feedback from @sampotts, so I did not.

I could make one with my first solution if you are interested in testing.

abc983ft commented 5 years ago

@dacostafilipe, would greatly appreciate it if you could make a release. I have iOS devices to test on (iPad and iPhone) but do not have a Mac. LamdaTest.com has a free plan and I might be able to test on MacOS Safari using it.

dacostafilipe commented 5 years ago

I have the two solutions in my fork.

The solution @friday suggested (see issue above): https://github.com/dacostafilipe/plyr/tree/fix-yt-autoplay

My solution that changes the starting state: https://github.com/dacostafilipe/plyr/tree/fix-yt-autoplay-v2

If you can confirm that the "v2" works, I would open a PR.

abc983ft commented 5 years ago

@dacostafilipe, thanks for sharing your forks.

I created two new codepens from modifying the (latest) codepen in the op. fix-yt-autoplay: https://codepen.io/tttmack123/pen/WmMqzJ fix-yt-autoplay-v2: https://codepen.io/tttmack123/pen/moXZBR

For Windows, both of these autoplay on Chrome, FireFox, Edge and Opera. MacOS was not tested. Android 7.1.1 (Chrome) and iOS 12.1.4 (Chrome and Safari): neither of these autoplay (but neither did v3.3.7 codepen in the OP)

Additionally, the display of the controls in fix-yt-autoplay-v2 have some issues. When it doesn't autoplay (iOS and Android), the button control bar is missing and the large play button is overlapped with the large built in YouTube play button. fix-yt-autoplay did not have this problem with displaying the controls. See these screenshots to compare the ways the controls are displayed on my iPad: https://imgur.com/a/AfgXPVF

sampotts commented 5 years ago

This should be resolved. In my testing I found it was being paused right away on autoplay. I added a check for the autoplay and it seemed to fix it... https://github.com/sampotts/plyr/commit/2eccf0dd05f1678749a2c10c84e5c2bff03f1435#commitcomment-33155584

Let me know if there's any issues 👍

Ivanca commented 4 years ago

After reading out the code this plugin actually deletes the iframe and creates it again when you change the "sources" property to point to a different youtube video... which is incredibly stupid because it could just use the youtube API that is already using to change the video ID, so of course is not gonna auto-play because youtube considers this a "new" player created when the tab is out of focus. I saw the plugin looking so professional just to stumble upon this clusterfuck.

sampotts commented 4 years ago

After reading out the code this plugin actually deletes the iframe and creates it again when you change the "sources" property to point to a different youtube video... which is incredibly stupid because it could just use the youtube API that is already using to change the video ID, so of course is not gonna auto-play because youtube considers this a "new" player created when the tab is out of focus. I saw the plugin looking so professional just to stumble upon this clusterfuck.

Given it's so "incredibly stupid", feel free to use your obviously superior intellect to put some effort in and open a PR to fix it? I really apologise for the "clusterfuck". For free software that must have been terrible for you to experience but thanks for the kind words.

sampotts commented 4 years ago

Also, I'm not alone in thinking this but autoplay videos are annoying. Hence browsers blocking it anyway.

Ivanca commented 4 years ago

Google Chrome uses an 'smart' system to decide if a domain is allowed auto-play or not (at chrome://media-engagement/) which works fine and has nothing to do with the issue at hand, which is that youtube doesn't let background tabs to autoplay on newly created players, which is necessary for any sort of playlist music players.

Anyway, I found a workaround and works fine which is to use the api directly exposed on youtube.embed as in player.embed.loadVideoById('dQw4w9WgXcQ')

I'm not gonna do a PR but if someone wants to do it the change needed on plyr is just to check if the already running on youtube and if so call loadVideoById instead of running all the existing code in source.change, plyr__poster would need to be updated as well (or opacity:0 hidden at least)

sampotts commented 4 years ago

Google Chrome uses an 'smart' system to decide if a domain is allowed auto-play or not (at chrome://media-engagement/) which works fine and has nothing to do with the issue at hand, which is that youtube doesn't let background tabs to autoplay on newly created players, which is necessary for any sort of playlist music players.

Google Chrome is not the only browser out there. Just for reference.

I'm not gonna do a PR

I was expecting a reply like that. 👍

friday commented 4 years ago

In addition YouTube isn't the only provider. At least back when I was involved Plyr supported replacing a video with another video from a different provider (for example changing YouTube to Vimeo, to HTML5, to HLS and back). If it wasn't for the "incredibly stupid" solution this would not be possible.

Ivanca commented 4 years ago

@friday The proper solution would be to have a player for each source (one for youtube, one for vimeo, etc) and hide them as needed, so that if the user is play-listing HTML5 videos only or Youtube videos only (the 2 most common playlisting use cases by far) neither need to be destroyed (or created more than once), this is specially critical on low-end mobile devices where too many DOM changes can make the device unresponsive.

sampotts commented 4 years ago

That's a terrible solution performance wise. Loading the different players and SDKs when not needed would be much worse performance. This is why the trend is to lazy load players. You're also the first person to bring this up by the way. Your use-case isn't everyone's use-case. I am well aware there's improvements to be made but you have to remember this is an open source project and 99% of the time it's just me doing the bulk of the work and have to prioritize what gets done.

Ivanca commented 4 years ago

Obviously you wouldn't spawn it first until it is needed, so if the user only needs the youtube one that would be the only one created; didn't think I would have to clarify such obvious details.

friday commented 4 years ago

Your rude and ignorant comments makes it clear that you don't know how browsers deal with auto play then, in addition to not knowing how human beings communicate or how free software works.

carlosfelipe1988 commented 4 years ago

Just had a look at the seeking code in youtube.js

At line L231 you define paused as true. So, in L393 you will always pause the video.

Even in the case where autoplay was added to the youtube iframe, you will still pause.

We could change L231 to something like:

player.media.paused = !player.config.autoplay;

funciona muy bien

Vadim-Podlevsky commented 2 years ago

@dacostafilipe, thanks for sharing your forks.

I created two new codepens from modifying the (latest) codepen in the op. fix-yt-autoplay: https://codepen.io/tttmack123/pen/WmMqzJ fix-yt-autoplay-v2: https://codepen.io/tttmack123/pen/moXZBR

For Windows, both of these autoplay on Chrome, FireFox, Edge and Opera. MacOS was not tested. Android 7.1.1 (Chrome) and iOS 12.1.4 (Chrome and Safari): neither of these autoplay (but neither did v3.3.7 codepen in the OP)

Additionally, the display of the controls in fix-yt-autoplay-v2 have some issues. When it doesn't autoplay (iOS and Android), the button control bar is missing and the large play button is overlapped with the large built in YouTube play button. fix-yt-autoplay did not have this problem with displaying the controls. See these screenshots to compare the ways the controls are displayed on my iPad: https://imgur.com/a/AfgXPVF

Your code from workpen solved this issue for me. Thank you so much!