Attachment 372741 Details for Bug 197358 – Patch for landing

[patch] Patch for landing

bug-197358-20190624111500.patch (text/plain), 5.08 KB, created by Charlie Turner on 2019-06-24 03:15:01 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Charlie Turner

Created: 2019-06-24 03:15:01 PDT

Size: 5.08 KB

patch

obsolete

>Subversion Revision: 246728
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0a476dba9c8664cd63eab3a6273dc225f5b09bf8..70ca6489dc2321d9fb8f94ff7d27ccb3532f22f5 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2019-06-24  Charlie Turner  <cturner@igalia.com>
>+
>+        [GStreamer] Volume level sometimes changes inappropriately
>+        https://bugs.webkit.org/show_bug.cgi?id=197358
>+
>+        Reviewed by Xabier Rodriguez-Calvar.
>+
>+        Be consistent with our application of volume scaling. We were
>+        setting volumes using cubic interpolation in setVolume() and using
>+        the inverse in volume(); however setting initial volumes was done
>+        linearly in setStreamVolumeElement, which was causing strange
>+        jumps in the volume level at non-deterministic times. The fix
>+        looks to be that we should use linear interpolation consistently,
>+        since PulseAudio already applies cubic scaling to software
>+        volumes.
>+
>+        Covered by existing tests.
>+
>+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
>+        (WebCore::MediaPlayerPrivateGStreamer::paused const): Bump the
>+        logging here to LOG level, it's very spammy at DEBUG.
>+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
>+        (WebCore::MediaPlayerPrivateGStreamerBase::setVolume): Switch to
>+        linear interpolation.
>+        (WebCore::MediaPlayerPrivateGStreamerBase::volume const): Ditto.
>+        (WebCore::MediaPlayerPrivateGStreamerBase::notifyPlayerOfVolumeChange):
>+        Ditto.
>+        (WebCore::MediaPlayerPrivateGStreamerBase::setStreamVolumeElement):
>+        Ditto, and be consistent here with the API, do not set the raw
>+        volume managed by MediaElement.
>+
> 2019-06-23  Antoine Quint  <graouts@apple.com>
> 
>         [Pointer Events] Respect pointer capture when dispatching mouse boundary events and updating :hover
>diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
>index c540d3d6626c7b2e879e6038c36dad7dfe5246a2..709a1b628466a12541bca8a142e96613f87adcde 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
>@@ -656,7 +656,7 @@ bool MediaPlayerPrivateGStreamer::paused() const
>     GstState state;
>     gst_element_get_state(m_pipeline.get(), &state, nullptr, 0);
>     bool paused = state <= GST_STATE_PAUSED;
>-    GST_DEBUG_OBJECT(pipeline(), "Paused: %s", toString(paused).utf8().data());
>+    GST_LOG_OBJECT(pipeline(), "Paused: %s", toString(paused).utf8().data());
>     return paused;
> }
> 
>diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp
>index 5108979228380ea5100990afdaee93661bb6b5df..d03d4b58a9abcd47ec1da5978c2db04f286361ed 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp
>@@ -555,7 +555,7 @@ void MediaPlayerPrivateGStreamerBase::setVolume(float volume)
>         return;
> 
>     GST_DEBUG_OBJECT(pipeline(), "Setting volume: %f", volume);
>-    gst_stream_volume_set_volume(m_volumeElement.get(), GST_STREAM_VOLUME_FORMAT_CUBIC, static_cast<double>(volume));
>+    gst_stream_volume_set_volume(m_volumeElement.get(), GST_STREAM_VOLUME_FORMAT_LINEAR, static_cast<double>(volume));
> }
> 
> float MediaPlayerPrivateGStreamerBase::volume() const
>@@ -563,7 +563,7 @@ float MediaPlayerPrivateGStreamerBase::volume() const
>     if (!m_volumeElement)
>         return 0;
> 
>-    return gst_stream_volume_get_volume(m_volumeElement.get(), GST_STREAM_VOLUME_FORMAT_CUBIC);
>+    return gst_stream_volume_get_volume(m_volumeElement.get(), GST_STREAM_VOLUME_FORMAT_LINEAR);
> }
> 
> 
>@@ -572,7 +572,7 @@ void MediaPlayerPrivateGStreamerBase::notifyPlayerOfVolumeChange()
>     if (!m_player || !m_volumeElement)
>         return;
>     double volume;
>-    volume = gst_stream_volume_get_volume(m_volumeElement.get(), GST_STREAM_VOLUME_FORMAT_CUBIC);
>+    volume = gst_stream_volume_get_volume(m_volumeElement.get(), GST_STREAM_VOLUME_FORMAT_LINEAR);
>     // 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).
>@@ -1208,7 +1208,7 @@ void MediaPlayerPrivateGStreamerBase::setStreamVolumeElement(GstStreamVolume* vo
>     // https://bugs.webkit.org/show_bug.cgi?id=118974 for more information.
>     if (!m_player->platformVolumeConfigurationRequired()) {
>         GST_DEBUG_OBJECT(pipeline(), "Setting stream volume to %f", m_player->volume());
>-        g_object_set(m_volumeElement.get(), "volume", m_player->volume(), nullptr);
>+        gst_stream_volume_set_volume(m_volumeElement.get(), GST_STREAM_VOLUME_FORMAT_LINEAR, static_cast<double>(m_player->volume()));
>     } else
>         GST_DEBUG_OBJECT(pipeline(), "Not setting stream volume, trusting system one");
>

Actions: View | Formatted Diff | Diff

Attachments on bug 197358: 372339 | 372353 | 372365 | 372555 | 372741