RESOLVED FIXED 197358
[GStreamer] Volume level sometimes changes inappropriately
https://bugs.webkit.org/show_bug.cgi?id=197358
Summary [GStreamer] Volume level sometimes changes inappropriately
Michael Catanzaro
Reported 2019-04-28 17:48:35 PDT
Since we enabled MSE the volume level on YouTube sometimes changes randomly. This is not easy to reproduce. You might have to play videos for quite a while to notice.
Attachments
Patch (8.03 KB, patch)
2019-06-18 08:01 PDT, Charlie Turner
no flags
Patch (8.42 KB, patch)
2019-06-18 11:28 PDT, Charlie Turner
no flags
Archive of layout-test-results from ews215 for win-future (13.42 MB, application/zip)
2019-06-18 12:54 PDT, EWS Watchlist
no flags
Patch (5.05 KB, patch)
2019-06-20 08:00 PDT, Charlie Turner
no flags
Patch for landing (5.08 KB, patch)
2019-06-24 03:15 PDT, Charlie Turner
no flags
Michael Catanzaro
Comment 1 2019-04-28 17:51:02 PDT
(In reply to Michael Catanzaro from comment #0) > Since we enabled MSE the volume level on YouTube sometimes changes randomly. Oh I should add: there are only two states, bugged volume and non-bugged. It will randomly flip between the two. One state is roughly twice as loud as the other.
oxumare
Comment 2 2019-05-10 05:52:29 PDT
Hi all, searching for a possible bug I ended here. What I can say about is: in certain videos it is sistematic, the volume turns low at exactly the same moment. For example in this it happens at 4:28 https://www.youtube.com/watch?v=30QbX5YRwq8 Disabling mediasource works fine.
Michael Catanzaro
Comment 3 2019-05-10 07:48:42 PDT
Hm, it actually doesn't happen at 4:28 there for me. :(
oxumare
Comment 4 2019-05-10 08:28:18 PDT
Hm, strange... with me it's sistematic. It starts with a good volume and at 4:28 turns out about a half, very annoying. And with this video https://www.youtube.com/watch?v=FvcHVo-49b0&list=PL6H6TfFpYvpcKyuiErpxxxQsVoCzCK7F0&index=7 it happens always at 2:26 This is what I have installed net-libs/webkit-gtk-2.24.1 media-libs/gst-plugins-bad (1.14.3(1.0)@01/09/2019): media-libs/gst-plugins-base (0.10.36-r2(0.10)@04/26/2019 media-libs/gst-plugins-good (1.14.4(1.0)@10/04/2018): media-libs/gst-plugins-ugly (1.14.3(1.0)@01/09/2019): media-libs/gstreamer (0.10.36-r2(0.10)@04/26/2019 media-plugins/gst-plugins-dash (1.14.3(1.0)@04/15/2019): media-plugins/gst-plugins-gtk (1.14.4(1.0)@01/09/2019): media-plugins/gst-plugins-libav (1.14.4.3.4.5(1.0)@01/09/2019): media-plugins/gst-plugins-libde265 (1.14.3(1.0)@04/15/2019): media-plugins/gst-plugins-openh264 (1.14.3(1.0)@03/31/2019): media-plugins/gst-plugins-opus (1.14.4-r1(1.0)@05/05/2019): media-plugins/gst-plugins-oss (1.14.4(1.0)@01/09/2019): media-plugins/gst-plugins-vpx (1.14.4(1.0)@04/15/2019): on gentoo. I made a browser with python gi and webkit-gtk, and am very satisfied. There are some quirk here and there but I think tha with time all will be fine.
oxumare
Comment 5 2019-05-10 08:32:56 PDT
Errata corrige: media-libs/gstreamer-1.14.4 in gentoo some packages are slotted, i.e. more than a version installed.
Carlos Garcia Campos
Comment 6 2019-05-22 03:49:30 PDT
I have noticed this too
Michael Catanzaro
Comment 7 2019-06-13 10:13:50 PDT
Carlos says he is experiencing this even with MSE disabled, so removing the [MSE] tag.
Michael Catanzaro
Comment 8 2019-06-13 10:18:08 PDT
(In reply to Michael Catanzaro from comment #7) > Carlos says he is experiencing this even with MSE disabled, so removing the > [MSE] tag. Sorry we had a misunderstanding, he was talking about something else he had disabled. This is definitely MSE. :)
Charlie Turner
Comment 9 2019-06-18 07:54:57 PDT
I am not convinced this is specific to MSE. I believe it may be due to inconsistent application of cubic scaling in our volume code. There are three places volume is handled here 1) In the MediaPlayer class, this takes whatever the JS app tells us is the volume. The scaling function is unspecified, so it's stored absolute (i.e., linearly). 2) In the assigned GstStreamVolume element, in our case a playbin. Here we are inconsistently using a cubic scale. 3) In the PA server, which uses cubic scaling anyway for software volumes, and for hardware volumes/pass-through I don't understand what it does, but it seems like the scaling function may again be undefined. On a new video load, we would set the GstStreamVolume element (in our case the playbin) using a raw value from the MediaPlayer (unscaled). Any volume changes (which can happen in myriad ways, sometimes depending on network conditions and/or ads playing) would cause a volume change and an inverse cubic evaluation (i.e., v^3 where v in [0..1]), causing the volume to jump down to something unexpected. This can also jump up when volumes are set in the wrong order, in this case it would be v^1/3, causing v to jump upwards in this range. With YouTube, some of the ways this can happen are, 1) When you are in autoplay and change the volume, that volume will be remembered (sometimes). After you set it, it will be stored as the cubic interpolation, but when the new video loads, it will be stored linearly (jumping audio) 2) Sometimes the JS media players set a seemingly random starting volume, maybe based off some kind of perceived loudness? Anyway, we have the same problem here as 1, this is set linearly, and it can be bounced by things like point 3) 3) When ads play, or when network conditions change, we get resolution changes. Playsink in GStreamer has a condition variable which fires whenever a new audio chain is generated which can happen when caps change like this (and for a couple other reasons that I won't explain further). When this happens, we again hit the cubic/linear bug, but at times which are non-deterministic. This explains most user reports I've seen. I'm in the unfortunate position that on 2.24.3, I can't reproduce the volume changing randomly during playback. I could trigger some strange things by manually switching resolutions and faffing with PA controls and seeing how subsequent resolution changes and/or ads playing affected the volume. I will post a speculative patch and ask that affected users try and let me know if it helps. I was developed on 2.24.3 and ported to current trunk. One unresolved issue I have noticed is that the settings of the PA controls do not reflect into the YouTube player. It seems like it doesn't care about volumechange events. I notice the same behavior with FF, but for example if you drop the PA volume down to 10% with YouTube on its interpretation of 100%, YouTube will still be on 100%. If you then drop YouTube to 50$, you do not get 5% volume, you get an increase in volume to 50%... Without seeing the decompiled YT player source code, I'm not hopeful I'll understand why that happens. Needless to say, with native controls, this is not an issue.
Charlie Turner
Comment 10 2019-06-18 08:01:04 PDT
There's a number of past greatest hits around volume bugs, some of which are, https://bugs.webkit.org/show_bug.cgi?id=54140 - initial bug report about volume issues https://bugs.webkit.org/show_bug.cgi?id=118974 - this was about users with PA flat volumes. Ubunutu disables it by default. https://bugzilla.redhat.com/show_bug.cgi?id=1240880 - complaining about flat volumes, just turn it off. https://bugs.webkit.org/show_bug.cgi?id=140358 - suggesting we use some API for Browsers provided by PA? https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/41 — potentially the API referred to for a "browsers" API for PA. I'm at least aware this might cause some regressions, but from my digging, the following change *seems* like it's more consistent.
Charlie Turner
Comment 11 2019-06-18 08:01:42 PDT
Michael Catanzaro
Comment 12 2019-06-18 08:44:46 PDT
(In reply to Charlie Turner from comment #9) > I will post a speculative patch and ask that affected users try and let me > know if it helps. I was developed on 2.24.3 and ported to current trunk. I don't think anybody can reproduce this: it just happens occasionally. The only way to really know if you've fixed it is to release this change to users and then ask a few weeks later if it's gone away. So if multimedia reviewers believe your patch is theoretically correct, then we should land it. P.S. You must mean 2.24.2, since 2.24.3 doesn't exist yet. (In reply to Charlie Turner from comment #10) > https://bugs.webkit.org/show_bug.cgi?id=140358 - suggesting we use some > API for Browsers provided by PA? I'm going to close this one since it's obsolete.
Michael Catanzaro
Comment 13 2019-06-18 08:45:14 PDT
All the EWS are failing because: In file included from /home/ews/gtk-wk2-ews/WebKit/Source/WebCore/html/track/VideoTrack.cpp:37: ../../Source/WebCore/html/HTMLMediaElement.h:719:10: error: ‘bool WebCore::HTMLMediaElement::mediaPlayerPlatformVolumeConfigurationRequired() const’ marked ‘override’, but does not override bool mediaPlayerPlatformVolumeConfigurationRequired() const override; ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Carlos Garcia Campos
Comment 14 2019-06-18 09:11:31 PDT
I'll try it out once it builds
Charlie Turner
Comment 15 2019-06-18 11:28:39 PDT
Created attachment 372353 [details] Patch Fix build error
EWS Watchlist
Comment 16 2019-06-18 12:54:48 PDT Comment hidden (spam)
EWS Watchlist
Comment 17 2019-06-18 12:54:51 PDT Comment hidden (spam)
Xabier Rodríguez Calvar
Comment 18 2019-06-19 23:53:21 PDT
Comment on attachment 372353 [details] Patch Let's leave aside the changes about volume initialization and keep only the ones related to cubic -> linear. Volume initialization will be tackled soon.
Charlie Turner
Comment 19 2019-06-20 08:00:24 PDT
Created attachment 372555 [details] Patch Do not remove the volume initialization code, for sites that do not set an initial volume, this code at least preserves the last set volume across page reloads at the cost of complexity. Note this is not the case for pretty much every video streaming site in the wild, since they all set initial volumes explicitly. It is also not a requirement of compliant user-agents, but may be removed in the future with a larger refactoring in the pipeline.
Charlie Turner
Comment 20 2019-06-24 03:15:01 PDT
Created attachment 372741 [details] Patch for landing
WebKit Commit Bot
Comment 21 2019-06-24 04:27:11 PDT
Comment on attachment 372741 [details] Patch for landing Clearing flags on attachment: 372741 Committed r246730: <https://trac.webkit.org/changeset/246730>
WebKit Commit Bot
Comment 22 2019-06-24 04:27:13 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 23 2019-06-24 04:28:21 PDT
Note You need to log in before you can comment on or make changes to this bug.