[WPE][GTK][WebRTC] Fixup VP8 encoding support
Created attachment 350658 [details] Patch
Comment on attachment 350658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350658&action=review > Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:380 > fragmentationInfo->fragmentationTimeDiff[0] = 0; > + gst_buffer_unmap(buffer, &map); You could use the GstMappedBuffer thing here maybe?
Created attachment 350746 [details] Patch
Comment on attachment 350746 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350746&action=review > Source/WebCore/ChangeLog:14 > + + And do not raise a GStreamer error when GStreanerVideoEnclder::OnEncodedImage typo Enclder > Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:84 > + SetRestrictionCaps(WTFMove(caps)); Are all the SetRestrictionCaps call sites OK now? > Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:255 > + GST_ERROR_OBJECT(m_pipeline.get(), "Encode callback failed: %d", result.error); > > return GST_FLOW_OK; Is it OK to return FLOW_OK also when an error is raised? > Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:370 > + memcpy(encodedImage->_buffer, map.data(), map.size()); I know we had this before but would there be a way to avoid this?
(In reply to Philippe Normand from comment #4) > Comment on attachment 350746 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=350746&action=review > > > Source/WebCore/ChangeLog:14 > > + + And do not raise a GStreamer error when GStreanerVideoEnclder::OnEncodedImage > > typo Enclder Fixed. > > Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:84 > > + SetRestrictionCaps(WTFMove(caps)); > > Are all the SetRestrictionCaps call sites OK now? It is the only call yes. > > Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:255 > > + GST_ERROR_OBJECT(m_pipeline.get(), "Encode callback failed: %d", result.error); > > > > return GST_FLOW_OK; > > Is it OK to return FLOW_OK also when an error is raised? It is not raising an error anymore, just a "small" runtime `GST_ERROR`, the only 2 return values for `webrtc::EncodedImageCallback::Result` are: * OK * ERROR_SEND_FAILED so basically we should not stop processing when `ERROR_SEND_FAILED` happens, so no reason to post an error on the bus. > > Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:370 > > + memcpy(encodedImage->_buffer, map.data(), map.size()); > > I know we had this before but would there be a way to avoid this? There might be a way but I can't see any encoder implementation avoiding this memcpy, it is an encoded frame so it shouldn't be a huge deal in practice.
Comment on attachment 350746 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350746&action=review >> Source/WebCore/ChangeLog:14 >> + + And do not raise a GStreamer error when GStreanerVideoEnclder::OnEncodedImage > > typo Enclder and GStreaner :)
Created attachment 350748 [details] Patch
Comment on attachment 350748 [details] Patch Clearing flags on attachment: 350748 Committed r236466: <https://trac.webkit.org/changeset/236466>
All reviewed patches have been landed. Closing bug.
<rdar://problem/44768397>