[webkit-reviews] review granted: [Bug 106760] [GStreamer] USE(NATIVE_FULLSCREEN_VIDEO) support : [Attachment 184870] Patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 106760: [GStreamer] USE(NATIVE_FULLSCREEN_VIDEO) support
https://bugs.webkit.org/show_bug.cgi?id=106760

Attachment 184870: Patch
https://bugs.webkit.org/attachment.cgi?id=184870&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=184870&action=review


>
Source/WebCore/platform/graphics/gstreamer/FullscreenVideoControllerGStreamer.c
pp:125
> +{
> +    if (volume < 0.0 || volume > 1.0)
> +	   return;
> +
> +    m_player->setVolume(volume);
> +}

Looks like volume will always be set using 0.05 increments, is that right?
Wondering if it wouldn't make sense to volume = 1.0 if volume > 1.0, and set
it, so that if the volume was at 0.98, say, and it got increaseVolume, it would
go to 1.0. And for volume < 0 as well.

>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1764

> +    // TODO.

I know you said this is fixed in another bug already, but I'd prefer if this
had notImplemented() while that patch doesn't land.