Apply the IOS headphone disconnect patch only after successful playback #6354
Conversation
|
💖 Thanks for opening this pull request! 💖 Things that will help get your PR across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
There was a problem hiding this comment.
Another small comment but this looks good to me. I'd have to test it. Also, I'm looking into #6330 which seems to stem from the original implementation of this but if things need to change I'll try to base it off of this PR assuming things look good.
|
also, I wonder if the issue is also just #6330, though, the changes here are good. |
There was a problem hiding this comment.
@gkatsev I strongly believe that #6330 describes a similar problem. The only thing that concerns me though, is, that stalled / suspend could happen within a timeframe of 14s, as reported in #6330. Maybe the iOS headphone disconnect is in general too intrusive and not robust enough yet.
In case this pull doesn't solve #6330 I would suggest to disable this iOS headphone disconnect check and introduce a new config option where this can be enabled. Or alternatively finding a better way how this disconnect could be identified in a more robust fashion. What do you think?
|
Yeah, I'm looking into that one. It's definitely possible that we may want to revert that change. Though, I'm thinking that maybe the change we may want to pause if we don't continue playback. Unfortunately, locally, when unplugging headphones I'm not getting any events but I see that Video.js is in a playing state. |
|
@gkatsev reverting would also fix my initial problem. Feel free to close my pull in case you revert the previous change. |
|
yeah, I would recommend sticking with Video.js 7.6.6 for now. If we end up keeping the change I'm definitely going to merge this in because it's a nice improvement but it looks more and more like we may want to revert that change. |
|
I think we're going to revert the change and be done with it. Really appreciate the PR, though. Thanks! |
|
7.7.4 is released with the reverted change. |
Description
Since using the version which includes #6199 we saw issues when using videojs in combination with videojs+ima. Because Google IMA uses the same video-tag on iOS to display video ads it happens that a
suspendevent is occurring when video sources get switched (in my case a preroll in a muted autoplay scenario). And callingpause()in this switching phase could lead to a state within IMA that the player gets stalled.A fix could either be to enable / disable this headphone-disconnect patch or this pull request, which only does stalled/suspend checks after a video-source was played for at least 1s.
Requirements Checklist