Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(589)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: webrtc/pc/mediasession_unittest.cc

Issue 2505003003: Let MediaSession generate a FlexFEC SSRC when FlexFEC is active. (Closed)
Patch Set: Rebase. Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« webrtc/pc/mediasession.cc ('K') | « webrtc/pc/mediasession.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/pc/mediasession_unittest.cc
diff --git a/webrtc/pc/mediasession_unittest.cc b/webrtc/pc/mediasession_unittest.cc
index ca1e4b39394c4af9bb75a2fb03842662cbd32c75..4a3d7a7b9a4b54f486ca27fed5a031ec753913cc 100644
--- a/webrtc/pc/mediasession_unittest.cc
+++ b/webrtc/pc/mediasession_unittest.cc
@@ -190,10 +190,10 @@ GetMediaDirection(const ContentInfo* content) {
return desc->direction();
}
-static void AddRtxCodec(const VideoCodec& rtx_codec,
- std::vector<VideoCodec>* codecs) {
- ASSERT_FALSE(cricket::FindCodecById(*codecs, rtx_codec.id));
- codecs->push_back(rtx_codec);
+static void AddSecondaryCodec(const VideoCodec& secondary_codec,
perkj_webrtc 2016/11/17 20:41:12 Why not just AddCodec if you want to keep it at al
brandtr 2016/11/21 09:16:30 After a second thought, I don't think the the asse
+ std::vector<VideoCodec>* codecs) {
+ ASSERT_FALSE(cricket::FindCodecById(*codecs, secondary_codec.id));
+ codecs->push_back(secondary_codec);
}
template <class T>
@@ -1631,7 +1631,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoAnswer) {
EXPECT_TRUE(data_streams[0] == updated_data_streams[0]);
}
-
// Create an updated offer after creating an answer to the original offer and
// verify that the codecs that were part of the original answer are not changed
// in the updated offer.
@@ -1694,12 +1693,14 @@ TEST_F(MediaSessionDescriptionFactoryTest,
opts.recv_audio = false;
std::vector<VideoCodec> f1_codecs = MAKE_VECTOR(kVideoCodecs1);
// This creates rtx for H264 with the payload type |f1_| uses.
- AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id), &f1_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id),
+ &f1_codecs);
f1_.set_video_codecs(f1_codecs);
std::vector<VideoCodec> f2_codecs = MAKE_VECTOR(kVideoCodecs2);
// This creates rtx for H264 with the payload type |f2_| uses.
- AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[0].id), &f2_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[0].id),
+ &f2_codecs);
f2_.set_video_codecs(f2_codecs);
std::unique_ptr<SessionDescription> offer(f1_.CreateOffer(opts, NULL));
@@ -1711,8 +1712,8 @@ TEST_F(MediaSessionDescriptionFactoryTest,
GetFirstVideoContentDescription(answer.get());
std::vector<VideoCodec> expected_codecs = MAKE_VECTOR(kVideoCodecsAnswer);
- AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id),
- &expected_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id),
+ &expected_codecs);
EXPECT_EQ(expected_codecs, vcd->codecs());
@@ -1739,7 +1740,8 @@ TEST_F(MediaSessionDescriptionFactoryTest,
RespondentCreatesOfferWithVideoAndRtxAfterCreatingAudioAnswer) {
std::vector<VideoCodec> f1_codecs = MAKE_VECTOR(kVideoCodecs1);
// This creates rtx for H264 with the payload type |f1_| uses.
- AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id), &f1_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id),
+ &f1_codecs);
f1_.set_video_codecs(f1_codecs);
MediaSessionOptions opts;
@@ -1763,7 +1765,7 @@ TEST_F(MediaSessionDescriptionFactoryTest,
std::vector<VideoCodec> f2_codecs = MAKE_VECTOR(kVideoCodecs2);
int used_pl_type = acd->codecs()[0].id;
f2_codecs[0].id = used_pl_type; // Set the payload type for H264.
- AddRtxCodec(VideoCodec::CreateRtxCodec(125, used_pl_type), &f2_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(125, used_pl_type), &f2_codecs);
f2_.set_video_codecs(f2_codecs);
std::unique_ptr<SessionDescription> updated_offer(
@@ -1800,7 +1802,8 @@ TEST_F(MediaSessionDescriptionFactoryTest,
std::vector<VideoCodec> f2_codecs = MAKE_VECTOR(kVideoCodecs2);
// This creates rtx for H264 with the payload type |f2_| uses.
- AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[0].id), &f2_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[0].id),
+ &f2_codecs);
f2_.set_video_codecs(f2_codecs);
std::unique_ptr<SessionDescription> offer(f1_.CreateOffer(opts, nullptr));
@@ -1826,8 +1829,8 @@ TEST_F(MediaSessionDescriptionFactoryTest,
// New offer should attempt to add H263, and RTX for H264.
expected_codecs.push_back(kVideoCodecs2[1]);
- AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs1[1].id),
- &expected_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs1[1].id),
+ &expected_codecs);
EXPECT_EQ(expected_codecs, updated_vcd->codecs());
}
@@ -1838,12 +1841,13 @@ TEST_F(MediaSessionDescriptionFactoryTest, RtxWithoutApt) {
opts.recv_audio = false;
std::vector<VideoCodec> f1_codecs = MAKE_VECTOR(kVideoCodecs1);
// This creates RTX without associated payload type parameter.
- AddRtxCodec(VideoCodec(126, cricket::kRtxCodecName), &f1_codecs);
+ AddSecondaryCodec(VideoCodec(126, cricket::kRtxCodecName), &f1_codecs);
f1_.set_video_codecs(f1_codecs);
std::vector<VideoCodec> f2_codecs = MAKE_VECTOR(kVideoCodecs2);
// This creates RTX for H264 with the payload type |f2_| uses.
- AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[0].id), &f2_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[0].id),
+ &f2_codecs);
f2_.set_video_codecs(f2_codecs);
std::unique_ptr<SessionDescription> offer(f1_.CreateOffer(opts, NULL));
@@ -1882,12 +1886,14 @@ TEST_F(MediaSessionDescriptionFactoryTest, FilterOutRtxIfAptDoesntMatch) {
opts.recv_audio = false;
std::vector<VideoCodec> f1_codecs = MAKE_VECTOR(kVideoCodecs1);
// This creates RTX for H264 in sender.
- AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id), &f1_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id),
+ &f1_codecs);
f1_.set_video_codecs(f1_codecs);
std::vector<VideoCodec> f2_codecs = MAKE_VECTOR(kVideoCodecs2);
// This creates RTX for H263 in receiver.
- AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[1].id), &f2_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs2[1].id),
+ &f2_codecs);
f2_.set_video_codecs(f2_codecs);
std::unique_ptr<SessionDescription> offer(f1_.CreateOffer(opts, NULL));
@@ -1912,16 +1918,19 @@ TEST_F(MediaSessionDescriptionFactoryTest,
opts.recv_audio = false;
std::vector<VideoCodec> f1_codecs = MAKE_VECTOR(kVideoCodecs1);
// This creates RTX for H264-SVC in sender.
- AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs1[0].id), &f1_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs1[0].id),
+ &f1_codecs);
f1_.set_video_codecs(f1_codecs);
// This creates RTX for H264 in sender.
- AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id), &f1_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id),
+ &f1_codecs);
f1_.set_video_codecs(f1_codecs);
std::vector<VideoCodec> f2_codecs = MAKE_VECTOR(kVideoCodecs2);
// This creates RTX for H264 in receiver.
- AddRtxCodec(VideoCodec::CreateRtxCodec(124, kVideoCodecs2[0].id), &f2_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(124, kVideoCodecs2[0].id),
+ &f2_codecs);
f2_.set_video_codecs(f2_codecs);
// H264-SVC codec is removed in the answer, therefore, associated RTX codec
@@ -1933,8 +1942,8 @@ TEST_F(MediaSessionDescriptionFactoryTest,
const VideoContentDescription* vcd =
GetFirstVideoContentDescription(answer.get());
std::vector<VideoCodec> expected_codecs = MAKE_VECTOR(kVideoCodecsAnswer);
- AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id),
- &expected_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id),
+ &expected_codecs);
EXPECT_EQ(expected_codecs, vcd->codecs());
}
@@ -1947,7 +1956,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, AddSecondRtxInNewOffer) {
opts.recv_audio = false;
std::vector<VideoCodec> f1_codecs = MAKE_VECTOR(kVideoCodecs1);
// This creates RTX for H264 for the offerer.
- AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id), &f1_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id),
+ &f1_codecs);
f1_.set_video_codecs(f1_codecs);
std::unique_ptr<SessionDescription> offer(f1_.CreateOffer(opts, nullptr));
@@ -1956,12 +1966,13 @@ TEST_F(MediaSessionDescriptionFactoryTest, AddSecondRtxInNewOffer) {
GetFirstVideoContentDescription(offer.get());
std::vector<VideoCodec> expected_codecs = MAKE_VECTOR(kVideoCodecs1);
- AddRtxCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id),
- &expected_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(126, kVideoCodecs1[1].id),
+ &expected_codecs);
EXPECT_EQ(expected_codecs, vcd->codecs());
// Now, attempt to add RTX for H264-SVC.
- AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs1[0].id), &f1_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs1[0].id),
+ &f1_codecs);
f1_.set_video_codecs(f1_codecs);
std::unique_ptr<SessionDescription> updated_offer(
@@ -1969,8 +1980,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, AddSecondRtxInNewOffer) {
ASSERT_TRUE(updated_offer);
vcd = GetFirstVideoContentDescription(updated_offer.get());
- AddRtxCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs1[0].id),
- &expected_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(125, kVideoCodecs1[0].id),
+ &expected_codecs);
EXPECT_EQ(expected_codecs, vcd->codecs());
}
@@ -1987,7 +1998,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, SimSsrcsGenerateMultipleRtxSsrcs) {
// Use a single real codec, and then add RTX for it.
std::vector<VideoCodec> f1_codecs;
f1_codecs.push_back(VideoCodec(97, "H264"));
- AddRtxCodec(VideoCodec::CreateRtxCodec(125, 97), &f1_codecs);
+ AddSecondaryCodec(VideoCodec::CreateRtxCodec(125, 97), &f1_codecs);
f1_.set_video_codecs(f1_codecs);
// Ensure that the offer has an RTX ssrc for each regular ssrc, and that there
@@ -2015,6 +2026,88 @@ TEST_F(MediaSessionDescriptionFactoryTest, SimSsrcsGenerateMultipleRtxSsrcs) {
EXPECT_EQ(3u, fid_ssrcs.size());
}
+// Test that, when the FlexFEC codec is added, a FlexFEC ssrc is created
+// together with a FEC-FR grouping.
+TEST_F(MediaSessionDescriptionFactoryTest, GenerateFlexfecSsrc) {
+ MediaSessionOptions opts;
+ opts.recv_video = true;
+ opts.recv_audio = false;
+
+ // Add single stream.
+ opts.AddSendVideoStream("stream1", "stream1label", 1);
+
+ // Use a single real codec, and then add FlexFEC for it.
+ std::vector<VideoCodec> f1_codecs;
+ f1_codecs.push_back(VideoCodec(97, "H264"));
+ AddSecondaryCodec(VideoCodec(118, "flexfec-03"), &f1_codecs);
+ f1_.set_video_codecs(f1_codecs);
+
+ // Ensure that the offer has a single FlexFEC ssrc and that
+ // there is no FEC-FR ssrc + grouping for each.
+ std::unique_ptr<SessionDescription> offer(f1_.CreateOffer(opts, nullptr));
+ ASSERT_TRUE(offer.get() != nullptr);
+ VideoContentDescription* desc = static_cast<VideoContentDescription*>(
+ offer->GetContentDescriptionByName(cricket::CN_VIDEO));
+ ASSERT_TRUE(desc != nullptr);
+ EXPECT_TRUE(desc->multistream());
+ const StreamParamsVec& streams = desc->streams();
+ // Single stream.
+ ASSERT_EQ(1u, streams.size());
+ // Stream should have 2 ssrcs: 1 for video, 1 for FlexFEC.
+ EXPECT_EQ(2u, streams[0].ssrcs.size());
+ // And should a FEC-FR group for FlexFEC.
perkj_webrtc 2016/11/17 20:41:12 and should have
brandtr 2016/11/21 09:16:30 Done.
+ EXPECT_TRUE(streams[0].has_ssrc_group("FEC-FR"));
+ std::vector<uint32_t> primary_ssrcs;
+ streams[0].GetPrimarySsrcs(&primary_ssrcs);
+ ASSERT_EQ(1u, primary_ssrcs.size());
+ uint32_t flexfec_ssrc;
+ EXPECT_TRUE(streams[0].GetFecFrSsrc(primary_ssrcs[0], &flexfec_ssrc));
+ EXPECT_NE(flexfec_ssrc, 0u);
+}
+
+// Test that FlexFEC is disabled for simulcast.
+// TODO(brandtr): Remove this test when we support simulcast, either through
+// multiple FlexfecSenders, or through multistream protection.
+TEST_F(MediaSessionDescriptionFactoryTest, SimSsrcsGenerateNoFlexfecSsrcs) {
+ MediaSessionOptions opts;
+ opts.recv_video = true;
+ opts.recv_audio = false;
+
+ // Add simulcast streams.
+ opts.AddSendVideoStream("stream1", "stream1label", 3);
+
+ // Use a single real codec, and then add FlexFEC for it.
+ std::vector<VideoCodec> f1_codecs;
+ f1_codecs.push_back(VideoCodec(97, "H264"));
+ AddSecondaryCodec(VideoCodec(118, "flexfec-03"), &f1_codecs);
+ f1_.set_video_codecs(f1_codecs);
+
+ // Ensure that the offer has no FlexFEC ssrcs for each regular ssrc, and that
+ // there is no FEC-FR ssrc + grouping for each.
+ std::unique_ptr<SessionDescription> offer(f1_.CreateOffer(opts, nullptr));
+ ASSERT_TRUE(offer.get() != nullptr);
+ VideoContentDescription* desc = static_cast<VideoContentDescription*>(
+ offer->GetContentDescriptionByName(cricket::CN_VIDEO));
+ ASSERT_TRUE(desc != nullptr);
+ EXPECT_FALSE(desc->multistream());
+ const StreamParamsVec& streams = desc->streams();
+ // Single stream.
+ ASSERT_EQ(1u, streams.size());
+ // Stream should have 3 ssrcs: 3 for video, 0 for FlexFEC.
+ EXPECT_EQ(3u, streams[0].ssrcs.size());
+ // And should have a SIM group for the simulcast.
+ EXPECT_TRUE(streams[0].has_ssrc_group("SIM"));
+ // And not a FEC-FR group for FlexFEC.
+ EXPECT_FALSE(streams[0].has_ssrc_group("FEC-FR"));
+ std::vector<uint32_t> primary_ssrcs;
+ streams[0].GetPrimarySsrcs(&primary_ssrcs);
+ EXPECT_EQ(3u, primary_ssrcs.size());
+ for (uint32_t primary_ssrc : primary_ssrcs) {
+ uint32_t flexfec_ssrc;
+ EXPECT_FALSE(streams[0].GetFecFrSsrc(primary_ssrc, &flexfec_ssrc));
+ }
+}
+
// Create an updated offer after creating an answer to the original offer and
// verify that the RTP header extensions that were part of the original answer
// are not changed in the updated offer.
@@ -2862,7 +2955,8 @@ void TestAudioCodecsAnswer(MediaContentDirection offer_direction,
<< "Only inactive offers are allowed to not generate any audio content";
}
}
-}
+
+} // namespace
class AudioCodecsOfferTest
: public ::testing::TestWithParam<std::tr1::tuple<MediaContentDirection,
« webrtc/pc/mediasession.cc ('K') | « webrtc/pc/mediasession.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698