[Webkit-unassigned] [Bug 240229] [GStreamer][VideoCapture] Add support for capturing encoded video streams from a webcam

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=240229

--- Comment #3 from Loïc Le Page <llepage at igalia.com> ---
Comment on attachment 459042
  --> https://bugs.webkit.org/attachment.cgi?id=459042
Patch

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

>> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp:158
>> +        std::string mimeType;
> 
> We don't use std::string much here, we use String, AtomString, CString from WTF::. I wonder though could this one be const char*?

Effectively, the string memory is hold by deviceCaps which is only destroyed when the function returns, so it can perfectly be a simple const char*.
Thanks for seeing it ;)

>> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp:182
>> +        selector.stopCondition.frameRate = static_cast<double>(numerator) / static_cast<double>(denominator);
> 
> gst_util_fraction_to_double()

done

>> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp:198
>> +                || !gst_structure_get_fraction(structure, "framerate", &numerator, &denominator))
> 
> One line, you can go up to 120 chars per line.

Here it takes more than 120 chars if you try to group the conditions together on the same line.

>> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp:201
>> +            double frameRate = static_cast<double>(numerator) / static_cast<double>(denominator);
> 
> Ditto

done

>> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp:204
>> +                && (frameRate >= selector->stopCondition.frameRate)) {
> 
> One line and no need for so many () I think?

done

>> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCapturer.cpp:211
>> +                && (frameRate >= selector->maxFrameRate)) {
> 
> Ditto

done

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220510/635d3010/attachment.htm>