[webkit-reviews] review granted: [Bug 189734] [GStreamer] Support arbitrary video resolution in getUserMedia API : [Attachment 350999] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Oct 1 10:23:37 PDT 2018
Xabier RodrÃguez Calvar <calvaris at igalia.com> has granted Thibault Saunier
<tsaunier at gnome.org>'s request for review:
Bug 189734: [GStreamer] Support arbitrary video resolution in getUserMedia API
https://bugs.webkit.org/show_bug.cgi?id=189734
Attachment 350999: Patch
https://bugs.webkit.org/attachment.cgi?id=350999&action=review
--- Comment #4 from Xabier RodrÃguez Calvar <calvaris at igalia.com> ---
Comment on attachment 350999
--> https://bugs.webkit.org/attachment.cgi?id=350999
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=350999&action=review
>
Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:2
06
> + 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:2
07
> + for (guint i = 0; i < gst_caps_get_size(caps.get()); i++) {
guint -> unsigned
>
Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:2
36
> + guint frameRatesLength =
static_cast<guint>(gst_value_list_get_size(frameRateValues));
unsigned and cast to the same
>
Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:2
38
> + for (guint j = 0; j < frameRatesLength; j++) {
ditto
>
Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:2
54
> + 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:2
64
> + setSupportedPresets(WTFMove(presets));
> + } else
> + setSupportedPresets(WTFMove(presets));
It looks like you can set the supported presets out of the if and the else,
right?
More information about the webkit-reviews
mailing list