Bug 189734 – [GStreamer] Support arbitrary video resolution in getUserMedia API

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Claudio Saavedra]

fast/mediastream/MediaStreamTrack-getCapabilities.html is failing since r236015 landed in bug 178109. That added support for arbitrary video resolutions in the getUserMedia API. Not sure if something is missing in the gstreamer side or actually the testing infrastructure, but filing this for now.

The failure:

--- /home/buildbot/wpe/wpe-linux-64-release-tests/build/layout-test-results/fast/mediastream/MediaStreamTrack-getCapabilities-expected.txt
+++ /home/buildbot/wpe/wpe-linux-64-release-tests/build/layout-test-results/fast/mediastream/MediaStreamTrack-getCapabilities-actual.txt
@@ -4,12 +4,12 @@
 
 
 video track capabilities:
-  capabilities.aspectRatio = { max: 1.778, min: 1 }
+  capabilities.aspectRatio = { max: 1.778, min: 1.333 }
   capabilities.deviceId = <UUID>
   capabilities.facingMode = [ user ]
   capabilities.frameRate = { max: 30, min: 5 }
-  capabilities.height = { max: 720, min: 112 }
-  capabilities.width = { max: 1280, min: 112 }
+  capabilities.height = { max: 720, min: 480 }
+  capabilities.width = { max: 1280, min: 640 }
 
 audio track capabilities:
   capabilities.deviceId = <UUID>

[Claudio Saavedra]

Other tests failing, same revision:

fast/mediastream/apply-constraints-advanced.html
fast/mediastream/apply-constraints-video.html
fast/mediastream/getUserMedia-default.html

From what I see, the problem might be that in the gstreamer backend the minimum size of video is 640x480, whereas this expect it to be 112x112. Similarly, the default should be 320x240. This from looking at the test results, not that I really have any idea about this.

[Thibault Saunier]

Created attachment 350963 [details]
Patch

[Thibault Saunier]

Created attachment 350999 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 350999 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=350999&action=review

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:206
> +    GRefPtr<GstCaps> caps = adoptGRef(m_capturer->caps());

Suggestion for the future:

I think GStreamerCapturer::caps() should return GRefPtr<GstCaps>, it would make the lifecycle of this line easier to understand.

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:207
> +    for (guint i = 0; i < gst_caps_get_size(caps.get()); i++) {

guint -> unsigned

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:236
> +            guint frameRatesLength = static_cast<guint>(gst_value_list_get_size(frameRateValues));

unsigned and cast to the same

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:238
> +            for (guint j = 0; j < frameRatesLength; j++) {

ditto

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:254
> +        GST_INFO("Could not find any presets for caps: %" GST_PTR_FORMAT " just let anything go out.",
> +            caps.get());

You can put this in the same line

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:264
> +        setSupportedPresets(WTFMove(presets));
> +    } else
> +        setSupportedPresets(WTFMove(presets));

It looks like you can set the supported presets out of the if and the else, right?

[Thibault Saunier]

Created attachment 352073 [details]
Patch for landing

[Thibault Saunier]

Created attachment 352074 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 352074 [details]
Patch for landing

Clearing flags on attachment: 352074

Committed r237048: <https://trac.webkit.org/changeset/237048>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.