<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[236466] trunk/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/236466">236466</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2018-09-25 11:41:07 -0700 (Tue, 25 Sep 2018)</dd>
</dl>

<h3>Log Message</h3>
<pre>[WPE][GTK][WebRTC] Fixup VP8 encoding support
https://bugs.webkit.org/show_bug.cgi?id=189921

Previous leak fixing commit introduced a regression in
the way the encoded buffer were prepared in the default
GStreamerVideoEncoder::Fragmentize implementation (when
encoding with VP8 basically).

+ Fix a build warning in the decoder.
+ Fix some wrong object members namings.
+ Properly move the caps reference when setting restriction caps.
+ Do not raise a GStreamer error when GStreamerVideoEncoder::OnEncodedImage
  fails - this might be a network issue and other encoders do not consider that
  fatal.
+ Use GstMappedBuffer where appropriate.

Patch by Thibault Saunier <tsaunier@igalia.com> on 2018-09-25
Reviewed by Philippe Normand.

* platform/mediastream/libwebrtc/GStreamerVideoDecoderFactory.cpp:
* platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:
(WebCore::GStreamerVideoEncoder::InitEncode):
(WebCore::GStreamerVideoEncoder::newSampleCallback):
(WebCore::GStreamerVideoEncoder::Fragmentize):
(WebCore::GStreamerVideoEncoder::SetRestrictionCaps):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformmediastreamlibwebrtcGStreamerVideoDecoderFactorycpp">trunk/Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoDecoderFactory.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformmediastreamlibwebrtcGStreamerVideoEncoderFactorycpp">trunk/Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (236465 => 236466)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2018-09-25 18:37:29 UTC (rev 236465)
+++ trunk/Source/WebCore/ChangeLog      2018-09-25 18:41:07 UTC (rev 236466)
</span><span class="lines">@@ -1,3 +1,30 @@
</span><ins>+2018-09-25  Thibault Saunier  <tsaunier@igalia.com>
+
+        [WPE][GTK][WebRTC] Fixup VP8 encoding support
+        https://bugs.webkit.org/show_bug.cgi?id=189921
+
+        Previous leak fixing commit introduced a regression in
+        the way the encoded buffer were prepared in the default
+        GStreamerVideoEncoder::Fragmentize implementation (when
+        encoding with VP8 basically).
+
+        + Fix a build warning in the decoder.
+        + Fix some wrong object members namings.
+        + Properly move the caps reference when setting restriction caps.
+        + Do not raise a GStreamer error when GStreamerVideoEncoder::OnEncodedImage
+          fails - this might be a network issue and other encoders do not consider that
+          fatal.
+        + Use GstMappedBuffer where appropriate.
+
+        Reviewed by Philippe Normand.
+
+        * platform/mediastream/libwebrtc/GStreamerVideoDecoderFactory.cpp:
+        * platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp:
+        (WebCore::GStreamerVideoEncoder::InitEncode):
+        (WebCore::GStreamerVideoEncoder::newSampleCallback):
+        (WebCore::GStreamerVideoEncoder::Fragmentize):
+        (WebCore::GStreamerVideoEncoder::SetRestrictionCaps):
+
</ins><span class="cx"> 2018-09-25  Eric Carlson  <eric.carlson@apple.com>
</span><span class="cx"> 
</span><span class="cx">         [MediaStream] Update constraints supported by getDisplayMedia
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformmediastreamlibwebrtcGStreamerVideoDecoderFactorycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoDecoderFactory.cpp (236465 => 236466)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoDecoderFactory.cpp     2018-09-25 18:37:29 UTC (rev 236465)
+++ trunk/Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoDecoderFactory.cpp        2018-09-25 18:41:07 UTC (rev 236466)
</span><span class="lines">@@ -165,7 +165,7 @@
</span><span class="cx">         GST_BUFFER_PTS(buffer.get()) = (static_cast<guint64>(renderTimeMs) * GST_MSECOND) - m_firstBufferPts;
</span><span class="cx">         m_dtsPtsMap[GST_BUFFER_PTS(buffer.get())] = inputImage._timeStamp;
</span><span class="cx"> 
</span><del>-        GST_LOG_OBJECT(pipeline(), "%ld Decoding: %" GST_PTR_FORMAT, renderTimeMs, buffer);
</del><ins>+        GST_LOG_OBJECT(pipeline(), "%ld Decoding: %" GST_PTR_FORMAT, renderTimeMs, buffer.get());
</ins><span class="cx">         auto sample = adoptGRef(gst_sample_new(buffer.get(), GetCapsForFrame(inputImage), nullptr, nullptr));
</span><span class="cx">         switch (gst_app_src_push_sample(GST_APP_SRC(m_src), sample.get())) {
</span><span class="cx">         case GST_FLOW_OK:
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformmediastreamlibwebrtcGStreamerVideoEncoderFactorycpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp (236465 => 236466)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp     2018-09-25 18:37:29 UTC (rev 236465)
+++ trunk/Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoderFactory.cpp        2018-09-25 18:41:07 UTC (rev 236466)
</span><span class="lines">@@ -81,7 +81,7 @@
</span><span class="cx">         auto caps = adoptGRef(gst_caps_copy(m_restrictionCaps.get()));
</span><span class="cx">         gst_caps_set_simple(caps.get(), "framerate", GST_TYPE_FRACTION, frameRate, 1, nullptr);
</span><span class="cx"> 
</span><del>-        SetRestrictionCaps(caps);
</del><ins>+        SetRestrictionCaps(WTFMove(caps));
</ins><span class="cx"> 
</span><span class="cx">         if (m_bitrateSetter && m_encoder)
</span><span class="cx">             m_bitrateSetter(m_encoder, newBitrate);
</span><span class="lines">@@ -109,7 +109,7 @@
</span><span class="cx"> 
</span><span class="cx">         m_encodedFrame._size = codecSettings->width * codecSettings->height * 3;
</span><span class="cx">         m_encodedFrame._buffer = new uint8_t[m_encodedFrame._size];
</span><del>-        encoded_image_buffer_.reset(m_encodedFrame._buffer);
</del><ins>+        m_encodedImageBuffer.reset(m_encodedFrame._buffer);
</ins><span class="cx">         m_encodedFrame._completeFrame = true;
</span><span class="cx">         m_encodedFrame._encodedWidth = 0;
</span><span class="cx">         m_encodedFrame._encodedHeight = 0;
</span><span class="lines">@@ -159,7 +159,7 @@
</span><span class="cx">     int32_t Release() final
</span><span class="cx">     {
</span><span class="cx">         m_encodedFrame._buffer = nullptr;
</span><del>-        encoded_image_buffer_.reset();
</del><ins>+        m_encodedImageBuffer.reset();
</ins><span class="cx">         GRefPtr<GstBus> bus = adoptGRef(gst_pipeline_get_bus(GST_PIPELINE(m_pipeline.get())));
</span><span class="cx">         gst_bus_set_sync_handler(bus.get(), nullptr, nullptr, nullptr);
</span><span class="cx"> 
</span><span class="lines">@@ -230,7 +230,7 @@
</span><span class="cx">         auto caps = gst_sample_get_caps(sample.get());
</span><span class="cx"> 
</span><span class="cx">         webrtc::RTPFragmentationHeader fragmentationInfo;
</span><del>-        Fragmentize(&m_encodedFrame, &encoded_image_buffer_, buffer, &fragmentationInfo);
</del><ins>+        Fragmentize(&m_encodedFrame, &m_encodedImageBuffer, &m_encodedImageBufferSize, buffer, &fragmentationInfo);
</ins><span class="cx">         if (!m_encodedFrame._size)
</span><span class="cx">             return GST_FLOW_OK;
</span><span class="cx"> 
</span><span class="lines">@@ -249,13 +249,9 @@
</span><span class="cx">         PopulateCodecSpecific(&codecSpecifiInfos, buffer);
</span><span class="cx"> 
</span><span class="cx">         webrtc::EncodedImageCallback::Result result = m_imageReadyCb->OnEncodedImage(m_encodedFrame, &codecSpecifiInfos, &fragmentationInfo);
</span><del>-        if (result.error != webrtc::EncodedImageCallback::Result::OK) {
-            GST_ELEMENT_ERROR(m_pipeline.get(), LIBRARY, FAILED, (nullptr),
-                ("Encode callback failed: %d", result.error));
</del><ins>+        if (result.error != webrtc::EncodedImageCallback::Result::OK)
+            GST_ERROR_OBJECT(m_pipeline.get(), "Encode callback failed: %d", result.error);
</ins><span class="cx"> 
</span><del>-            return GST_FLOW_ERROR;
-        }
-
</del><span class="cx">         return GST_FLOW_OK;
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="lines">@@ -359,23 +355,25 @@
</span><span class="cx"> 
</span><span class="cx">     virtual void PopulateCodecSpecific(webrtc::CodecSpecificInfo*, GstBuffer*) = 0;
</span><span class="cx"> 
</span><del>-    virtual void Fragmentize(webrtc::EncodedImage* encodedImage, std::unique_ptr<uint8_t[]>* encoded_image_buffer, GstBuffer* buffer,
-        webrtc::RTPFragmentationHeader* fragmentationInfo)
</del><ins>+    virtual void Fragmentize(webrtc::EncodedImage* encodedImage, std::unique_ptr<uint8_t[]>* encodedImageBuffer,
+        size_t* bufferSize, GstBuffer* buffer, webrtc::RTPFragmentationHeader* fragmentationInfo)
</ins><span class="cx">     {
</span><del>-        GstMapInfo map;
</del><ins>+        GstMappedBuffer map(buffer, GST_MAP_READ);
</ins><span class="cx"> 
</span><del>-        gst_buffer_map(buffer, &map, GST_MAP_READ);
-        if (encodedImage->_size < map.size) {
-            encodedImage->_size = map.size;
</del><ins>+        if (*bufferSize < map.size()) {
+            encodedImage->_size = map.size();
</ins><span class="cx">             encodedImage->_buffer = new uint8_t[encodedImage->_size];
</span><del>-            encoded_image_buffer->reset(encodedImage->_buffer);
-            memcpy(encodedImage->_buffer, map.data, map.size);
</del><ins>+            encodedImageBuffer->reset(encodedImage->_buffer);
+            *bufferSize = map.size();
</ins><span class="cx">         }
</span><del>-        gst_buffer_unmap(buffer, &map);
</del><span class="cx"> 
</span><ins>+        memcpy(encodedImage->_buffer, map.data(), map.size());
+        encodedImage->_length = map.size();
+        encodedImage->_size = map.size();
+
</ins><span class="cx">         fragmentationInfo->VerifyAndAllocateFragmentationHeader(1);
</span><span class="cx">         fragmentationInfo->fragmentationOffset[0] = 0;
</span><del>-        fragmentationInfo->fragmentationLength[0] = gst_buffer_get_size(buffer);
</del><ins>+        fragmentationInfo->fragmentationLength[0] = map.size();
</ins><span class="cx">         fragmentationInfo->fragmentationPlType[0] = 0;
</span><span class="cx">         fragmentationInfo->fragmentationTimeDiff[0] = 0;
</span><span class="cx">     }
</span><span class="lines">@@ -392,7 +390,7 @@
</span><span class="cx">     void SetRestrictionCaps(GRefPtr<GstCaps> caps)
</span><span class="cx">     {
</span><span class="cx">         if (caps && m_profile.get() && gst_caps_is_equal(m_restrictionCaps.get(), caps.get()))
</span><del>-            g_object_set(m_profile.get(), "restriction-caps", caps, nullptr);
</del><ins>+            g_object_set(m_profile.get(), "restriction-caps", caps.get(), nullptr);
</ins><span class="cx"> 
</span><span class="cx">         m_restrictionCaps = caps;
</span><span class="cx">     }
</span><span class="lines">@@ -413,7 +411,8 @@
</span><span class="cx">     GRefPtr<GstEncodingProfile> m_profile;
</span><span class="cx">     BitrateSetter m_bitrateSetter;
</span><span class="cx">     webrtc::EncodedImage m_encodedFrame;
</span><del>-    std::unique_ptr<uint8_t[]> encoded_image_buffer_;
</del><ins>+    std::unique_ptr<uint8_t[]> m_encodedImageBuffer;
+    size_t m_encodedImageBufferSize;
</ins><span class="cx"> };
</span><span class="cx"> 
</span><span class="cx"> class H264Encoder : public GStreamerVideoEncoder {
</span><span class="lines">@@ -431,10 +430,9 @@
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     // FIXME - MT. safety!
</span><del>-    void Fragmentize(webrtc::EncodedImage* encodedImage, std::unique_ptr<uint8_t[]>* encoded_image_buffer,
</del><ins>+    void Fragmentize(webrtc::EncodedImage* encodedImage, std::unique_ptr<uint8_t[]>* encodedImageBuffer, size_t *bufferSize,
</ins><span class="cx">         GstBuffer* gstbuffer, webrtc::RTPFragmentationHeader* fragmentationHeader) final
</span><span class="cx">     {
</span><del>-        GstMapInfo map;
</del><span class="cx">         GstH264NalUnit nalu;
</span><span class="cx">         auto parserResult = GST_H264_PARSER_OK;
</span><span class="cx"> 
</span><span class="lines">@@ -444,9 +442,9 @@
</span><span class="cx">         std::vector<GstH264NalUnit> nals;
</span><span class="cx"> 
</span><span class="cx">         const uint8_t startCode[4] = { 0, 0, 0, 1 };
</span><del>-        gst_buffer_map(gstbuffer, &map, GST_MAP_READ);
</del><ins>+        GstMappedBuffer map(gstbuffer, GST_MAP_READ);
</ins><span class="cx">         while (parserResult == GST_H264_PARSER_OK) {
</span><del>-            parserResult = gst_h264_parser_identify_nalu(m_parser, map.data, offset, map.size, &nalu);
</del><ins>+            parserResult = gst_h264_parser_identify_nalu(m_parser, map.data(), offset, map.size(), &nalu);
</ins><span class="cx"> 
</span><span class="cx">             nalu.sc_offset = offset;
</span><span class="cx">             nalu.offset = offset + sizeof(startCode);
</span><span class="lines">@@ -461,7 +459,8 @@
</span><span class="cx">         if (encodedImage->_size < requiredSize) {
</span><span class="cx">             encodedImage->_size = requiredSize;
</span><span class="cx">             encodedImage->_buffer = new uint8_t[encodedImage->_size];
</span><del>-            encoded_image_buffer->reset(encodedImage->_buffer);
</del><ins>+            encodedImageBuffer->reset(encodedImage->_buffer);
+            *bufferSize = map.size();
</ins><span class="cx">         }
</span><span class="cx"> 
</span><span class="cx">         // Iterate nal units and fill the Fragmentation info.
</span><span class="lines">@@ -470,20 +469,18 @@
</span><span class="cx">         encodedImage->_length = 0;
</span><span class="cx">         for (std::vector<GstH264NalUnit>::iterator nal = nals.begin(); nal != nals.end(); ++nal, fragmentIndex++) {
</span><span class="cx"> 
</span><del>-            ASSERT(map.data[nal->sc_offset + 0] == startCode[0]);
-            ASSERT(map.data[nal->sc_offset + 1] == startCode[1]);
-            ASSERT(map.data[nal->sc_offset + 2] == startCode[2]);
-            ASSERT(map.data[nal->sc_offset + 3] == startCode[3]);
</del><ins>+            ASSERT(map.data()[nal->sc_offset + 0] == startCode[0]);
+            ASSERT(map.data()[nal->sc_offset + 1] == startCode[1]);
+            ASSERT(map.data()[nal->sc_offset + 2] == startCode[2]);
+            ASSERT(map.data()[nal->sc_offset + 3] == startCode[3]);
</ins><span class="cx"> 
</span><span class="cx">             fragmentationHeader->fragmentationOffset[fragmentIndex] = nal->offset;
</span><span class="cx">             fragmentationHeader->fragmentationLength[fragmentIndex] = nal->size;
</span><span class="cx"> 
</span><del>-            memcpy(encodedImage->_buffer + encodedImage->_length, &map.data[nal->sc_offset],
</del><ins>+            memcpy(encodedImage->_buffer + encodedImage->_length, &map.data()[nal->sc_offset],
</ins><span class="cx">                 sizeof(startCode) + nal->size);
</span><span class="cx">             encodedImage->_length += nal->size + sizeof(startCode);
</span><span class="cx">         }
</span><del>-
-        gst_buffer_unmap(gstbuffer, &map);
</del><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     GstElement* CreateFilter() final
</span></span></pre>
</div>
</div>

</body>
</html>