Bug 189471 - [GStreamer] Sound loop with Google Hangouts and WhatsApp notifications
Summary: [GStreamer] Sound loop with Google Hangouts and WhatsApp notifications
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords:
: 196000 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-09-10 01:23 PDT by Alberto Garcia
Modified: 2019-03-26 02:41 PDT (History)
9 users (show)

See Also:


Attachments
Gst debug log from epiphany (279.39 KB, application/octet-stream)
2019-01-15 00:28 PST, Pekka Paalanen
no flags Details
Gst debug log from epiphany with simple audio tag (109.04 KB, application/octet-stream)
2019-01-15 03:57 PST, Pekka Paalanen
no flags Details
The notification sound in Mattermost (5.62 KB, application/octet-stream)
2019-01-15 04:18 PST, Pekka Paalanen
no flags Details
Patch (9.60 KB, patch)
2019-03-22 04:57 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (16.00 KB, patch)
2019-03-25 03:22 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (15.47 KB, patch)
2019-03-25 08:02 PDT, Philippe Normand
calvaris: review+
Details | Formatted Diff | Diff
Archive of layout-test-results from ews202 for win-future (12.90 MB, application/zip)
2019-03-25 09:57 PDT, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alberto Garcia 2018-09-10 01:23:31 PDT
I noticed this with WebKitGTK+ 2.22.0.

