Bug 197358 - [GStreamer] Volume level sometimes changes inappropriately
Summary: [GStreamer] Volume level sometimes changes inappropriately
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Charlie Turner
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-28 17:48 PDT by Michael Catanzaro
Modified: 2019-06-24 04:28 PDT (History)
10 users (show)

See Also:


Attachments
Patch (8.03 KB, patch)
2019-06-18 08:01 PDT, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch (8.42 KB, patch)
2019-06-18 11:28 PDT, Charlie Turner
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews215 for win-future (13.42 MB, application/zip)
2019-06-18 12:54 PDT, Build Bot
no flags Details
Patch (5.05 KB, patch)
2019-06-20 08:00 PDT, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch for landing (5.08 KB, patch)
2019-06-24 03:15 PDT, Charlie Turner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 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.
Comment 1 Michael Catanzaro 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.
Comment 2 oxumare 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.
Comment 3 Michael Catanzaro 2019-05-10 07:48:42 PDT
Hm, it actually doesn't happen at 4:28 there for me. :(
Comment 4 oxumare 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.
Comment 5 oxumare 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.
Comment 6 Carlos Garcia Campos 2019-05-22 03:49:30 PDT
I have noticed this too
Comment 7 Michael Catanzaro 2019-06-13 10:13:50 PDT
Carlos says he is experiencing this even with MSE disabled, so removing the [MSE] tag.
Comment 8 Michael Catanzaro 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. :)
Comment 9 Charlie Turner 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.
Comment 10 Charlie Turner 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.
Comment 11 Charlie Turner 2019-06-18 08:01:42 PDT
Created attachment 372339 [details]
Patch
Comment 12 Michael Catanzaro 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.
Comment 13 Michael Catanzaro 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;
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Comment 14 Carlos Garcia Campos 2019-06-18 09:11:31 PDT
I'll try it out once it builds
Comment 15 Charlie Turner 2019-06-18 11:28:39 PDT
Created attachment 372353 [details]
Patch

Fix build error
Comment 16 Build Bot 2019-06-18 12:54:48 PDT Comment hidden (spam)
Comment 17 Build Bot 2019-06-18 12:54:51 PDT Comment hidden (spam)
Comment 18 Xabier Rodríguez Calvar 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.
Comment 19 Charlie Turner 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.
Comment 20 Charlie Turner 2019-06-24 03:15:01 PDT
Created attachment 372741 [details]
Patch for landing
Comment 21 WebKit Commit Bot 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>
Comment 22 WebKit Commit Bot 2019-06-24 04:27:13 PDT
All reviewed patches have been landed.  Closing bug.
Comment 23 Radar WebKit Bug Importer 2019-06-24 04:28:21 PDT
<rdar://problem/52049700>