Attachment 372555 Details for Bug 197358 – Patch

[patch] Patch

bug-197358-20190620160022.patch (text/plain), 5.05 KB, created by Charlie Turner on 2019-06-20 08:00:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Charlie Turner

Created: 2019-06-20 08:00:24 PDT

Size: 5.05 KB

patch

obsolete

>Subversion Revision: 246536
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 681ccb6215f852bfb2669b8667e190a29895113c..7a89f4b2eebb1186fdfc0de7819255b1df097ccd 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2019-06-20  Charlie Turner  <cturner@igalia.com>
>+
>+        [GStreamer] Volume level sometimes changes inappropriately
>+        https://bugs.webkit.org/show_bug.cgi?id=197358
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-18  Zan Dobersek  <zdobersek@igalia.com>
> 
>         [WebGL] Extensions3DOpenGLES::bindVertexArrayOES() should allow zero array object
>diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
>index 0e0835b01f8acd9d03300520686f05a270e13b2d..e1d1b2debca8a51246ff98801e5c2d69f062d4b6 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
>@@ -663,7 +663,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 6ea36580f2d50e8b380f7d515c6a4017343aa8fd..5b02a23ac4c9fd1f5a1be01287f5c29126af98d5 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp
>@@ -573,7 +573,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
>@@ -581,7 +581,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);
> }
> 
> 
>@@ -590,7 +590,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).
>@@ -1232,7 +1232,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