Bug 1652884 - Make disabled tracks, that haven't seen a frame, black. r=jib

This affects HTMLMediaElement, MediaRecorder and RTCPeerConnection, which are
the implementors of NotifyEnabledStateChanged.

This use case is going to become more common as users will be able to globally
mute cameras before requesting one. While muted, the camera is off and no frames
will flow. The old logic for showing disabled video tracks as black relied on a
frame appearing that could be turned black.

With this patch in this case we will create a frame if none has been seen yet,
and it will have a hardcoded size.

Depends on D87127

Differential Revision: https://phabricator.services.mozilla.com/D87128

dom/media/VideoFrameConverter.h

@@ -139,14 +139,20 @@ class VideoFrameConverter {
                   ("VideoFrameConverter Track is now %s",
                    aTrackEnabled ? "enabled" : "disabled"));
           mTrackEnabled = aTrackEnabled;
-          if (!aTrackEnabled && mLastFrameConverted) {
-            // After disabling, we re-send the last frame as black in case the
-            // source had already stopped and no frame is coming soon.
-            ProcessVideoFrame(
-                FrameToProcess{nullptr, TimeStamp::Now(),
-                               gfx::IntSize(mLastFrameConverted->width(),
-                                            mLastFrameConverted->height()),
-                               true});
+          if (!aTrackEnabled) {
+            // After disabling we immediately send a frame as black, so it can
+            // be seen quickly, even if no frames are flowing.
+            if (mLastFrameQueuedForProcessing.Serial() != -2) {
+              // This track has already seen a frame so we re-send the last one
+              // queued as black.
+              FrameToProcess f = mLastFrameQueuedForProcessing;
+              f.mTime = TimeStamp::Now();
+              ProcessVideoFrame(f);
+            } else {
+              // This track has not yet seen any frame. We make one up.
+              QueueForProcessing(nullptr, TimeStamp::Now(),
+                                 gfx::IntSize(640, 480), true);
+            }
           }
         }));
     MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));

dom/media/VideoOutput.h

@@ -189,7 +189,9 @@ class VideoOutput : public DirectMediaTrackListener {
     if (!mEnabled || mFrames.Length() > 1) {
       // Re-send frames when disabling, as new frames may not arrive. When
       // enabling we keep them black until new frames arrive, or re-send if we
-      // already have frames in the future.
+      // already have frames in the future. If we're disabling and there are no
+      // frames available yet, we invent one. Unfortunately with a hardcoded
+      // size.
       //
       // Since mEnabled will affect whether
       // frames are real, or black, we assign new FrameIDs whenever we re-send
@@ -197,6 +199,13 @@ class VideoOutput : public DirectMediaTrackListener {
       for (auto& idChunkPair : mFrames) {
         idChunkPair.first = mVideoFrameContainer->NewFrameID();
       }
+      if (mFrames.IsEmpty()) {
+        VideoSegment v;
+        v.AppendFrame(nullptr, gfx::IntSize(640, 480), PRINCIPAL_HANDLE_NONE,
+                      true, TimeStamp::Now());
+        mFrames.AppendElement(std::make_pair(mVideoFrameContainer->NewFrameID(),
+                                             *v.GetLastChunk()));
+      }
       SendFramesEnsureLocked();
     }
   }

dom/media/gtest/TestVideoFrameConverter.cpp

@@ -175,15 +175,16 @@ TEST_F(VideoFrameConverterTest, BlackOnDisable) {
   mConverter->QueueVideoChunk(GenerateChunk(640, 480, future2), false);
   mConverter->QueueVideoChunk(GenerateChunk(640, 480, future3), false);
   auto frames = WaitForNConverted(2);
-  EXPECT_GT(TimeStamp::Now() - now, TimeDuration::FromMilliseconds(1100));
+  EXPECT_GT(TimeStamp::Now() - now, TimeDuration::FromSeconds(1));
   ASSERT_EQ(frames.size(), 2U);
+  // The first frame was created instantly by SetTrackEnabled().
   EXPECT_EQ(frames[0].first.width(), 640);
   EXPECT_EQ(frames[0].first.height(), 480);
-  EXPECT_GT(frames[0].second - now, future1 - now);
+  EXPECT_GT(frames[0].second - now, TimeDuration::FromSeconds(0));
+  // The second frame was created by the same-frame timer (after 1s).
   EXPECT_EQ(frames[1].first.width(), 640);
   EXPECT_EQ(frames[1].first.height(), 480);
-  EXPECT_GT(frames[1].second - now,
-            future1 - now + TimeDuration::FromSeconds(1));
+  EXPECT_GT(frames[1].second - now, TimeDuration::FromSeconds(1));
   // Check that the second frame comes between 1s and 2s after the first.
   EXPECT_NEAR(frames[1].first.timestamp_us(),
               frames[0].first.timestamp_us() + ((PR_USEC_PER_SEC * 3) / 2),

dom/media/gtest/TestVideoTrackEncoder.cpp

@@ -1279,6 +1279,46 @@ TEST(VP8VideoTrackEncoder, DisableBetweenFrames)
   EXPECT_EQ(PR_USEC_PER_SEC / 1000 * 100UL, frames[2]->mDuration);
 }
 
+// Test that an encoding which is disabled before the first frame becomes black
+// immediately.
+TEST(VP8VideoTrackEncoder, DisableBeforeFirstFrame)
+{
+  TestVP8TrackEncoder encoder;
+  YUVBufferGenerator generator;
+  generator.Init(mozilla::gfx::IntSize(640, 480));
+  nsTArray<RefPtr<EncodedFrame>> frames;
+  TimeStamp now = TimeStamp::Now();
+
+  // Disable the track at t=0.
+  // Pass a frame in at t=50ms.
+  // Enable the track at t=100ms.
+  // Stop encoding at t=200ms.
+  // Should yield 2 frames, 1 black [0, 100); 1 real [100, 200).
+
+  VideoSegment segment;
+  segment.AppendFrame(generator.GenerateI420Image(), generator.GetSize(),
+                      PRINCIPAL_HANDLE_NONE, false,
+                      now + TimeDuration::FromMilliseconds(50));
+
+  encoder.SetStartOffset(now);
+  encoder.Disable(now);
+  encoder.AppendVideoSegment(std::move(segment));
+
+  encoder.Enable(now + TimeDuration::FromMilliseconds(100));
+  encoder.AdvanceCurrentTime(now + TimeDuration::FromMilliseconds(200));
+  encoder.NotifyEndOfStream();
+  ASSERT_TRUE(NS_SUCCEEDED(encoder.GetEncodedTrack(frames)));
+  EXPECT_TRUE(encoder.IsEncodingComplete());
+
+  ASSERT_EQ(2UL, frames.Length());
+
+  // [0, 100ms)
+  EXPECT_EQ(PR_USEC_PER_SEC / 1000 * 100UL, frames[0]->mDuration);
+
+  // [100ms, 200ms)
+  EXPECT_EQ(PR_USEC_PER_SEC / 1000 * 100UL, frames[1]->mDuration);
+}
+
 // Test that an encoding which is enabled on a frame timestamp encodes
 // frames as expected.
 TEST(VP8VideoTrackEncoder, EnableOnFrameTime)