nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1611957: Make EncodingConstraints.maxFps a Maybe instead of having 0 represent no limit. r=ng

Browse source 
0 is a valid limit in setParameters according to the spec. For now, we treat
max-fr=0 in SDP as no limit, since that is what we have been doing.

Differential Revision: https://phabricator.services.mozilla.com/D144009
This commit is contained in:
Byron Campen 2022-04-26 14:07:37 +00:00
parent 97079204ac
commit 288124e4a5
9 changed files with 73 additions and 43 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

3
dom/media/webrtc/common/EncodingConstraints.h
View file

@ -15,7 +15,6 @@ class EncodingConstraints {
EncodingConstraints()
: maxWidth(0),
maxHeight(0),
maxFps(0),
maxFs(0),
maxBr(0),
maxPps(0),
@ -44,7 +43,7 @@ class EncodingConstraints {
uint32_t maxWidth;
uint32_t maxHeight;
uint32_t maxFps;
Maybe<double> maxFps;
uint32_t maxFs;
uint32_t maxBr;
uint32_t maxPps;

2
dom/media/webrtc/jsapi/PeerConnectionImpl.cpp
View file

@ -685,7 +685,7 @@ class ConfigureCodec {
}
}
videoCodec.mConstraints.maxFs = mVP8MaxFs;
videoCodec.mConstraints.maxFps = mVP8MaxFr;
videoCodec.mConstraints.maxFps = Some(mVP8MaxFr);
}
if (mUseTmmbr) {

18
dom/media/webrtc/jsep/JsepCodecDescription.h
View file

@ -5,6 +5,7 @@
#ifndef _JSEPCODECDESCRIPTION_H_
#define _JSEPCODECDESCRIPTION_H_
#include <cmath>
#include <string>
#include "sdp/SdpMediaSection.h"
#include "sdp/SdpHelper.h"
@ -369,7 +370,7 @@ class JsepVideoCodecDescription : public JsepCodecDescription {
auto codec = MakeUnique<JsepVideoCodecDescription>("120", "VP8", 90000);
// Defaults for mandatory params
codec->mConstraints.maxFs = 12288; // Enough for 2048x1536
codec->mConstraints.maxFps = 60;
codec->mConstraints.maxFps = Some(60);
if (aUseRtx) {
codec->EnableRtx("124");
}
@ -380,7 +381,7 @@ class JsepVideoCodecDescription : public JsepCodecDescription {
auto codec = MakeUnique<JsepVideoCodecDescription>("121", "VP9", 90000);
// Defaults for mandatory params
codec->mConstraints.maxFs = 12288; // Enough for 2048x1536
codec->mConstraints.maxFps = 60;
codec->mConstraints.maxFps = Some(60);
if (aUseRtx) {
codec->EnableRtx("125");
}
@ -526,7 +527,12 @@ class JsepVideoCodecDescription : public JsepCodecDescription {
GetVP8Parameters(mDefaultPt, msection));
vp8Params.max_fs = mConstraints.maxFs;
vp8Params.max_fr = mConstraints.maxFps;
if (mConstraints.maxFps.isSome()) {
vp8Params.max_fr =
static_cast<unsigned int>(std::round(*mConstraints.maxFps));
} else {
vp8Params.max_fr = 60;
}
msection.SetFmtp(SdpFmtpAttributeList::Fmtp(mDefaultPt, vp8Params));
}
}
@ -736,7 +742,11 @@ class JsepVideoCodecDescription : public JsepCodecDescription {
GetVP8Parameters(mDefaultPt, remoteMsection));
mConstraints.maxFs = vp8Params.max_fs;
mConstraints.maxFps = vp8Params.max_fr;
// Right now, we treat max-fr=0 (or the absence of max-fr) as no limit.
// We will eventually want to stop doing this (bug 1762600).
if (vp8Params.max_fr) {
mConstraints.maxFps = Some(vp8Params.max_fr);
}
}
}

29
dom/media/webrtc/libwebrtcglue/VideoConduit.cpp
View file

