Cloned video MediaStreamTrack has another resolution than original track

https://bugs.webkit.org/show_bug.cgi?id=261329 rdar://115551680 Reviewed by Jean-Yves Avenard. When cloning a track, we are adding a new SourceProxy as obserer of the underlying source. This works fine for video tracks in case the SourceProxy is not resizing/reducing frame rate of the source via addVideoFrameObserver size/frame rate parameters. This resizing may happen for instance if the application wants a resolution that is not directly supported by the camera. Before the patch, the clone would register itself via addVideoFrameObserver without parameters, which would trigger no resizing/reducing frame rate. After the patch, we first copy the settings from the original source to the cloned source. We then call addVideoFrameObserver with the right parameters. Covered by added test. * LayoutTests/webrtc/video-clone-track-expected.txt: Added. * LayoutTests/webrtc/video-clone-track.html: Added. * Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp: (WebKit::UserMediaCaptureManagerProxy::SourceProxy::SourceProxy): (WebKit::UserMediaCaptureManagerProxy::SourceProxy::~SourceProxy): (WebKit::UserMediaCaptureManagerProxy::SourceProxy::observeMedia): (WebKit::UserMediaCaptureManagerProxy::createMediaSourceForCaptureDeviceWithConstraints): (WebKit::UserMediaCaptureManagerProxy::clone): Canonical link: https://commits.webkit.org/268307@main
WebKit · Sep 22, 2023 · a55b02b · a55b02b
1 parent 6a7d10f
commit a55b02b
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 6 deletions.
diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
@@ -1820,6 +1820,8 @@ fast/mediastream/video-rotation.html [ Skip ]
 webrtc/video-rotation-no-cvo.html [ Failure Timeout ]
 webrtc/video-rotation-black.html [ Crash Failure Pass Timeout ]
 
+webkit.org/b/261329 webrtc/video-clone-track.html [ Failure ]
+
 webkit.org/b/256758 fast/mediastream/captureStream/canvas3d.html [ Failure ]
 
 webkit.org/b/194611 http/wpt/webrtc/getUserMedia-processSwapping.html [ Failure ]

diff --git a/LayoutTests/webrtc/video-clone-track-expected.txt b/LayoutTests/webrtc/video-clone-track-expected.txt
@@ -0,0 +1,4 @@
+
+
+PASS Ensure cloned track gets the expected width and height
+
diff --git a/LayoutTests/webrtc/video-clone-track.html b/LayoutTests/webrtc/video-clone-track.html
@@ -0,0 +1,34 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <script src="../resources/testharness.js"></script>
+        <script src="../resources/testharnessreport.js"></script>
+    </head>
+    <body>
+        <video id="video" autoplay=""></video>
+        <script src ="routines.js"></script>
+        <script>
+promise_test(async (test) => {
+    const localStream = await navigator.mediaDevices.getUserMedia({video: {width:640, height:360 }});
+    const localStream2 = localStream.clone();
+    const stream = await new Promise((resolve, reject) => {
+        createConnections((firstConnection) => {
+            const sender = firstConnection.addTrack(localStream2.getVideoTracks()[0], localStream2);
+        }, (secondConnection) => {
+            secondConnection.ontrack = (trackEvent) => {
+                resolve(trackEvent.streams[0]);
+            };
+        });
+        setTimeout(() => reject("Test timed out"), 5000);
+    });
+
+    video.srcObject = stream;
+    await video.play();
+
+    assert_equals(video.videoWidth, 640);
+    assert_equals(video.videoHeight, 360);
+}, "Ensure cloned track gets the expected width and height");
+        </script>
+    </body>
+</html>
diff --git a/Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp b/Source/WebKit/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp
@@ -65,27 +65,34 @@ class UserMediaCaptureManagerProxy::SourceProxy
         , m_videoFrameObjectHeap(WTFMove(videoFrameObjectHeap))
     {
         m_source->addObserver(*this);
+    }
+
+    ~SourceProxy()
+    {
         switch (m_source->type()) {
         case RealtimeMediaSource::Type::Audio:
-            m_source->addAudioSampleObserver(*this);
+            m_source->removeAudioSampleObserver(*this);
             break;
         case RealtimeMediaSource::Type::Video:
-            m_source->addVideoFrameObserver(*this);
+            m_source->removeVideoFrameObserver(*this);
             break;
         }
+        m_source->removeObserver(*this);
     }
 
-    ~SourceProxy()
+    void observeMedia()
     {
         switch (m_source->type()) {
         case RealtimeMediaSource::Type::Audio:
-            m_source->removeAudioSampleObserver(*this);
+            m_source->addAudioSampleObserver(*this);
             break;
         case RealtimeMediaSource::Type::Video:
-            m_source->removeVideoFrameObserver(*this);
+            if (m_widthConstraint || m_heightConstraint || m_frameRateConstraint)
+                m_source->addVideoFrameObserver(*this, { m_widthConstraint, m_heightConstraint }, m_frameRateConstraint);
+            else
+                m_source->addVideoFrameObserver(*this);
             break;
         }
-        m_source->removeObserver(*this);
     }
 
     RealtimeMediaSource& source() { return m_source; }
@@ -427,6 +434,7 @@ void UserMediaCaptureManagerProxy::createMediaSourceForCaptureDeviceWithConstrai
     Ref connection = m_connectionProxy->connection();
     RefPtr remoteVideoFrameObjectHeap = shouldUseGPUProcessRemoteFrames ? m_connectionProxy->remoteVideoFrameObjectHeap() : nullptr;
     auto proxy = makeUnique<SourceProxy>(id, WTFMove(connection), ProcessIdentity { m_connectionProxy->resourceOwner() }, WTFMove(source), WTFMove(remoteVideoFrameObjectHeap));
+    proxy->observeMedia();
 
     if (constraints && proxy->source().type() == RealtimeMediaSource::Type::Video) {
         if (auto result = proxy->applyConstraints(*constraints)) {
@@ -508,6 +516,7 @@ void UserMediaCaptureManagerProxy::clone(RealtimeMediaSourceIdentifier clonedID,
         RefPtr remoteVideoFrameObjectHeap = m_connectionProxy->remoteVideoFrameObjectHeap();
         auto cloneProxy = makeUnique<SourceProxy>(newSourceID, WTFMove(connection), ProcessIdentity { m_connectionProxy->resourceOwner() }, WTFMove(sourceClone), WTFMove(remoteVideoFrameObjectHeap));
         cloneProxy->copySettings(*proxy);
+        cloneProxy->observeMedia();
         m_proxies.add(newSourceID, WTFMove(cloneProxy));
     }
 }