RemoteVideoEncoderCallbacks and RemoteVideoDecoderCallbacks HashMaps …

…could be corrupted by JS https://bugs.webkit.org/show_bug.cgi?id=258123 rdar://110777506 Reviewed by Eric Carlson. JS can provide signed timestamps, which are used as keys in RemoteVideoDecoderCallbacks and RemoteVideoEncoderCallbacks maps. Move to StdUnorderedMap to support all keys. * LayoutTests/http/wpt/webcodecs/videoFrame-negative-timestamp-expected.txt: Added. * LayoutTests/http/wpt/webcodecs/videoFrame-negative-timestamp.html: Added. * Source/WebKit/WebProcess/GPU/media/RemoteVideoCodecFactory.cpp: (WebKit::RemoteVideoDecoderCallbacks::addDuration): (WebKit::RemoteVideoEncoderCallbacks::addDuration): (WebKit::RemoteVideoDecoderCallbacks::notifyDecodingResult): (WebKit::RemoteVideoEncoderCallbacks::notifyEncodedChunk): Canonical link: https://commits.webkit.org/259548.831@safari-7615-branch
WebKit · Jun 15, 2023 · d263e8a · d263e8a
1 parent 9cd4491
commit d263e8a
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 6 deletions.
diff --git a/LayoutTests/http/wpt/webcodecs/videoFrame-negative-timestamp-expected.txt b/LayoutTests/http/wpt/webcodecs/videoFrame-negative-timestamp-expected.txt
@@ -0,0 +1,5 @@
+
+PASS VP8 decoded frames
+PASS VP9 decoded frames
+PASS H.264 decoded frames
+
diff --git a/LayoutTests/http/wpt/webcodecs/videoFrame-negative-timestamp.html b/LayoutTests/http/wpt/webcodecs/videoFrame-negative-timestamp.html
@@ -0,0 +1,90 @@
+<!DOCTYPE html>
+<html>
+<header>
+<script src='/resources/testharness.js'></script>
+<script src='/resources/testharnessreport.js'></script>
+</header>
+<body>
+<script>
+
+function makeOffscreenCanvas(width, height, timestamp) {
+  let canvas = new OffscreenCanvas(width, height);
+  let ctx = canvas.getContext('2d');
+  ctx.fillStyle = 'rgba(50, 100, 150, 255)';
+  ctx.fillRect(0, 0, width, height);
+  return new VideoFrame(canvas, { timestamp, duration : 10 });
+}
+
+async function doEncodeDecode(encoderConfig, timestamp)
+{
+  let resolve, reject;
+  const promise = new Promise((res, rej) => {
+    resolve = res;
+    reject = rej;
+  });
+
+  const decoder = new VideoDecoder({
+    output(frame) {
+      resolve(frame);
+    },
+    error(e) {
+      reject(e.message);
+    }
+  });
+
+  const encoderInit = {
+    output(chunk, metadata) {
+      let config = metadata.decoderConfig;
+      if (config) {
+        decoder.configure(config);
+      }
+      decoder.decode(chunk);
+    },
+    error(e) {
+      reject(e.message);
+    }
+  };
+
+  const encoder = new VideoEncoder(encoderInit);
+  encoder.configure(encoderConfig);
+
+  const w = encoderConfig.width;
+  const h = encoderConfig.height;
+  const frame = makeOffscreenCanvas(w, h, timestamp);
+  encoder.encode(frame, { keyFrame: true });
+  frame.close();
+
+  await encoder.flush();
+  await decoder.flush();
+  encoder.close();
+  decoder.close();
+
+  return promise;
+}
+
+function doTest(codec, title)
+{
+  const config = { codec };
+  config.width = 320;
+  config.height = 200;
+  config.bitrate = 1000000;
+  config.framerate = 30;
+
+  promise_test(async t => {
+    const frame1 = await doEncodeDecode(config, -2);
+    t.add_cleanup(() => frame1.close());
+
+    const frame2 = await doEncodeDecode(config, -1);
+    t.add_cleanup(() => frame2.close());
+
+    const frame3 = await doEncodeDecode(config, 0);
+    t.add_cleanup(() => frame3.close());
+  }, title);
+}
+
+doTest('vp8', "VP8 decoded frames");
+doTest('vp09.00.10.08', "VP9 decoded frames");
+doTest('avc1.42001E', "H.264 decoded frames");
+</script>
+</body>
+</html>
diff --git a/Source/WebKit/WebProcess/GPU/media/RemoteVideoCodecFactory.cpp b/Source/WebKit/WebProcess/GPU/media/RemoteVideoCodecFactory.cpp
@@ -30,6 +30,7 @@
 
 #include "LibWebRTCCodecs.h"
 #include "WebProcess.h"
