Bug 223195 – [GStreamer] Videos start playing muted in epiphany with no unmute icon visible in tab, webkit_web_view_get_is_muted() returns incorrect results

• Component: Media
• Status: RESOLVED FIXED

Fedora 34 (alpha)
Epiphany 40.beta
gstreamer1-1.18.2-2
pipewire-0.3.23-2

Videos played on epiphany start with sound muted and no icon indication in the tab. One has to go to settings > sound and unmute the specific WebKitWebProcess . Can be seen on youtube.com and in other places.

Also, a related issue seems to be that (sometimes) the unmute sound icon is not shown in the tab. As in, when playing a video with sound and after clicking the sound icon on the tab to mute, no unmute icon replaces it.

[Philippe Normand]

Works as expected in Ephy TP 40.rc-8-g7ff912064+

Can't reproduce it anymore since updating to epiphany 40.rc .

Should I close this?

[Michael Catanzaro]

Yeah if you can't reproduce, let's close it. That said, there's little chance that an Epiphany change is related....

[Michael Catanzaro]

Reopening because I hit this today myself on youtube.com. Either the WebKitWebProcess set its own volume to zero, or pipewire decided to for some reason.

Sadly I'm not sure how to reproduce.

[Michael Catanzaro]

Ah sorry, it only happens when I mute a YouTube video and then quit Epiphany when the video is playing, such that the YouTube tab is focused when I close the window. If any other tab is focused when closing the browser, then it is not muted when I reopen the browser.

That's weird, but the bigger problem is there's no way to unmute. That's because webkit_web_view_is_playing_audio() is broken, which we just discovered recently. I was planning to report a separate bug for that, but maybe I'll repurpose this one, since I think that's what we're really dealing with here....

[Michael Catanzaro]

