Title: [291049] trunk
Revision
291049
Author
you...@apple.com
Date
2022-03-09 07:25:40 -0800 (Wed, 09 Mar 2022)

Log Message

WebRTC decoded frames are not correctly rotated in case GPU Process DOM rendering flag is set to true
https://bugs.webkit.org/show_bug.cgi?id=237468
<rdar://problem/89807876>

Reviewed by Eric Carlson.

Source/WebCore:

We were creating remote video frames at webrtc decoder level but at that level, we do not know yet the rotation and timestamps of the frame.
We need to set those values when the frame is exposed to RealtimeIncomingVideoSource.
Previous tests did not catch the regression as we were correctly computing the size of the video using the webrtc rotation and not the rotation from the frame.
We should probably migrate to RemoteVideoFrameProxy as a buffer wrapper so that we can easily create remote video frames from a RemoteVideoFrameProxy buffer.
In the meantime, we use const_cast in VideoFrame::initializeCharacteristics.

Covered by updated test.

* platform/VideoFrame.cpp:
* platform/VideoFrame.h:
* platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm:
* testing/Internals.cpp:
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

* webrtc/video-rotation.html:
Observe actual video frame rotation value.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (291048 => 291049)


--- trunk/LayoutTests/ChangeLog	2022-03-09 14:30:14 UTC (rev 291048)
+++ trunk/LayoutTests/ChangeLog	2022-03-09 15:25:40 UTC (rev 291049)
@@ -1,3 +1,14 @@
+2022-03-09  Youenn Fablet  <you...@apple.com>
+
+        WebRTC decoded frames are not correctly rotated in case GPU Process DOM rendering flag is set to true
+        https://bugs.webkit.org/show_bug.cgi?id=237468
+        <rdar://problem/89807876>
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/video-rotation.html:
+        Observe actual video frame rotation value.
+
 2022-03-08  Antoine Quint  <grao...@webkit.org>
 
         [web-animations] font-family should support discrete animation

Modified: trunk/LayoutTests/webrtc/video-rotation.html (291048 => 291049)


--- trunk/LayoutTests/webrtc/video-rotation.html	2022-03-09 14:30:14 UTC (rev 291048)
+++ trunk/LayoutTests/webrtc/video-rotation.html	2022-03-09 15:25:40 UTC (rev 291049)
@@ -50,6 +50,27 @@
     });
 }
 