+#include <wtf/StdUnorderedMap.h>
 
 namespace WebKit {
 
@@ -43,15 +44,15 @@ class RemoteVideoDecoderCallbacks : public ThreadSafeRefCounted<RemoteVideoDecod
 
     // Must be called on the VideoDecoder thread, or within postTaskCallback.
     void close() { m_isClosed = true; }
-    void addDuration(int64_t timestamp, uint64_t duration) { m_timestampToDuration.add(timestamp + 1, duration); }
+    void addDuration(int64_t timestamp, uint64_t duration) { m_timestampToDuration.insert_or_assign(timestamp, duration); }
 
 private:
     RemoteVideoDecoderCallbacks(VideoDecoder::OutputCallback&&, VideoDecoder::PostTaskCallback&&);
 
     VideoDecoder::OutputCallback m_outputCallback;
     VideoDecoder::PostTaskCallback m_postTaskCallback;
     bool m_isClosed { false };
-    HashMap<int64_t, uint64_t> m_timestampToDuration;
+    StdUnorderedMap<int64_t, uint64_t> m_timestampToDuration;
 };
 
 class RemoteVideoDecoder : public WebCore::VideoDecoder {
@@ -84,7 +85,7 @@ class RemoteVideoEncoderCallbacks : public ThreadSafeRefCounted<RemoteVideoEncod
 
     // Must be called on the VideoDecoder thread, or within postTaskCallback.
     void close() { m_isClosed = true; }
-    void addDuration(int64_t timestamp, uint64_t duration) { m_timestampToDuration.add(timestamp + 1, duration); }
+    void addDuration(int64_t timestamp, uint64_t duration) { m_timestampToDuration.insert_or_assign(timestamp, duration); }
 
 private:
     RemoteVideoEncoderCallbacks(VideoEncoder::DescriptionCallback&&, VideoEncoder::OutputCallback&&, VideoEncoder::PostTaskCallback&&);
@@ -93,7 +94,7 @@ class RemoteVideoEncoderCallbacks : public ThreadSafeRefCounted<RemoteVideoEncod
     VideoEncoder::OutputCallback m_outputCallback;
     VideoEncoder::PostTaskCallback m_postTaskCallback;
     bool m_isClosed { false };
-    HashMap<int64_t, uint64_t> m_timestampToDuration;
+    StdUnorderedMap<int64_t, uint64_t> m_timestampToDuration;
 };
 
 class RemoteVideoEncoder : public WebCore::VideoEncoder {
@@ -223,7 +224,12 @@ void RemoteVideoDecoderCallbacks::notifyDecodingResult(RefPtr<WebCore::VideoFram
             protectedThis->m_outputCallback(makeUnexpected("Decoder failure"_s));
             return;
         }
-        auto duration = protectedThis->m_timestampToDuration.take(timestamp + 1);
+        uint64_t duration = 0;
+        auto iterator = protectedThis->m_timestampToDuration.find(timestamp);
+        if (iterator != protectedThis->m_timestampToDuration.end()) {
+            duration = iterator->second;
+            protectedThis->m_timestampToDuration.erase(iterator);
+        }
         protectedThis->m_outputCallback(VideoDecoder::DecodedFrame { frame.releaseNonNull(), static_cast<int64_t>(timestamp), duration });
     });
 }
@@ -290,7 +296,10 @@ void RemoteVideoEncoderCallbacks::notifyEncodedChunk(Vector<uint8_t>&& data, boo
             return;
 
         // FIXME: Remove from the map.
-        auto duration = protectedThis->m_timestampToDuration.get(timestamp + 1);
+        uint64_t duration = 0;
+        auto iterator = protectedThis->m_timestampToDuration.find(timestamp);
+        if (iterator != protectedThis->m_timestampToDuration.end())
+            duration = iterator->second;
         protectedThis->m_outputCallback({ WTFMove(data), isKeyFrame, static_cast<int64_t>(timestamp), duration });
     });
 }