@ -139,8 +139,12 @@ unsigned int SelectSendFrameRate(const VideoCodecConfig& codecConfig,
if (cur_fs > 0) { // in case no frames have been sent
new_framerate = codecConfig.mEncodingConstraints.maxMbps / cur_fs;
new_framerate =
MinIgnoreZero(new_framerate, codecConfig.mEncodingConstraints.maxFps);
if (codecConfig.mEncodingConstraints.maxFps.isSome()) {
// libwebrtc does not handle a max framerate of 0 at all
new_framerate = std::min(
new_framerate, static_cast<unsigned int>(std::round(
*codecConfig.mEncodingConstraints.maxFps)));
}
}
}
return new_framerate;
@ -676,10 +680,19 @@ void WebrtcVideoConduit::OnControlConfigChange() {
this, streamCount);
{
// libwebrtc does not handle non-integer max framerate.
Maybe<unsigned> negotiatedMaxFps;
if (codecConfig->mEncodingConstraints.maxFps.isSome()) {
unsigned integerMaxFps = static_cast<unsigned int>(
std::round(*codecConfig->mEncodingConstraints.maxFps));
// libwebrtc does not handle a max framerate of 0, even though the
// spec says this is valid. So, we need to treat 0 as no limit.
if (integerMaxFps) {
negotiatedMaxFps = Some(integerMaxFps);
}
}
const unsigned max_framerate =
codecConfig->mEncodingConstraints.maxFps > 0
? codecConfig->mEncodingConstraints.maxFps
: DEFAULT_VIDEO_MAX_FRAMERATE;
negotiatedMaxFps.refOr(DEFAULT_VIDEO_MAX_FRAMERATE);
// apply restrictions from maxMbps/etc
mSendingFramerate = SelectSendFrameRate(*codecConfig, max_framerate,
mLastWidth, mLastHeight);
@ -1821,8 +1834,10 @@ void WebrtcVideoConduit::DumpCodecDB() const {
CSFLogDebug(LOGTAG, "Payload Type: %d", entry.mType);
CSFLogDebug(LOGTAG, "Payload Max Frame Size: %d",
entry.mEncodingConstraints.maxFs);
CSFLogDebug(LOGTAG, "Payload Max Frame Rate: %d",
entry.mEncodingConstraints.maxFps);
if (entry.mEncodingConstraints.maxFps.isSome()) {
CSFLogDebug(LOGTAG, "Payload Max Frame Rate: %f",
*entry.mEncodingConstraints.maxFps);
}
}
}

7
dom/media/webrtc/sdp/RsdparsaSdpAttributeList.cpp
View file

