[GStreamer] Fix EncodedImage timestamps to match what libWebRTC expects
Created attachment 350957 [details] Patch
Comment on attachment 350957 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350957&action=review Great catch, tricky problem, LGTM with some small changes. > Source/WebCore/ChangeLog:12 > + all the GStreamer processing pipelines as the WebRTC object basically wrap the "same" Nit, wrap -> wraps > Source/WebCore/ChangeLog:26 > + because of that issue. Nit, is this a leftover? > Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:424 > + std::map<GstClockTime, RtpTimestamps> m_ptsRtpTimestampMap; We usually use HashMap type defined in WTF, is there any feature in the std::maps that we want to use?
Created attachment 352612 [details] Patch
Created attachment 353868 [details] Patch
Comment on attachment 353868 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353868&action=review > Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:70 > + , m_adapter(adoptGRef(gst_adapter_new())) Hum. I think this needs a GRefPtrGStreamer template specialization to avoid leaks.
Comment on attachment 353868 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353868&action=review >> Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:70 >> + , m_adapter(adoptGRef(gst_adapter_new())) > > Hum. I think this needs a GRefPtrGStreamer template specialization to avoid leaks. Ah no, it's a normal GObject. Nevermind :)
Comment on attachment 353868 [details] Patch Clearing flags on attachment: 353868 Committed r237819: <https://trac.webkit.org/changeset/237819>
All reviewed patches have been landed. Closing bug.
<rdar://problem/45813371>
Comment on attachment 353868 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353868&action=review I'm late, but if there's any follow up, we can fix this couple of things. > Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoDecoderFactory.cpp:242 > + m_bufferMapLock.lock(); > + // Make sure that the frame.timestamp == previsouly input_frame._timeStamp > + // as it is required by the VideoDecoder baseclass. > + auto timestamps = m_dtsPtsMap[GST_BUFFER_PTS(buffer)]; > + m_dtsPtsMap.erase(GST_BUFFER_PTS(buffer)); > + m_bufferMapLock.unlock(); Nit: I would have used a Locker here by either using unlockEarly or just using { } to create scope. Besides, previsouly -> previously. > Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:266 > + uint flags = GST_BUFFER_FLAGS(buffer); unsigned