(In reply to hoboprimate from comment #0)
> Fedora 34 (alpha)

It could be related to pulseaudio -> pipewire? Alex cannot reproduce on F33, but Jan-Michael and I can both reproduce on F34. The bug doesn't seem to be due to any change in WebKit.

[Michael Catanzaro]

Problem is that after muting the web view, webkit_web_view_is_playing_audio() returns FALSE. That seems kinda reasonable, but it's not what Epiphany and MiniBrowser expect. Both expect use webkit_web_view_is_playing_audio() to decide whether to show the audio indicator, so it needs to return TRUE if audio is playing but muted.

Specifically, MiniBrowser does this:

g_object_bind_property(tab->webView, "is-playing-audio", tab->titleAudioButton, "visible", G_BINDING_DEFAULT | G_BINDING_SYNC_CREATE);

so once the video is muted, the indicator disappears and there's no way to unmute.

[Michael Catanzaro]

I found further confirmation in https://bugs.webkit.org/show_bug.cgi?id=176119#c46, where Carlos says setting WebKitWebView:is-muted does not change the state of WebKitWebView:is-playing-audio. So that does appear to be the intended and past behavior.

Also, our API docs for WebKitWebView:is-muted say:

"Whether the WebKitWebView audio is muted. When TRUE, audio is silenced. It may still be playing, i.e. “is-playing-audio” may be TRUE."

So webkit_web_view_get_is_muted() and WebKitWebView:is-muted are returning incorrect results.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #8)
> So webkit_web_view_get_is_muted() and WebKitWebView:is-muted are returning
> incorrect results.

Sorry, Jan-Michael noticed this is a typo. There's nothing wrong with is-muted. The problem is WebKitWebView:is-playing-audio and webkit_web_view_is_playing_audio() return FALSE when the video is muted, but it needs to return TRUE.

[Philippe Normand]

*** Bug 223433 has been marked as a duplicate of this bug. ***

[Philippe Normand]

(In reply to Michael Catanzaro from comment #6)
> (In reply to hoboprimate from comment #0)
> > Fedora 34 (alpha)
> 
> It could be related to pulseaudio -> pipewire? Alex cannot reproduce on F33,
> but Jan-Michael and I can both reproduce on F34. The bug doesn't seem to be
> due to any change in WebKit.

Can't reproduce on F33 either. I think this could be related with pipewire indded. Please provide logs, as usual.

[Philippe Normand]

OK I've rebased to F34. Can reproduce the issue now:

0:02:25.077821336    69       0x652550 INFO       webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1235:isMuted:<media-player-1> Player is muted: false
0:02:25.077855310    69       0x652550 INFO       webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1706:setMuted:<media-player-1> Setting muted state to true
0:02:25.078523841    69       0x652550 DEBUG      webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1717:notifyPlayerOfMute:<media-player-1> Notifying player of new mute value: false
0:02:25.078564079    69       0x652550 DEBUG      webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1653:setVolume:<media-player-1> Setting volume: 0.530206
0:02:25.079093901    69       0x652550 DEBUG      webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1684:volumeChangedCallback:<media-player-1> Volume changed to: 0.530206
0:02:25.082536880    69       0x67c2a0 DEBUG      webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1684:volumeChangedCallback:<media-player-1> Volume changed to: 0.530206
0:02:25.083244316    69       0x652550 DEBUG      webkitmediaplayer MediaPlayerPrivateGStreamer.cpp:1717:notifyPlayerOfMute:<media-player-1> Notifying player of new mute value: true


So we call setMuted(true) and get a GObject notification from playbin but the value is false instead of true, and a while later another notification which is correct this time...

[Philippe Normand]

(In reply to Michael Catanzaro from comment #9)
> (In reply to Michael Catanzaro from comment #8)
> > So webkit_web_view_get_is_muted() and WebKitWebView:is-muted are returning
> > incorrect results.
> 
> Sorry, Jan-Michael noticed this is a typo. There's nothing wrong with
> is-muted. The problem is WebKitWebView:is-playing-audio and
> webkit_web_view_is_playing_audio() return FALSE when the video is muted, but
> it needs to return TRUE.

MediaProducer::MediaStateFlags HTMLMediaElement::mediaState() const
{
...
    if (hasAudio && !muted() && volume())
        state |= IsPlayingAudio;
}

So that's why webkit_web_view_is_playing_audio() returns FALSE when the video is muted.

[Michael Catanzaro]

(In reply to Philippe Normand from comment #13)
> MediaProducer::MediaStateFlags HTMLMediaElement::mediaState() const
> {
> ...
>     if (hasAudio && !muted() && volume())
>         state |= IsPlayingAudio;
> }
> 
> So that's why webkit_web_view_is_playing_audio() returns FALSE when the
> video is muted.

I found that yesterday and thought it looked concerning, given the API requires different behavior, but it clearly didn't work before now.

Any ideas what the path forward is? Do we dare try changing that...?

[Philippe Normand]

(In reply to Michael Catanzaro from comment #14)
> (In reply to Philippe Normand from comment #13)
> > MediaProducer::MediaStateFlags HTMLMediaElement::mediaState() const
> > {
> > ...
> >     if (hasAudio && !muted() && volume())
> >         state |= IsPlayingAudio;
> > }
> > 
> > So that's why webkit_web_view_is_playing_audio() returns FALSE when the
> > video is muted.
> 
> I found that yesterday and thought it looked concerning, given the API
> requires different behavior, but it clearly didn't work before now.
> 
> Any ideas what the path forward is? Do we dare try changing that...?

I'm not sure what to do yet. I think there are 3 issues:

- muting the WebProcess from the GNOME settings makes the speaker icon disappear from the MB tab that is playing media
- there's a racy behavior where muting audio through the MB speaker icon sometimes makes it disappear
- if a WebProcess was muted from GNOME settings and the browser is closed, reopening it and playing media leads to muted audio and invisible speaker icon in the tab

[Philippe Normand]

I've updated to Pipewire 0.3.24 and am now unable to reproduce this issue here. Can you check as well please?

[Philippe Normand]

Dang it, scratch this, it's a racy speaker icon disappearance.

[Philippe Normand]

Created attachment 424491 [details]
Patch

[Philippe Normand]

Created attachment 424494 [details]
Patch

[Eric Carlson]

Comment on attachment 424494 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424494&action=review

> Source/WebCore/html/HTMLMediaElement.cpp:7690
> +    bool isPlayingAudio = hasAudio && volume();

Shouldn't this be `bool isPlayingAudio = hasAudio && !muted() && volume()` to avoid changing behavior on non-GStreamer ports?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1678
> +    auto volume = this->volume();
> +    auto oldVolume = volume;
> +
>      // get_volume() can return values superior to 1.0 if the user
>      // applies software user gain via third party application (GNOME
>      // volume control for instance).
>      volume = CLAMP(volume, 0.0, 1.0);

Nit: this would be sightly more efficient as:

auto oldVolume = this->volume();
auto volume = CLAMP(oldVolume, 0.0, 1.0);

[Michael Catanzaro]

Comment on attachment 424494 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424494&action=review

>> Source/WebCore/html/HTMLMediaElement.cpp:7690
>> +    bool isPlayingAudio = hasAudio && volume();
> 
> Shouldn't this be `bool isPlayingAudio = hasAudio && !muted() && volume()` to avoid changing behavior on non-GStreamer ports?

On non-GStreamer ports, && !muted is tested on the next line. It's an #if !USE(GSTREAMER) condition.

[Carlos Garcia Campos]

Comment on attachment 424494 [details]
Patch

Would it be possible to add a unit test to check the fixed behavior?

[Philippe Normand]

(In reply to Carlos Garcia Campos from comment #22)
> Comment on attachment 424494 [details]
> Patch
> 
> Would it be possible to add a unit test to check the fixed behavior?

Yes I think so, at least update existing API tests. The current ones don't test mixed usage of the is_playing_audio and is_muted APIs, which is not great because in practice we do use both in interesting ways.

[Philippe Normand]

Created attachment 424998 [details]
Patch

[Michael Catanzaro]

Comment on attachment 424998 [details]
Patch

New API test is r=me, though 50ms * 20 = 1 second, which is a little long for a test. (I would be more concerned if this were a layout test. We don't have so many API tests that one extra second would be a big deal.)

(You already have Eric's r+ for the media changes.)

[Philippe Normand]

Created attachment 425368 [details]
Patch

[Philippe Normand]

(In reply to Michael Catanzaro from comment #25)
> Comment on attachment 424998 [details]
> Patch
> 
> New API test is r=me, though 50ms * 20 = 1 second, which is a little long
> for a test. (I would be more concerned if this were a layout test. We don't
> have so many API tests that one extra second would be a big deal.)
> 
> (You already have Eric's r+ for the media changes.)

I´ve changed it to 500ms.

[Philippe Normand]

Created attachment 425369 [details]
Patch

[EWS]

Committed r275600: <https://commits.webkit.org/r275600>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425369 [details].

[Radar WebKit Bug Importer]

<rdar://problem/76330028>

[Michael Catanzaro]

Hm, this is not completely fixed. We are still sometimes starting youtube videos without any sound. However, since 2.32.1, it's now always possible to fix it by pausing and restarting the video without having to modify system sound settings, so that is an improvement at least.

Do you want a fresh bug report? Unfortunately I can't reproduce it on demand, so no logs yet.

[Philippe Normand]

New bug yes please.

[Michael Catanzaro]

Bug #225857.

[Michael Catanzaro]

Unfortunately I am still seeing this with 2.32.1... it's happened twice now in the past couple days. This seems to be a separate issue from bug #225857.

[Philippe Normand]

I'm lost in the backlog... what are the steps to reproduce this?

[Michael Catanzaro]

(In reply to Philippe Normand from comment #35)
> I'm lost in the backlog... what are the steps to reproduce this?

We don't know, that's the problem. I think it's a race condition, so it's down to luck....

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #31)
> Hm, this is not completely fixed. We are still sometimes starting youtube
> videos without any sound. However, since 2.32.1, it's now always possible to
> fix it by pausing and restarting the video without having to modify system
> sound settings, so that is an improvement at least.

To be clear: this is wrong, you still sometimes have to modify system sound settings to get the sound back, that's why I reopened this bug.

When it happens, it happens to many tabs all at once. I'm not sure, but I think maybe the affected tabs are somehow inheriting volume level from one another?

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #37)
> To be clear: this is wrong, you still sometimes have to modify system sound
> settings to get the sound back, that's why I reopened this bug.

I'm not seeing this anymore, so closing again.

We still have bug #225857 that occurs regularly, though.