@ -1188,7 +1188,12 @@ void RsdparsaSdpAttributeList::LoadRids(RustAttributeList* attributeList) {
EncodingConstraints parameters;
parameters.maxWidth = rid.params.max_width;
parameters.maxHeight = rid.params.max_height;
parameters.maxFps = rid.params.max_fps;
// Right now, we treat max-fps=0 and the absence of max-fps as no limit.
// We will eventually want to treat max-fps=0 as 0 frames per second, and
// the absence of max-fps as no limit (bug 1762632).
if (rid.params.max_fps) {
parameters.maxFps = Some(rid.params.max_fps);
}
parameters.maxFs = rid.params.max_fs;
parameters.maxBr = rid.params.max_br;
parameters.maxPps = rid.params.max_pps;

5
dom/media/webrtc/sdp/SdpAttribute.cpp
View file

@ -813,10 +813,11 @@ bool SdpRidAttributeList::Rid::ParseParameters(std::istream& is,
return false;
}
} else if (key == "max-fps") {
if (!GetUnsigned<uint32_t>(is, 0, UINT32_MAX, &constraints.maxFps,
error)) {
uint32_t maxFps;
if (!GetUnsigned<uint32_t>(is, 0, UINT32_MAX, &maxFps, error)) {
return false;
}
constraints.maxFps = Some(maxFps);
} else if (key == "max-fs") {
if (!GetUnsigned<uint32_t>(is, 0, UINT32_MAX, &constraints.maxFs,
error)) {

4
media/webrtc/signaling/gtest/jsep_track_unittest.cpp
View file

@ -1695,7 +1695,7 @@ TEST_F(JsepTrackTest, NonDefaultVideoSdpFmtpLine) {
auto* video = static_cast<JsepVideoCodecDescription*>(codec.get());
video->mConstraints.maxFs = 1200;
if (codec->mName == "VP8") {
video->mConstraints.maxFps = 15;
video->mConstraints.maxFps = Some(15);
} else {
video->mConstraints.maxDpb = 6400;
video->mConstraints.maxBr = 1000;
@ -1710,7 +1710,7 @@ TEST_F(JsepTrackTest, NonDefaultVideoSdpFmtpLine) {
auto* video = static_cast<JsepVideoCodecDescription*>(codec.get());
video->mConstraints.maxFs = 32400;
if (codec->mName == "VP8") {
video->mConstraints.maxFps = 60;
video->mConstraints.maxFps = Some(60);
} else {
video->mConstraints.maxMbps = 1944000;
video->mConstraints.maxCpb = 800000;

45
media/webrtc/signaling/gtest/sdp_unittests.cpp
View file

@ -5132,7 +5132,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5147,7 +5147,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(96U, rid.formats[0]);
ASSERT_EQ(800U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5164,7 +5164,8 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(98U, rid.formats[2]);
ASSERT_EQ(800U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5178,7 +5179,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(800U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5190,7 +5191,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5203,7 +5204,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(96U, rid.formats[0]);
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5219,7 +5220,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(96U, rid.formats[0]);
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(30000U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5234,7 +5235,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(98U, rid.formats[2]);
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5246,7 +5247,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(800U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5258,7 +5259,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(640U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5270,7 +5271,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(30U, rid.constraints.maxFps);
ASSERT_EQ(30.0, *rid.constraints.maxFps);
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5282,7 +5283,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(3600U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5294,7 +5295,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(30000U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5306,7 +5307,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(9216000U, rid.constraints.maxPps);
@ -5318,7 +5319,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5331,7 +5332,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5343,7 +5344,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(0U, rid.constraints.maxWidth);
ASSERT_EQ(0U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5358,7 +5359,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(800U, rid.constraints.maxWidth);
ASSERT_EQ(600U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5373,7 +5374,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(97U, rid.formats[1]);
ASSERT_EQ(800U, rid.constraints.maxWidth);
ASSERT_EQ(600U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5386,7 +5387,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(800U, rid.constraints.maxWidth);
ASSERT_EQ(600U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5401,7 +5402,7 @@ TEST(NewSdpTestNoFixture, CheckRidValidParse)
ASSERT_EQ(0U, rid.formats.size());
ASSERT_EQ(800U, rid.constraints.maxWidth);
ASSERT_EQ(600U, rid.constraints.maxHeight);
ASSERT_EQ(0U, rid.constraints.maxFps);
ASSERT_FALSE(rid.constraints.maxFps.isSome());
ASSERT_EQ(0U, rid.constraints.maxFs);
ASSERT_EQ(0U, rid.constraints.maxBr);
ASSERT_EQ(0U, rid.constraints.maxPps);
@ -5502,7 +5503,7 @@ TEST(NewSdpTestNoFixture, CheckRidSerialize)
SdpRidAttributeList::Rid rid;
rid.id = "foo";
rid.direction = sdp::kSend;
rid.constraints.maxFps = 30;
rid.constraints.maxFps = Some(30);
std::ostringstream os;
rid.Serialize(os);
ASSERT_EQ("foo send max-fps=30", os.str());

3
media/webrtc/signaling/gtest/videoconduit_unittests.cpp
View file

@ -352,7 +352,6 @@ TEST_F(VideoConduitTest, TestConfigureSendMediaCodecMaxFps) {
mControl.Update([&](auto& aControl) {
aControl.mTransmitting = true;
EncodingConstraints constraints;
constraints.maxFps = 0;
VideoCodecConfig codecConfig(120, "VP8", constraints);
codecConfig.mEncodings.emplace_back();
aControl.mVideoSendCodec = Some(codecConfig);
@ -367,7 +366,7 @@ TEST_F(VideoConduitTest, TestConfigureSendMediaCodecMaxFps) {
mControl.Update([&](auto& aControl) {
EncodingConstraints constraints;
constraints.maxFps = 42;
constraints.maxFps = Some(42);
VideoCodecConfig codecConfig(120, "VP8", constraints);
codecConfig.mEncodings.emplace_back();
aControl.mVideoSendCodec = Some(codecConfig);