I open Google Hangouts (https://hangouts.google.com/) in order to chat with some contacts. When someone sends me a message there's a notification sound which should play just once. However with the latest WebKitGTK+ the sound plays in a loop every second until I close the browser.
Comment 1 Michael Catanzaro 2018-10-29 14:31:51 PDT
Reportedly also happens with WhatsApp
Comment 2 Michael Catanzaro 2019-01-03 06:06:41 PST
FWIW I keep getting more reports about this. Is WebAudio broken?
Comment 3 Pekka Paalanen 2019-01-14 07:41:35 PST
This happens for me in Epiphany browser application mode running Mattermost chat app. Every time I get a highlight and it plays a "ding", that "ding" is left on endless loop. Every new highlight opens a new Pulseaudio stream (as seen in pavucontrol) from Epiphany playing "ding" in loop.

I'm on Debian with:
- libwebkit2gtk-4.0-37:amd64 2.22.5-1
- epiphany-browser      3.30.2-2

I don't know when it started, because this is a fresh install.
Comment 4 Philippe Normand 2019-01-14 08:06:57 PST
Can you please get debug logs?

GST_DEBUG_NO_COLOR=1 GST_DEBUG=3,webkit*:6 epiphany ... 2> gst.log
Comment 5 Pekka Paalanen 2019-01-15 00:28:30 PST
Created attachment 359144 [details]
Gst debug log from epiphany

This is the log from:
GST_DEBUG_NO_COLOR=1 GST_DEBUG="3,webkit*:6" epiphany --application-mode --profile=...

Someone pings me once in Mattermost, which plays one "ding", which remains looping until I close epiphany.

The uncompressed log is 4 MB.
Comment 6 Philippe Normand 2019-01-15 02:16:28 PST
(In reply to Michael Catanzaro from comment #2)
> FWIW I keep getting more reports about this. Is WebAudio broken?

This is not related with WebAudio.


(In reply to Pekka Paalanen from comment #5)
> Created attachment 359144 [details]
> Gst debug log from epiphany
> 

Thanks Pekka!
So hum it looks like MediaPlayerPrivateGStreamer::play() is called again after EOS. I don't know why yet unfortunately :(

It would be great if we could have a standalone test for this, I don't use Hangouts, Mattermost or WhatsApp (in browser).

Pekka, can download the Mattermost notification mp3 file and check if the issue happens also when playing the file through a basic <audio> tag?
Comment 7 Pekka Paalanen 2019-01-15 02:24:53 PST
(In reply to Philippe Normand from comment #6)
> Pekka, can download the Mattermost notification mp3 file and check if the
> issue happens also when playing the file through a basic <audio> tag?

If you give me a full example of such HTML file, sure. I wouldn't know how to use an <audio> tag. Do you want it with the same logging as before?
Comment 8 Philippe Normand 2019-01-15 02:32:40 PST
A file containing this:

<audio autoplay src="uri-goes-here"/>

With same logging level yes please :)
Comment 9 Pekka Paalanen 2019-01-15 03:57:33 PST
Created attachment 359153 [details]
Gst debug log from epiphany with simple audio tag

I made a file ding.html with:

<html>
<body>
<audio autoplay src="file:///home/pq/tmp/ding.mp3"/>
</body>
</html>

and ran it with:
GST_DEBUG_NO_COLOR=1 GST_DEBUG="3,webkit*:6" epiphany ./ding.html 2> epiphany-audio-tag.log

Indeed, the ding plays in endless loop, just like in Mattermost in app mode. The Gst log is attached, 2 MB uncompressed.
Comment 10 Philippe Normand 2019-01-15 04:00:03 PST
Can you attach the mp3 file here too?
Comment 11 Pekka Paalanen 2019-01-15 04:18:59 PST
Created attachment 359154 [details]
The notification sound in Mattermost
Comment 12 Philippe Normand 2019-01-15 04:36:50 PST
Here with Ephy 3.30.2 and WebKitGTK 2.22.5 the ding plays one time only.
Comment 13 Alberto Garcia 2019-01-15 04:44:10 PST
I can also hear it only once (Ephy 3.22.7 and WebKitGTK+ 2.22.5), which is interesting because I do get an infinite sound loop in this same system with the sound notifications from Google Hangouts.
Comment 14 Philippe Normand 2019-03-20 07:54:00 PDT
*** Bug 196000 has been marked as a duplicate of this bug. ***
Comment 15 Philippe Normand 2019-03-20 11:26:06 PDT
There's at least 2 issues

1) if the media is remote and the duration query fails, MediaTime::positiveInfiniteTime() is returned from MediaPlayerPrivateGStreamer::durationMediaTime() and that's what triggers the loop

With current trunk this no longer happens, at least for the test case reported by Avner. With 2.24 it still happens, most likely because that branch doesn't have the rewritten webkitwebsrc element

2) if the media is local, well Pekka reported a loop issue as well but I can't reproduce that one here, so it will require more investigation.

For 1) I plan to clean-up our duration query handling, we used to cache it, we don't anymore but now I think we should :) And not returning positiveInfinite() either, that's not done in the AVF player either, I don't know why we do that...
Comment 16 Alberto Garcia 2019-03-22 00:47:14 PDT
I just tried Google Hangouts with the most recent trunk (r243327) and unfortunately the problem still happens.
Comment 17 Philippe Normand 2019-03-22 04:57:59 PDT
Created attachment 365713 [details]
Patch
Comment 18 Philippe Normand 2019-03-22 04:59:10 PDT
Avner, Berto, anyone, can you please test this patch?
Comment 19 softrobot 2019-03-22 05:46:46 PDT
Is there a version of WPE I can try this patch on?
Comment 20 Philippe Normand 2019-03-22 06:03:30 PDT
(In reply to softrobot from comment #19)
> Is there a version of WPE I can try this patch on?

No, if you want to test this patch, download it and apply it with `patch`.
Comment 21 softrobot 2019-03-22 06:29:14 PDT
Yes I have added the patch to buildroot to apply on 2.23.91, but the compilation failed. I think there was some error applying the patch, I will get back to it later tonight.
Comment 22 Philippe Normand 2019-03-22 07:10:42 PDT
For the stable 2.24 branch this might not apply cleanly indeed. Sorry ;)
Comment 23 Alberto Garcia 2019-03-24 11:42:12 PDT
(In reply to Philippe Normand from comment #18)
> Avner, Berto, anyone, can you please test this patch?

Tested on top of r243420, I confirm that Google Hangouts works fine now, thanks!
Comment 24 Philippe Normand 2019-03-25 03:22:23 PDT
Created attachment 365858 [details]
Patch
Comment 25 softrobot 2019-03-25 05:33:23 PDT
Sorry it took me a while, but I can confirm this patch also works on WPE 2.23.91!
Thanks
Comment 26 Xabier Rodríguez Calvar 2019-03-25 05:43:08 PDT
Comment on attachment 365858 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:511
> +    MediaTime duration = platformDuration();
> +    if (!duration || duration.isInvalid())
> +        return MediaTime::zeroTime();

Why zero and not invalid?

How is this working with live sources?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:195
> +    mutable MediaTime m_cachedDuration;
> +    mutable MediaTime m_reportedDuration;

I wonder why we have this duality. Can you please explain?
Comment 27 Philippe Normand 2019-03-25 05:55:19 PDT
That's code copied from the AVF player actually... I should add a comment about it.
Comment 28 Philippe Normand 2019-03-25 05:56:26 PDT
Comment on attachment 365858 [details]
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:511
>> +        return MediaTime::zeroTime();
> 
> Why zero and not invalid?
> 
> How is this working with live sources?

Live sources use positiveInfinite(), which is valid, I think.
Comment 29 Philippe Normand 2019-03-25 08:02:11 PDT
Created attachment 365863 [details]
Patch
Comment 30 EWS Watchlist 2019-03-25 09:57:46 PDT
Comment on attachment 365863 [details]
Patch

Attachment 365863 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11660814

New failing tests:
js/error-should-not-strong-reference-global-object.html
Comment 31 EWS Watchlist 2019-03-25 09:57:57 PDT
Created attachment 365870 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment 32 Philippe Normand 2019-03-26 02:34:29 PDT
Committed r243489: <https://trac.webkit.org/changeset/243489>
Comment 33 Alberto Garcia 2019-03-26 02:41:13 PDT
(In reply to Philippe Normand from comment #32)
> Committed r243489: <https://trac.webkit.org/changeset/243489>

\o/