Skip to content

Commit

Permalink
RTCRtpSender maxFramerate encoding parameter has no effect
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259271
rdar://problem/112397603

Reviewed by Eric Carlson.

React to AddOrUpdateSink max_frame_rate_bps signal to enforce maxFramerate by decimating at the source level.
Update RemoteRealtimeVideoSource::remoteVideoFrameAvailable to compute the observed frame rate so that decimation can happen in WebProcess.
Update RealtimeOutgoingVideoSource to call addVideoFrameObserve with specified frame rate.

* LayoutTests/webrtc/video-maxFramerate-expected.txt: Added.
* LayoutTests/webrtc/video-maxFramerate.html: Added.
* Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp:
(WebCore::RealtimeOutgoingVideoSource::startObservingVideoFrames):
(WebCore::RealtimeOutgoingVideoSource::updateBlackFramesSending):
(WebCore::RealtimeOutgoingVideoSource::AddOrUpdateSink):
* Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:
* Source/WebKit/WebProcess/cocoa/RemoteRealtimeVideoSource.cpp:
(WebKit::RemoteRealtimeVideoSource::remoteVideoFrameAvailable):
* Source/WebKit/WebProcess/cocoa/RemoteRealtimeVideoSource.h:

Canonical link: https://commits.webkit.org/266128@main
  • Loading branch information
youennf committed Jul 18, 2023
1 parent 4b27f17 commit 3f6cf3b
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 2 deletions.
1 change: 1 addition & 0 deletions LayoutTests/platform/glib/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1722,6 +1722,7 @@ webkit.org/b/235885 webrtc/datachannel/getStats-no-prflx-remote-candidate.html [
webkit.org/b/235885 fast/mediastream/RTCPeerConnection-statsSelector.html [ Skip ]

webrtc/video-av1.html [ Skip ]
webrtc/video-maxFramerate.html [ Failure ]

# GStreamer's DTLS agent currently generates RSA certificates only. DTLS 1.2 is not supported yet (AFAIK).
webrtc/datachannel/dtls10.html [ Failure ]
Expand Down
4 changes: 4 additions & 0 deletions LayoutTests/webrtc/video-maxFramerate-expected.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@


PASS Testing that maxFramerate has an effect

57 changes: 57 additions & 0 deletions LayoutTests/webrtc/video-maxFramerate.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Testing macFramerate</title>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
</head>
<body>
<video id="video" autoplay=""></video>
<canvas id="canvas" width="640" height="480"></canvas>
<script src ="routines.js"></script>
<script>
var pc1, pc2;
promise_test(async (test) => {
const localStream = await navigator.mediaDevices.getUserMedia({video:true});
const stream = await new Promise((resolve, reject) => {
createConnections((firstConnection) => {
pc1 = firstConnection;
const sender = firstConnection.addTrack(localStream.getVideoTracks()[0], localStream);
}, (secondConnection) => {
pc2 = secondConnection;
secondConnection.ontrack = (trackEvent) => {
resolve(trackEvent.streams[0]);
};
});
setTimeout(() => reject("Test timed out"), 5000);
});

video.srcObject = stream;
await video.play();

let settings = stream.getVideoTracks()[0].getSettings();
assert_equals(settings.width, 640, "remote track settings width");
assert_equals(settings.height, 480, "remote track settings height");

let counter = 0;
while (++counter < 100 && settings.frameRate < 5) {
await new Promise(resolve => setTimeout(resolve, 50));
settings = stream.getVideoTracks()[0].getSettings();
}
assert_greater_than(settings.frameRate, 5, "remote track settings frame rate");

const parameters = pc1.getSenders()[0].getParameters();
parameters.encodings[0].maxFramerate = 1;
await pc1.getSenders()[0].setParameters(parameters);

counter = 0;
while (++counter < 100 && settings.frameRate > 4) {
await new Promise(resolve => setTimeout(resolve, 50));
settings = stream.getVideoTracks()[0].getSettings();
}
assert_less_than(settings.frameRate, 4, "remote track settings reduced frame rate");
}, "Testing that maxFramerate has an effect");
</script>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ void RealtimeOutgoingVideoSource::unobserveSource()
m_videoSource->source().removeVideoFrameObserver(*this);
}

