Bug 1529581 - Propagate gecko timestamps for video into libwebrtc. r=bwc

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

dom/media/VideoFrameConverter.h

@@ -232,8 +232,16 @@ class VideoFrameConverter {
       return;
     }
 
+    // The same frame timer can in theory skip firing. Detect the number of
+    // seconds we're offset from the last frame. This floors to whole seconds.
+    // libwebrtc cannot handle timestamps in the future.
+    const webrtc::TimeDelta diff =
+        self->mTimestampMaker.GetNowRealtime() -
+        webrtc::Timestamp::Micros(self->mLastFrameConverted->timestamp_us());
+    const int64_t seconds = diff.us() / USECS_PER_S;
+    MOZ_ASSERT(seconds > 0);
     self->mLastFrameConverted->set_timestamp_us(
-        self->mTimestampMaker.GetNowRealtime().us());
+        self->mLastFrameConverted->timestamp_us() + (seconds * USECS_PER_S));
     for (RefPtr<VideoConverterListener>& listener : self->mListeners) {
       listener->OnVideoFrameConverted(*self->mLastFrameConverted);
     }
@@ -249,9 +257,13 @@ class VideoFrameConverter {
     const int sameFrameIntervalInMs = 1000;
     NS_NewTimerWithFuncCallback(
         getter_AddRefs(mSameFrameTimer), &SameFrameTick, this,
-        sameFrameIntervalInMs, nsITimer::TYPE_REPEATING_SLACK,
+        sameFrameIntervalInMs, nsITimer::TYPE_REPEATING_PRECISE_CAN_SKIP,
         "VideoFrameConverter::mSameFrameTimer", mTaskQueue);
 
+    // Check that time doesn't go backwards
+    MOZ_ASSERT_IF(mLastFrameConverted, aVideoFrame.timestamp_us() >
+                                           mLastFrameConverted->timestamp_us());
+
     mLastFrameConverted = MakeUnique<webrtc::VideoFrame>(aVideoFrame);
 
     for (RefPtr<VideoConverterListener>& listener : mListeners) {
@@ -308,9 +320,8 @@ class VideoFrameConverter {
       return;
     }
 
-    // See Bug 1529581 - Ideally we'd use the mTimestamp from the chunk
-    // passed into QueueVideoChunk rather than the webrtc.org clock here.
-    const webrtc::Timestamp now = mTimestampMaker.GetNowRealtime();
+    const webrtc::Timestamp time =
+        mTimestampMaker.ConvertMozTimeToRealtime(aFrame.mTime);
 
     if (aFrame.mForceBlack) {
       // Send a black image.
@@ -329,9 +340,10 @@ class VideoFrameConverter {
               ("Sending a black video frame"));
       webrtc::I420Buffer::SetBlack(buffer);
 
-      webrtc::VideoFrame frame(buffer, 0,  // not setting rtp timestamp
-                               now.ms(), webrtc::kVideoRotation_0);
-      VideoFrameConverted(frame);
+      VideoFrameConverted(webrtc::VideoFrame::Builder()
+                              .set_video_frame_buffer(buffer)
+                              .set_timestamp_us(time.us())
+                              .build());
       return;
     }
 
@@ -354,12 +366,12 @@ class VideoFrameConverter {
                 data->mCbCrStride, data->mCrChannel, data->mCbCrStride,
                 rtc::KeepRefUntilDone(image));
 
-        webrtc::VideoFrame i420_frame(video_frame_buffer,
-                                      0,  // not setting rtp timestamp
-                                      now.ms(), webrtc::kVideoRotation_0);
         MOZ_LOG(gVideoFrameConverterLog, LogLevel::Verbose,
                 ("Sending an I420 video frame"));
-        VideoFrameConverted(i420_frame);
+        VideoFrameConverted(webrtc::VideoFrame::Builder()
+                                .set_video_frame_buffer(video_frame_buffer)
+                                .set_timestamp_us(time.us())
+                                .build());
         return;
       }
     }
