Make |rtcp_send_transport| mandatory in FlexfecReceiveStream::Config.
That object will be used when we enable RTCP reporting from FlexfecReceiveStream.
Other related changes:
- Stop using FlexfecConfig (from config.h) at receive side in WebRtcVideoEngine2.
- Add a IsCompleteAndEnabled() method to FlexfecReceiveStream::Config, to be
used in WebRtcVideoEngine2.
- Centralize the construction of the FlexfecReceiveStream::Config in unit tests.
This will make future additions to the unit tests cleaner.
- Simplify setup for receiving FlexFEC in VideoQualityTest.
BUG=webrtc:5654
Review-Url: https://codereview.webrtc.org/2589713003
Cr-Original-Commit-Position: refs/heads/master@{#16059}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 8313a6fa8f78d18e0f5e2bff31086da01d0db4bf
diff --git a/call/call_unittest.cc b/call/call_unittest.cc
index 7a7e7a9..99d6812 100644
--- a/call/call_unittest.cc
+++ b/call/call_unittest.cc
@@ -17,6 +17,7 @@
#include "webrtc/modules/audio_coding/codecs/mock/mock_audio_decoder_factory.h"
#include "webrtc/modules/audio_mixer/audio_mixer_impl.h"
#include "webrtc/test/gtest.h"
+#include "webrtc/test/mock_transport.h"
#include "webrtc/test/mock_voice_engine.h"
namespace {
@@ -222,7 +223,8 @@
TEST(CallTest, CreateDestroy_FlexfecReceiveStream) {
CallHelper call;
- FlexfecReceiveStream::Config config;
+ MockTransport rtcp_send_transport;
+ FlexfecReceiveStream::Config config(&rtcp_send_transport);
config.payload_type = 118;
config.remote_ssrc = 38837212;
config.protected_media_ssrcs = {27273};
@@ -234,7 +236,8 @@
TEST(CallTest, CreateDestroy_FlexfecReceiveStreams) {
CallHelper call;
- FlexfecReceiveStream::Config config;
+ MockTransport rtcp_send_transport;
+ FlexfecReceiveStream::Config config(&rtcp_send_transport);
config.payload_type = 118;
std::list<FlexfecReceiveStream*> streams;
@@ -259,7 +262,8 @@
TEST(CallTest, MultipleFlexfecReceiveStreamsProtectingSingleVideoStream) {
CallHelper call;
- FlexfecReceiveStream::Config config;
+ MockTransport rtcp_send_transport;
+ FlexfecReceiveStream::Config config(&rtcp_send_transport);
config.payload_type = 118;
config.protected_media_ssrcs = {1324234};
FlexfecReceiveStream* stream;
diff --git a/call/flexfec_receive_stream.h b/call/flexfec_receive_stream.h
index fe57643..babeee0 100644
--- a/call/flexfec_receive_stream.h
+++ b/call/flexfec_receive_stream.h
@@ -31,8 +31,17 @@
};
struct Config {
+ explicit Config(Transport* rtcp_send_transport)
+ : rtcp_send_transport(rtcp_send_transport) {
+ RTC_DCHECK(rtcp_send_transport);
+ }
+
std::string ToString() const;
+ // Returns true if all RTP information is available in order to
+ // enable receiving FlexFEC.
+ bool IsCompleteAndEnabled() const;
+
// Payload type for FlexFEC.
int payload_type = -1;
diff --git a/call/flexfec_receive_stream_impl.cc b/call/flexfec_receive_stream_impl.cc
index 13390a1..95bcfc1 100644
--- a/call/flexfec_receive_stream_impl.cc
+++ b/call/flexfec_receive_stream_impl.cc
@@ -46,6 +46,19 @@
return ss.str();
}
+bool FlexfecReceiveStream::Config::IsCompleteAndEnabled() const {
+ // Check if FlexFEC is enabled.
+ if (payload_type < 0)
+ return false;
+ // Do we have the necessary SSRC information?
+ if (remote_ssrc == 0)
+ return false;
+ // TODO(brandtr): Update this check when we support multistream protection.
+ if (protected_media_ssrcs.size() != 1u)
+ return false;
+ return true;
+}
+
namespace {
// TODO(brandtr): Update this function when we support multistream protection.
diff --git a/call/flexfec_receive_stream_unittest.cc b/call/flexfec_receive_stream_unittest.cc
index c25858f..2402636 100644
--- a/call/flexfec_receive_stream_unittest.cc
+++ b/call/flexfec_receive_stream_unittest.cc
@@ -13,16 +13,32 @@
#include "webrtc/base/array_view.h"
#include "webrtc/call/flexfec_receive_stream_impl.h"
#include "webrtc/modules/rtp_rtcp/include/flexfec_receiver.h"
-#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
-#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h"
#include "webrtc/modules/rtp_rtcp/mocks/mock_recovered_packet_receiver.h"
+#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
+#include "webrtc/modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "webrtc/test/gmock.h"
#include "webrtc/test/gtest.h"
+#include "webrtc/test/mock_transport.h"
namespace webrtc {
namespace {
+constexpr uint8_t kFlexfecPlType = 118;
+constexpr uint8_t kFlexfecSsrc[] = {0x00, 0x00, 0x00, 0x01};
+constexpr uint8_t kMediaSsrc[] = {0x00, 0x00, 0x00, 0x02};
+
+FlexfecReceiveStream::Config CreateDefaultConfig(
+ Transport* rtcp_send_transport) {
+ FlexfecReceiveStream::Config config(rtcp_send_transport);
+ config.payload_type = kFlexfecPlType;
+ config.remote_ssrc = ByteReader<uint32_t>::ReadBigEndian(kFlexfecSsrc);
+ config.protected_media_ssrcs = {
+ ByteReader<uint32_t>::ReadBigEndian(kMediaSsrc)};
+ EXPECT_TRUE(config.IsCompleteAndEnabled());
+ return config;
+}
+
RtpPacketReceived ParsePacket(rtc::ArrayView<const uint8_t> packet) {
RtpPacketReceived parsed_packet(nullptr);
EXPECT_TRUE(parsed_packet.Parse(packet));
@@ -31,40 +47,58 @@
} // namespace
-TEST(FlexfecReceiveStreamTest, ConstructDestruct) {
- FlexfecReceiveStream::Config config;
- config.payload_type = 118;
- config.remote_ssrc = 424223;
- config.protected_media_ssrcs = {912512};
- MockRecoveredPacketReceiver recovered_packet_receiver;
+TEST(FlexfecReceiveStreamConfigTest, IsCompleteAndEnabled) {
+ MockTransport rtcp_send_transport;
+ FlexfecReceiveStream::Config config(&rtcp_send_transport);
- FlexfecReceiveStreamImpl receive_stream(config, &recovered_packet_receiver);
+ config.local_ssrc = 18374743;
+ config.rtcp_mode = RtcpMode::kCompound;
+ config.transport_cc = true;
+ config.rtp_header_extensions.emplace_back(TransportSequenceNumber::kUri, 7);
+ EXPECT_FALSE(config.IsCompleteAndEnabled());
+
+ config.payload_type = 123;
+ EXPECT_FALSE(config.IsCompleteAndEnabled());
+
+ config.remote_ssrc = 238423838;
+ EXPECT_FALSE(config.IsCompleteAndEnabled());
+
+ config.protected_media_ssrcs.push_back(138989393);
+ EXPECT_TRUE(config.IsCompleteAndEnabled());
+
+ config.protected_media_ssrcs.push_back(33423423);
+ EXPECT_FALSE(config.IsCompleteAndEnabled());
}
-TEST(FlexfecReceiveStreamTest, StartStop) {
- FlexfecReceiveStream::Config config;
- config.payload_type = 118;
- config.remote_ssrc = 1652392;
- config.protected_media_ssrcs = {23300443};
- MockRecoveredPacketReceiver recovered_packet_receiver;
- FlexfecReceiveStreamImpl receive_stream(config, &recovered_packet_receiver);
+class FlexfecReceiveStreamTest : public ::testing::Test {
+ protected:
+ FlexfecReceiveStreamTest()
+ : config_(CreateDefaultConfig(&rtcp_send_transport_)),
+ receive_stream_(config_, &recovered_packet_receiver_) {}
- receive_stream.Start();
- receive_stream.Stop();
+ FlexfecReceiveStream::Config config_;
+ MockRecoveredPacketReceiver recovered_packet_receiver_;
+ MockTransport rtcp_send_transport_;
+
+ FlexfecReceiveStreamImpl receive_stream_;
+};
+
+TEST_F(FlexfecReceiveStreamTest, ConstructDestruct) {}
+
+TEST_F(FlexfecReceiveStreamTest, StartStop) {
+ receive_stream_.Start();
+ receive_stream_.Stop();
}
// Create a FlexFEC packet that protects a single media packet and ensure
// that the callback is called. Correctness of recovery is checked in the
// FlexfecReceiver unit tests.
-TEST(FlexfecReceiveStreamTest, RecoversPacketWhenStarted) {
- constexpr uint8_t kFlexfecPlType = 118;
+TEST_F(FlexfecReceiveStreamTest, RecoversPacketWhenStarted) {
constexpr uint8_t kFlexfecSeqNum[] = {0x00, 0x01};
constexpr uint8_t kFlexfecTs[] = {0x00, 0x11, 0x22, 0x33};
- constexpr uint8_t kFlexfecSsrc[] = {0x00, 0x00, 0x00, 0x01};
constexpr uint8_t kMediaPlType = 107;
constexpr uint8_t kMediaSeqNum[] = {0x00, 0x02};
constexpr uint8_t kMediaTs[] = {0xaa, 0xbb, 0xcc, 0xdd};
- constexpr uint8_t kMediaSsrc[] = {0x00, 0x00, 0x00, 0x02};
// This packet mask protects a single media packet, i.e., the FlexFEC payload
// is a copy of that media packet. When inserted in the FlexFEC pipeline,
@@ -91,13 +125,8 @@
kPayloadBits, kPayloadBits, kPayloadBits, kPayloadBits};
// clang-format on
- FlexfecReceiveStream::Config config;
- config.payload_type = kFlexfecPlType;
- config.remote_ssrc = ByteReader<uint32_t>::ReadBigEndian(kFlexfecSsrc);
- config.protected_media_ssrcs = {
- ByteReader<uint32_t>::ReadBigEndian(kMediaSsrc)};
testing::StrictMock<MockRecoveredPacketReceiver> recovered_packet_receiver;
- FlexfecReceiveStreamImpl receive_stream(config, &recovered_packet_receiver);
+ FlexfecReceiveStreamImpl receive_stream(config_, &recovered_packet_receiver);
// Do not call back before being started.
receive_stream.AddAndProcessReceivedPacket(ParsePacket(kFlexfecPacket));