void RealtimeOutgoingVideoSource::startObservingVideoFrames()
{
if (m_maxFrameRate) {
m_videoSource->source().addVideoFrameObserver(*this, { }, *m_maxFrameRate);
return;
}
m_videoSource->source().addVideoFrameObserver(*this);
}

void RealtimeOutgoingVideoSource::setSource(Ref<MediaStreamTrackPrivate>&& newSource)
{
ASSERT(isMainThread());
Expand Down Expand Up @@ -114,13 +123,19 @@ void RealtimeOutgoingVideoSource::stop()
void RealtimeOutgoingVideoSource::updateBlackFramesSending()
{
if (!m_muted && m_enabled) {
m_videoSource->source().addVideoFrameObserver(*this);
if (!m_isObservingVideoFrames) {
m_isObservingVideoFrames = true;
startObservingVideoFrames();
}
if (m_blackFrameTimer.isActive())
m_blackFrameTimer.stop();
return;
}

m_videoSource->source().removeVideoFrameObserver(*this);
if (m_isObservingVideoFrames) {
m_isObservingVideoFrames = false;
m_videoSource->source().removeVideoFrameObserver(*this);
}
sendBlackFramesIfNeeded();
}

Expand Down Expand Up @@ -161,6 +176,20 @@ void RealtimeOutgoingVideoSource::AddOrUpdateSink(rtc::VideoSinkInterface<webrtc
if (sinkWants.rotation_applied)
applyRotation();

std::optional<double> maxFrameRate;
if (sinkWants.max_framerate_fps != std::numeric_limits<int>::max())
maxFrameRate = sinkWants.max_framerate_fps;
ensureOnMainThread([this, protectedThis = Ref { *this }, maxFrameRate] {
if (m_maxFrameRate == maxFrameRate)
return;
m_maxFrameRate = maxFrameRate;
if (!m_isObservingVideoFrames)
return;
m_videoSource->source().removeVideoFrameObserver(*this);
m_isObservingVideoFrames = false;
updateBlackFramesSending();
});

Locker locker { m_sinksLock };
m_sinks.add(sink);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class RealtimeOutgoingVideoSource

void sourceMutedChanged();
void sourceEnabledChanged();
void startObservingVideoFrames();

// MediaStreamTrackPrivate::Observer API
void trackMutedChanged(MediaStreamTrackPrivate&) final { sourceMutedChanged(); }
Expand All @@ -152,6 +153,8 @@ class RealtimeOutgoingVideoSource
bool m_muted { false };
uint32_t m_width { 0 };
uint32_t m_height { 0 };
std::optional<double> m_maxFrameRate;
bool m_isObservingVideoFrames { false };

#if !RELEASE_LOG_DISABLED
Ref<const Logger> m_logger;
Expand Down
12 changes: 12 additions & 0 deletions Source/WebKit/WebProcess/cocoa/RemoteRealtimeVideoSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,18 @@ Ref<RealtimeMediaSource> RemoteRealtimeVideoSource::clone()

void RemoteRealtimeVideoSource::remoteVideoFrameAvailable(VideoFrame& frame, VideoFrameTimeMetadata metadata)
{
MediaTime sampleTime = frame.presentationTime();

auto frameTime = sampleTime.toDouble();
m_observedFrameTimeStamps.append(frameTime);
m_observedFrameTimeStamps.removeAllMatching([&](auto time) {
return time <= frameTime - 2;
});

auto interval = m_observedFrameTimeStamps.last() - m_observedFrameTimeStamps.first();
if (interval > 1)
m_observedFrameRate = (m_observedFrameTimeStamps.size() / interval);

setIntrinsicSize(expandedIntSize(frame.presentationSize()));
videoFrameAvailable(frame, metadata);
}
Expand Down
4 changes: 4 additions & 0 deletions Source/WebKit/WebProcess/cocoa/RemoteRealtimeVideoSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ class RemoteRealtimeVideoSource final : public RemoteRealtimeMediaSource {
Ref<RealtimeMediaSource> clone() final;
void endProducingData() final;
bool setShouldApplyRotation(bool) final;
double observedFrameRate() const final { return m_observedFrameRate; }

Deque<double> m_observedFrameTimeStamps;
double m_observedFrameRate { 0 };
};

} // namespace WebKit
Expand Down

0 comments on commit 3f6cf3b

Please sign in to comment.