@@ -391,9 +403,10 @@ class VideoFrameConverter {
       return;
     }
 
-    webrtc::VideoFrame frame(buffer, 0,  // not setting rtp timestamp
-                             now.ms(), webrtc::kVideoRotation_0);
-    VideoFrameConverted(frame);
+    VideoFrameConverted(webrtc::VideoFrame::Builder()
+                            .set_video_frame_buffer(buffer)
+                            .set_timestamp_us(time.us())
+                            .build());
   }
 
   const dom::RTCStatsTimestampMaker mTimestampMaker;

dom/media/gtest/TestVideoFrameConverter.cpp

@@ -6,6 +6,7 @@
 #include <iterator>
 
 #include "gtest/gtest.h"
+#include "libwebrtcglue/SystemTime.h"
 #include "MediaEventSource.h"
 #include "VideoFrameConverter.h"
 #include "WaitFor.h"
@@ -31,12 +32,13 @@ class FrameListener : public VideoConverterListener {
 
 class VideoFrameConverterTest : public ::testing::Test {
  protected:
+  const dom::RTCStatsTimestampMaker mTimestampMaker;
   RefPtr<VideoFrameConverter> mConverter;
   RefPtr<FrameListener> mListener;
 
   VideoFrameConverterTest()
-      : mConverter(
-            MakeAndAddRef<VideoFrameConverter>(dom::RTCStatsTimestampMaker())),
+      : mTimestampMaker(dom::RTCStatsTimestampMaker()),
+        mConverter(MakeAndAddRef<VideoFrameConverter>(mTimestampMaker)),
         mListener(MakeAndAddRef<FrameListener>()) {
     mConverter->AddListener(mListener);
   }
@@ -172,13 +174,7 @@ TEST_F(VideoFrameConverterTest, Duplication) {
   EXPECT_FALSE(IsFrameBlack(frame1));
   EXPECT_GT(conversionTime1 - now,
             SameFrameTimeDuration() + TimeDuration::FromMilliseconds(100));
-  // Check that the second frame comes between 1s and 2s after the first.
-  EXPECT_GT(TimeDuration::FromMicroseconds(frame1.timestamp_us()) -
-                TimeDuration::FromMicroseconds(frame0.timestamp_us()),
-            SameFrameTimeDuration());
-  EXPECT_LT(TimeDuration::FromMicroseconds(frame1.timestamp_us()) -
-                TimeDuration::FromMicroseconds(frame0.timestamp_us()),
-            TimeDuration::FromSeconds(2));
+  EXPECT_EQ(frame1.timestamp_us() - frame0.timestamp_us(), USECS_PER_S);
 }
 
 TEST_F(VideoFrameConverterTest, DropsOld) {
@@ -228,10 +224,8 @@ TEST_F(VideoFrameConverterTest, BlackOnDisable) {
   EXPECT_EQ(frame1.height(), 480);
   EXPECT_TRUE(IsFrameBlack(frame1));
   EXPECT_GT(conversionTime1 - now, SameFrameTimeDuration());
-  // Check that the second frame comes between 1s and 2s after the first.
-  EXPECT_NEAR(frame1.timestamp_us(),
-              frame0.timestamp_us() + ((PR_USEC_PER_SEC * 3) / 2),
-              PR_USEC_PER_SEC / 2);
+  // Check that the second frame comes 1s after the first.
+  EXPECT_EQ(frame1.timestamp_us(), frame0.timestamp_us() + PR_USEC_PER_SEC);
 }
 
 TEST_F(VideoFrameConverterTest, ClearFutureFramesOnJumpingBack) {
@@ -310,3 +304,30 @@ TEST_F(VideoFrameConverterTest, NoConversionsWhileInactive) {
   EXPECT_EQ(frame.height(), 600);
   EXPECT_FALSE(IsFrameBlack(frame));
 }
+
+TEST_F(VideoFrameConverterTest, TimestampPropagation) {
+  auto framesPromise = TakeNConvertedFrames(2);
+  TimeStamp now = TimeStamp::Now();
+  TimeStamp t1 = now + TimeDuration::FromMilliseconds(1);
+  TimeStamp t2 = now + TimeDuration::FromMilliseconds(29);
+
+  mConverter->SetActive(true);
+  mConverter->QueueVideoChunk(GenerateChunk(640, 480, t1), false);
+  mConverter->QueueVideoChunk(GenerateChunk(800, 600, t2), false);
+
+  auto frames = WaitFor(framesPromise).unwrap();
+  ASSERT_EQ(frames.size(), 2U);
+  const auto& [frame0, conversionTime0] = frames[0];
+  EXPECT_EQ(frame0.width(), 640);
+  EXPECT_EQ(frame0.height(), 480);
+  EXPECT_FALSE(IsFrameBlack(frame0));
+  EXPECT_EQ(frame0.timestamp_us(),
+            mTimestampMaker.ConvertMozTimeToRealtime(t1).us());
+
+  const auto& [frame1, conversionTime1] = frames[1];
+  EXPECT_EQ(frame1.width(), 800);
+  EXPECT_EQ(frame1.height(), 600);
+  EXPECT_FALSE(IsFrameBlack(frame1));
+  EXPECT_EQ(frame1.timestamp_us(),
+            mTimestampMaker.ConvertMozTimeToRealtime(t2).us());
+}

dom/media/moz.build

@@ -333,6 +333,7 @@ LOCAL_INCLUDES += [
     "/caps",
     "/docshell/base",
     "/dom/base",
+    "/dom/media/webrtc",
     "/layout/generic",
     "/layout/xul",
     "/media/libyuv/libyuv/include",

dom/media/webrtc/jsapi/RTCStatsReport.cpp

@@ -77,13 +77,18 @@ DOMHighResTimeStamp RTCStatsTimestampMaker::ConvertNtpToDomTime(
   return ReduceRealtimePrecision(realtime - webrtc::TimeDelta::Micros(500));
 }
 
+webrtc::Timestamp RTCStatsTimestampMaker::ConvertMozTimeToRealtime(
+    TimeStamp aMozTime) const {
+  return webrtc::Timestamp::Micros(
+      (aMozTime - mStartRealtime).ToMicroseconds());
+}
+
 DOMHighResTimeStamp RTCStatsTimestampMaker::GetNow() const {
   return ReduceRealtimePrecision(GetNowRealtime());
 }
 
 webrtc::Timestamp RTCStatsTimestampMaker::GetNowRealtime() const {
-  return webrtc::Timestamp::Micros(
-      (TimeStamp::Now() - mStartRealtime).ToMicroseconds());
+  return ConvertMozTimeToRealtime(TimeStamp::Now());
 }
 
 NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(RTCStatsReport, mParent)

dom/media/webrtc/jsapi/RTCStatsReport.h

@@ -37,6 +37,8 @@ namespace dom {
  * and our integration with it.
  *
  * It converts and uses time from these clocks:
+ * - Moz      : Monotonic, unspecified (but constant) and inaccessible epoch, as
+ *              implemented by mozilla::TimeStamp
  * - Realtime : Monotonic, unspecified (but constant) epoch.
  * - 1Jan1970 : Monotonic, unix epoch (00:00:00 UTC on 1 January 1970).
  * - Ntp      : Monotonic, ntp epoch (00:00:00 UTC on 1 January 1900).
@@ -53,6 +55,7 @@ class RTCStatsTimestampMaker {
   DOMHighResTimeStamp GetNow() const;
 
   webrtc::Timestamp GetNowRealtime() const;
+  webrtc::Timestamp ConvertMozTimeToRealtime(TimeStamp aMozTime) const;
   webrtc::Timestamp ConvertRealtimeTo1Jan1970(
       webrtc::Timestamp aRealtime) const;
   DOMHighResTimeStamp ConvertNtpToDomTime(webrtc::Timestamp aNtpTime) const;