+async function waitForTrackRotation(track, angle, counter)
+{
+    if (!window.internals)
+        return;
+
+    if (!counter) {
+        internals.observeMediaStreamTrack(track);
+        counter = 1;
+    }
+
+    await new Promise(resolve => setTimeout(resolve, 50));
+    const value = await internals.mediaStreamTrackVideoFrameRotation()
+    if (angle === value)
+        return;    
+
+    if (++counter > 50)
+        return Promise.reject("waitForTrackRotation timed out with value " + value + " while expecting " + angle);
+
+    return waitForTrackRotation(track, angle, counter);
+}
+
 var track;
 promise_test((test) => {
     if (window.testRunner)
@@ -62,8 +83,6 @@
 
             createConnections((firstConnection) => {
                 firstConnection.addTrack(track, localStream);
-                if (window.internals)
-                    internals.applyRotationForOutgoingVideoSources(firstConnection);
             }, (secondConnection) => {
                 secondConnection._ontrack_ = (trackEvent) => {
                     resolve(trackEvent.streams[0]);
@@ -95,26 +114,30 @@
     await waitForVideoSize(localVideo, 240, 320);
 }, "Track is enabled and rotated, local video should not be black and should change size");
 
-promise_test((test) => {
+promise_test(async (test) => {
     if (window.internals)
         window.internals.setCameraMediaStreamTrackOrientation(track, 90);
     if (window.testRunner)
         testRunner.setMockCameraOrientation(90);
 
-    return checkVideoBlack(false, remoteVideo, "canvas2").then(() => {
+    await checkVideoBlack(false, remoteVideo, "canvas2").then(() => {
         return waitForVideoSize(remoteVideo, 240, 320);
     });
+
+    await waitForTrackRotation(remoteVideo.srcObject.getVideoTracks()[0], 90);
 }, "Track is enabled and rotated, remote video should not be black and should change size");
 
-promise_test((test) => {
+promise_test(async (test) => {
     if (window.internals)
         window.internals.setCameraMediaStreamTrackOrientation(track, 180);
     if (window.testRunner)
         testRunner.setMockCameraOrientation(180);
 
-    return checkVideoBlack(false, remoteVideo, "canvas3").then(() => {
+    await checkVideoBlack(false, remoteVideo, "canvas3").then(() => {
         return waitForVideoSize(remoteVideo, 320, 240);
     });
+
+    await waitForTrackRotation(remoteVideo.srcObject.getVideoTracks()[0], 180);
 }, "Track is enabled and rotated again, video should not be black and should change size");
         </script>
     </body>

Modified: trunk/Source/WebCore/ChangeLog (291048 => 291049)


--- trunk/Source/WebCore/ChangeLog	2022-03-09 14:30:14 UTC (rev 291048)
+++ trunk/Source/WebCore/ChangeLog	2022-03-09 15:25:40 UTC (rev 291049)
@@ -1,3 +1,26 @@
+2022-03-09  Youenn Fablet  <you...@apple.com>
+
+        WebRTC decoded frames are not correctly rotated in case GPU Process DOM rendering flag is set to true
+        https://bugs.webkit.org/show_bug.cgi?id=237468
+        <rdar://problem/89807876>
+
+        Reviewed by Eric Carlson.
+
+        We were creating remote video frames at webrtc decoder level but at that level, we do not know yet the rotation and timestamps of the frame.
+        We need to set those values when the frame is exposed to RealtimeIncomingVideoSource.
+        Previous tests did not catch the regression as we were correctly computing the size of the video using the webrtc rotation and not the rotation from the frame.
+        We should probably migrate to RemoteVideoFrameProxy as a buffer wrapper so that we can easily create remote video frames from a RemoteVideoFrameProxy buffer.
+        In the meantime, we use const_cast in VideoFrame::initializeCharacteristics.
+
+        Covered by updated test.
+
+        * platform/VideoFrame.cpp:
+        * platform/VideoFrame.h:
+        * platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm:
+        * testing/Internals.cpp:
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2022-03-09  Fujii Hironori  <hironori.fu...@sony.com>
 
         [WinCairo] Improve WCTiledBacking and TextureMapperSparseBackingStore

Modified: trunk/Source/WebCore/platform/VideoFrame.cpp (291048 => 291049)


--- trunk/Source/WebCore/platform/VideoFrame.cpp	2022-03-09 14:30:14 UTC (rev 291048)
+++ trunk/Source/WebCore/platform/VideoFrame.cpp	2022-03-09 15:25:40 UTC (rev 291049)
@@ -112,6 +112,13 @@
 {
 }
 
+void VideoFrame::initializeCharacteristics(MediaTime presentationTime, bool isMirrored, VideoRotation rotation)
+{
+    const_cast<MediaTime&>(m_presentationTime) = presentationTime;
+    const_cast<bool&>(m_isMirrored) = isMirrored;
+    const_cast<VideoRotation&>(m_rotation) = rotation;
 }
 
+}
+
 #endif

Modified: trunk/Source/WebCore/platform/VideoFrame.h (291048 => 291049)


--- trunk/Source/WebCore/platform/VideoFrame.h	2022-03-09 14:30:14 UTC (rev 291048)
+++ trunk/Source/WebCore/platform/VideoFrame.h	2022-03-09 15:25:40 UTC (rev 291049)
@@ -57,6 +57,8 @@
     virtual RefPtr<WebCore::VideoFrameCV> asVideoFrameCV() = 0;
 #endif
 
+    void initializeCharacteristics(MediaTime presentationTime, bool isMirrored, VideoRotation);
+
 protected:
     WEBCORE_EXPORT VideoFrame(MediaTime presentationTime, bool isMirrored, VideoRotation);
     const MediaTime m_presentationTime;

Modified: trunk/Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm (291048 => 291049)


--- trunk/Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm	2022-03-09 14:30:14 UTC (rev 291048)
+++ trunk/Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm	2022-03-09 15:25:40 UTC (rev 291049)
@@ -130,7 +130,9 @@
 
     if (auto* provider = videoFrameBufferProvider(frame)) {
         // The only supported provider is VideoFrame.
-        return static_cast<VideoFrame*>(provider);
+        auto* videoFrame = static_cast<VideoFrame*>(provider);
+        videoFrame->initializeCharacteristics(MediaTime { frame.timestamp_us(), 1000000 }, false, rotation);
+        return videoFrame;
     }
 
     // In case of in memory samples, we have non interleaved YUV data while CVPixelBuffers prefer interleaved YUV data.

Modified: trunk/Source/WebCore/testing/Internals.cpp (291048 => 291049)


--- trunk/Source/WebCore/testing/Internals.cpp	2022-03-09 14:30:14 UTC (rev 291048)
+++ trunk/Source/WebCore/testing/Internals.cpp	2022-03-09 15:25:40 UTC (rev 291049)
@@ -5510,6 +5510,7 @@
 {
     stopObservingRealtimeMediaSource();
 
+    m_trackVideoRotation = -1;
     m_trackSource = &track.source();
     m_trackSource->addObserver(*this);
     switch (m_trackSource->type()) {
@@ -5532,6 +5533,11 @@
     m_nextTrackFramePromise = makeUnique<TrackFramePromise>(WTFMove(promise));
 }
 
+void Internals::mediaStreamTrackVideoFrameRotation(DOMPromiseDeferred<IDLShort>&& promise)
+{
+    promise.resolve(m_trackVideoRotation);
+}
+
 void Internals::videoSampleAvailable(MediaSample& sample, VideoSampleMetadata)
 {
     callOnMainThread([this, weakThis = WeakPtr { *this }, sample = Ref { sample }] {
@@ -5538,6 +5544,7 @@
         if (!weakThis)
             return;
         m_trackVideoSampleCount++;
+        m_trackVideoRotation = static_cast<int>(sample->videoRotation());
         if (!m_nextTrackFramePromise)
             return;
 

Modified: trunk/Source/WebCore/testing/Internals.h (291048 => 291049)


--- trunk/Source/WebCore/testing/Internals.h	2022-03-09 14:30:14 UTC (rev 291048)
+++ trunk/Source/WebCore/testing/Internals.h	2022-03-09 15:25:40 UTC (rev 291049)
@@ -873,6 +873,7 @@
     void observeMediaStreamTrack(MediaStreamTrack&);
     using TrackFramePromise = DOMPromiseDeferred<IDLInterface<ImageData>>;
     void grabNextMediaStreamTrackFrame(TrackFramePromise&&);
+    void mediaStreamTrackVideoFrameRotation(DOMPromiseDeferred<IDLShort>&&);
     void delayMediaStreamTrackSamples(MediaStreamTrack&, float);
     void setMediaStreamTrackMuted(MediaStreamTrack&, bool);
     void removeMediaStreamTrack(MediaStream&, MediaStreamTrack&);
@@ -1301,6 +1302,7 @@
     unsigned long m_trackAudioSampleCount { 0 };
     RefPtr<RealtimeMediaSource> m_trackSource;
     std::unique_ptr<TrackFramePromise> m_nextTrackFramePromise;
+    int m_trackVideoRotation { 0 };
 #endif
 #if ENABLE(MEDIA_SESSION)
     std::unique_ptr<ArtworkImageLoader> m_artworkLoader;

Modified: trunk/Source/WebCore/testing/Internals.idl (291048 => 291049)


--- trunk/Source/WebCore/testing/Internals.idl	2022-03-09 14:30:14 UTC (rev 291048)
+++ trunk/Source/WebCore/testing/Internals.idl	2022-03-09 15:25:40 UTC (rev 291049)
@@ -918,6 +918,7 @@
     [Conditional=MEDIA_STREAM] undefined setCameraMediaStreamTrackOrientation(MediaStreamTrack track, short orientation);
     [Conditional=MEDIA_STREAM] undefined observeMediaStreamTrack(MediaStreamTrack track);
     [Conditional=MEDIA_STREAM] Promise<ImageData> grabNextMediaStreamTrackFrame();
+    [Conditional=MEDIA_STREAM] Promise<short> mediaStreamTrackVideoFrameRotation();
     [Conditional=MEDIA_STREAM] readonly attribute unsigned long trackAudioSampleCount;
     [Conditional=MEDIA_STREAM] readonly attribute unsigned long trackVideoSampleCount;
     [Conditional=MEDIA_STREAM] undefined delayMediaStreamTrackSamples(MediaStreamTrack track, float delay);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to