From 2b3041306738fd64191ab514e6d0f42990a3ce83 Mon Sep 17 00:00:00 2001 From: Andreas Pehrson Date: Tue, 2 Nov 2021 14:35:59 +0000 Subject: [PATCH] Bug 1529581 - Propagate gecko timestamps for video into libwebrtc. r=bwc Differential Revision: https://phabricator.services.mozilla.com/D128249 --- dom/media/VideoFrameConverter.h | 43 ++++++++++++------- dom/media/gtest/TestVideoFrameConverter.cpp | 47 +++++++++++++++------ dom/media/moz.build | 1 + dom/media/webrtc/jsapi/RTCStatsReport.cpp | 9 +++- dom/media/webrtc/jsapi/RTCStatsReport.h | 3 ++ 5 files changed, 73 insertions(+), 30 deletions(-) diff --git a/dom/media/VideoFrameConverter.h b/dom/media/VideoFrameConverter.h index 5f141d90676e..6072b66117ea 100644 --- a/dom/media/VideoFrameConverter.h +++ b/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& 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(aVideoFrame); for (RefPtr& 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; diff --git a/dom/media/gtest/TestVideoFrameConverter.cpp b/dom/media/gtest/TestVideoFrameConverter.cpp index 585cf66a3bb6..70d4388965de 100644 --- a/dom/media/gtest/TestVideoFrameConverter.cpp +++ b/dom/media/gtest/TestVideoFrameConverter.cpp @@ -6,6 +6,7 @@ #include #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 mConverter; RefPtr mListener; VideoFrameConverterTest() - : mConverter( - MakeAndAddRef(dom::RTCStatsTimestampMaker())), + : mTimestampMaker(dom::RTCStatsTimestampMaker()), + mConverter(MakeAndAddRef(mTimestampMaker)), mListener(MakeAndAddRef()) { 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()); +} diff --git a/dom/media/moz.build b/dom/media/moz.build index e6862fad0c74..156f19101658 100644 --- a/dom/media/moz.build +++ b/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", diff --git a/dom/media/webrtc/jsapi/RTCStatsReport.cpp b/dom/media/webrtc/jsapi/RTCStatsReport.cpp index b7470d253726..a3c20c73365c 100644 --- a/dom/media/webrtc/jsapi/RTCStatsReport.cpp +++ b/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) diff --git a/dom/media/webrtc/jsapi/RTCStatsReport.h b/dom/media/webrtc/jsapi/RTCStatsReport.h index 3942b9e0a0cc..ba2ada36c03e 100644 --- a/dom/media/webrtc/jsapi/RTCStatsReport.h +++ b/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;