Bug 192530
Summary: | [GStreamer] HTMLMediaElement::m_player->paused is always true while video is playing | ||
---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> |
Component: | Media | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED WORKSFORME | ||
Severity: | Normal | CC: | aboya, bugs-noreply, mcatanzaro, pnormand |
Priority: | P2 | ||
Version: | WebKit Nightly Build | ||
Hardware: | PC | ||
OS: | Linux | ||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=219354 |
Michael Catanzaro
I was debugging why SleepDisablerGLib is broken currently. After getting a bit distracted by the inhibit portal being broken (separate bug, not our fault) I realized HTMLMediaElement is never creating SleepDisabler objects in the first place.
Turns out that when playing a YouTube video, HTMLMediaElement::shouldDisableSleep always returns early in this condition here:
if (!m_player || m_player->paused() || loop())
return SleepType::None;
because m_player->paused() is returning true. But this happens when the video is playing, not when it is paused. So yeah that's not good.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Michael Catanzaro
BTW I wonder if this state tracking issue is why YouTube videos often restart improperly when moused over.
Michael Catanzaro
Strangely it's not a recent regression, I tried bisecting this but found this to be the case as far back as the 2.20(!) branchpoint.
Alicia Boya García
Can you confirm if this affects the non-MSE media player?
Michael Catanzaro
I tried to disable the setting in MiniBrowser and noted that paused() was still returning true during playback, but I'm not sure if the setting really took effect or not.
Unfortunately I can't confirm outside my WebKit jhbuild because MSE is now required for YouTube video playback without H.264.
Alicia Boya García
(In reply to Michael Catanzaro from comment #4)
> I tried to disable the setting in MiniBrowser and noted that paused() was
> still returning true during playback, but I'm not sure if the setting really
> took effect or not.
>
> Unfortunately I can't confirm outside my WebKit jhbuild because MSE is now
> required for YouTube video playback without H.264.
What about this one? https://www.w3schools.com/html/html5_video.asp
Michael Catanzaro
(In reply to Alicia Boya García from comment #5)
> What about this one? https://www.w3schools.com/html/html5_video.asp
OK that worked. The bug occurs the same with and without MSE.
P.S. Apparently I've never seen that part of Big Buck Bunny. It was traumatizing. :(
Philippe Normand
I can't reproduce this issue.
diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp
index 88eae188a0e..8d2f2d18379 100644
--- a/Source/WebCore/html/HTMLMediaElement.cpp
+++ b/Source/WebCore/html/HTMLMediaElement.cpp
@@ -6896,6 +6896,7 @@ HTMLMediaElement::SleepType HTMLMediaElement::shouldDisableSleep() const
#if !PLATFORM(COCOA) && !PLATFORM(GTK) && !PLATFORM(WPE)
return SleepType::None;
#endif
+ WTFLogAlways("paused: %d", m_player->paused());
if (!m_player || m_player->paused() || loop())
return SleepType::None;
$ run-minibrowser --gtk https://www.w3schools.com/html/html5_video.asp
paused: 1
paused: 1
paused: 1
paused: 1
paused: 1
paused: 1
# ^ this is expected because autoplay is disabled for this element
# now let's press play.
paused: 0
paused: 0
# Playback about to end
paused: 1
paused: 1
# Done...
Michael Catanzaro
I can confirm this seems fixed, in both WebKit JHBuild and my personal JHBuild.
Now, HTMLMediaElement::updateSleepDisabling is not being called when paused/unpaused, which is bad, but I believe we have other bug reports for that.