diff --git a/call/call.cc b/call/call.cc index 08e9dc2553..959fd4bb3f 100644 --- a/call/call.cc +++ b/call/call.cc @@ -330,6 +330,10 @@ class Call : public webrtc::Call, RtpStateMap suspended_video_send_ssrcs_ RTC_GUARDED_BY(configuration_sequence_checker_); + using RtpPayloadStateMap = std::map; + RtpPayloadStateMap suspended_video_payload_states_ + RTC_GUARDED_BY(configuration_sequence_checker_); + webrtc::RtcEventLog* event_log_; // The following members are only accessed (exclusively) from one thread and @@ -751,7 +755,8 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream( num_cpu_cores_, module_process_thread_.get(), &worker_queue_, call_stats_.get(), transport_send_.get(), bitrate_allocator_.get(), video_send_delay_stats_.get(), event_log_, std::move(config), - std::move(encoder_config), suspended_video_send_ssrcs_); + std::move(encoder_config), suspended_video_send_ssrcs_, + suspended_video_payload_states_); { WriteLockScoped write_lock(*send_crit_); @@ -790,12 +795,15 @@ void Call::DestroyVideoSendStream(webrtc::VideoSendStream* send_stream) { } RTC_CHECK(send_stream_impl != nullptr); - VideoSendStream::RtpStateMap rtp_state = - send_stream_impl->StopPermanentlyAndGetRtpStates(); - - for (VideoSendStream::RtpStateMap::iterator it = rtp_state.begin(); - it != rtp_state.end(); ++it) { - suspended_video_send_ssrcs_[it->first] = it->second; + VideoSendStream::RtpStateMap rtp_states; + VideoSendStream::RtpPayloadStateMap rtp_payload_states; + send_stream_impl->StopPermanentlyAndGetRtpStates(&rtp_states, + &rtp_payload_states); + for (const auto& kv : rtp_states) { + suspended_video_send_ssrcs_[kv.first] = kv.second; + } + for (const auto& kv : rtp_payload_states) { + suspended_video_payload_states_[kv.first] = kv.second; } UpdateAggregateNetworkState(); diff --git a/common_types.h b/common_types.h index e7d912c90d..f7c88b87b8 100644 --- a/common_types.h +++ b/common_types.h @@ -912,6 +912,11 @@ struct RtpKeepAliveConfig final { bool operator!=(const RtpKeepAliveConfig& o) const { return !(*this == o); } }; +// Currently only VP8/VP9 specific. +struct RtpPayloadState { + int16_t picture_id = -1; +}; + } // namespace webrtc #endif // COMMON_TYPES_H_ diff --git a/video/payload_router.cc b/video/payload_router.cc index a7a20deadc..017b9370f8 100644 --- a/video/payload_router.cc +++ b/video/payload_router.cc @@ -14,6 +14,9 @@ #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/video_coding/include/video_codec_interface.h" #include "rtc_base/checks.h" +#include "rtc_base/random.h" +#include "rtc_base/timeutils.h" +#include "system_wrappers/include/field_trial.h" namespace webrtc { @@ -89,11 +92,56 @@ void CopyCodecSpecific(const CodecSpecificInfo* info, RTPVideoHeader* rtp) { } // namespace +// Currently only used if forced fallback for VP8 is enabled. +// Consider adding tl0idx and set for VP8 and VP9. +// Make picture id not codec specific. +class PayloadRouter::RtpPayloadParams final { + public: + RtpPayloadParams(const uint32_t ssrc, const RtpPayloadState* state) + : ssrc_(ssrc) { + Random random(rtc::TimeMicros()); + state_.picture_id = + state ? state->picture_id : (random.Rand() & 0x7FFF); + } + ~RtpPayloadParams() {} + + void Set(RTPVideoHeader* rtp_video_header) { + if (rtp_video_header->codec == kRtpVideoVp8 && + rtp_video_header->codecHeader.VP8.pictureId != kNoPictureId) { + rtp_video_header->codecHeader.VP8.pictureId = state_.picture_id; + state_.picture_id = (state_.picture_id + 1) & 0x7FFF; + } + } + + uint32_t ssrc() const { return ssrc_; } + + RtpPayloadState state() const { return state_; } + + private: + const uint32_t ssrc_; + RtpPayloadState state_; +}; + PayloadRouter::PayloadRouter(const std::vector& rtp_modules, - int payload_type) + const std::vector& ssrcs, + int payload_type, + const std::map& states) : active_(false), rtp_modules_(rtp_modules), - payload_type_(payload_type) { + payload_type_(payload_type), + forced_fallback_enabled_((webrtc::field_trial::IsEnabled( + "WebRTC-VP8-Forced-Fallback-Encoder"))) { + RTC_DCHECK_EQ(ssrcs.size(), rtp_modules.size()); + // SSRCs are assumed to be sorted in the same order as |rtp_modules|. + for (uint32_t ssrc : ssrcs) { + // Restore state if it previously existed. + const RtpPayloadState* state = nullptr; + auto it = states.find(ssrc); + if (it != states.end()) { + state = &it->second; + } + params_.push_back(RtpPayloadParams(ssrc, state)); + } } PayloadRouter::~PayloadRouter() {} @@ -115,6 +163,15 @@ bool PayloadRouter::IsActive() { return active_ && !rtp_modules_.empty(); } +std::map PayloadRouter::GetRtpPayloadStates() const { + rtc::CritScope lock(&crit_); + std::map payload_states; + for (const auto& param : params_) { + payload_states[param.ssrc()] = param.state(); + } + return payload_states; +} + EncodedImageCallback::Result PayloadRouter::OnEncodedImage( const EncodedImage& encoded_image, const CodecSpecificInfo* codec_specific_info, @@ -149,6 +206,12 @@ EncodedImageCallback::Result PayloadRouter::OnEncodedImage( int stream_index = rtp_video_header.simulcastIdx; RTC_DCHECK_LT(stream_index, rtp_modules_.size()); + if (forced_fallback_enabled_) { + // Sets picture id. The SW and HW encoder have separate picture id + // sequences, set picture id to not cause sequence discontinuties at encoder + // changes. + params_[stream_index].Set(&rtp_video_header); + } uint32_t frame_id; bool send_result = rtp_modules_[stream_index]->SendOutgoingData( encoded_image._frameType, payload_type_, encoded_image._timeStamp, diff --git a/video/payload_router.h b/video/payload_router.h index c5765661df..13b6cae7ce 100644 --- a/video/payload_router.h +++ b/video/payload_router.h @@ -11,6 +11,7 @@ #ifndef VIDEO_PAYLOAD_ROUTER_H_ #define VIDEO_PAYLOAD_ROUTER_H_ +#include #include #include "api/video_codecs/video_encoder.h" @@ -31,7 +32,9 @@ class PayloadRouter : public EncodedImageCallback { public: // Rtp modules are assumed to be sorted in simulcast index order. PayloadRouter(const std::vector& rtp_modules, - int payload_type); + const std::vector& ssrcs, + int payload_type, + const std::map& states); ~PayloadRouter(); // PayloadRouter will only route packets if being active, all packets will be @@ -39,6 +42,8 @@ class PayloadRouter : public EncodedImageCallback { void SetActive(bool active); bool IsActive(); + std::map GetRtpPayloadStates() const; + // Implements EncodedImageCallback. // Returns 0 if the packet was routed / sent, -1 otherwise. EncodedImageCallback::Result OnEncodedImage( @@ -49,6 +54,8 @@ class PayloadRouter : public EncodedImageCallback { void OnBitrateAllocationUpdated(const BitrateAllocation& bitrate); private: + class RtpPayloadParams; + void UpdateModuleSendingState() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_); rtc::CriticalSection crit_; @@ -58,6 +65,9 @@ class PayloadRouter : public EncodedImageCallback { const std::vector rtp_modules_; const int payload_type_; + const bool forced_fallback_enabled_; + std::vector params_ RTC_GUARDED_BY(crit_); + RTC_DISALLOW_COPY_AND_ASSIGN(PayloadRouter); }; diff --git a/video/payload_router_unittest.cc b/video/payload_router_unittest.cc index 5f56a20700..fe4ea12bda 100644 --- a/video/payload_router_unittest.cc +++ b/video/payload_router_unittest.cc @@ -9,29 +9,41 @@ */ #include +#include #include "call/video_config.h" #include "modules/rtp_rtcp/include/rtp_rtcp.h" #include "modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" #include "modules/video_coding/include/video_codec_interface.h" +#include "test/field_trial.h" #include "test/gmock.h" #include "test/gtest.h" #include "video/payload_router.h" using ::testing::_; using ::testing::AnyNumber; +using ::testing::Invoke; using ::testing::NiceMock; using ::testing::Return; +using ::testing::Unused; namespace webrtc { +namespace { +const int8_t kPayloadType = 96; +const uint32_t kSsrc1 = 12345; +const uint32_t kSsrc2 = 23456; +const int16_t kPictureId = 123; +const int16_t kTl0PicIdx = 20; +const uint8_t kTemporalIdx = 1; +const int16_t kInitialPictureId1 = 222; +const int16_t kInitialPictureId2 = 44; +} // namespace TEST(PayloadRouterTest, SendOnOneModule) { NiceMock rtp; std::vector modules(1, &rtp); - std::vector streams(1); uint8_t payload = 'a'; - int8_t payload_type = 96; EncodedImage encoded_image; encoded_image._timeStamp = 1; encoded_image.capture_time_ms_ = 2; @@ -39,9 +51,9 @@ TEST(PayloadRouterTest, SendOnOneModule) { encoded_image._buffer = &payload; encoded_image._length = 1; - PayloadRouter payload_router(modules, payload_type); + PayloadRouter payload_router(modules, {kSsrc1}, kPayloadType, {}); - EXPECT_CALL(rtp, SendOutgoingData(encoded_image._frameType, payload_type, + EXPECT_CALL(rtp, SendOutgoingData(encoded_image._frameType, kPayloadType, encoded_image._timeStamp, encoded_image.capture_time_ms_, &payload, encoded_image._length, nullptr, _, _)) @@ -51,7 +63,7 @@ TEST(PayloadRouterTest, SendOnOneModule) { payload_router.OnEncodedImage(encoded_image, nullptr, nullptr).error); payload_router.SetActive(true); - EXPECT_CALL(rtp, SendOutgoingData(encoded_image._frameType, payload_type, + EXPECT_CALL(rtp, SendOutgoingData(encoded_image._frameType, kPayloadType, encoded_image._timeStamp, encoded_image.capture_time_ms_, &payload, encoded_image._length, nullptr, _, _)) @@ -62,7 +74,7 @@ TEST(PayloadRouterTest, SendOnOneModule) { payload_router.OnEncodedImage(encoded_image, nullptr, nullptr).error); payload_router.SetActive(false); - EXPECT_CALL(rtp, SendOutgoingData(encoded_image._frameType, payload_type, + EXPECT_CALL(rtp, SendOutgoingData(encoded_image._frameType, kPayloadType, encoded_image._timeStamp, encoded_image.capture_time_ms_, &payload, encoded_image._length, nullptr, _, _)) @@ -72,7 +84,7 @@ TEST(PayloadRouterTest, SendOnOneModule) { payload_router.OnEncodedImage(encoded_image, nullptr, nullptr).error); payload_router.SetActive(true); - EXPECT_CALL(rtp, SendOutgoingData(encoded_image._frameType, payload_type, + EXPECT_CALL(rtp, SendOutgoingData(encoded_image._frameType, kPayloadType, encoded_image._timeStamp, encoded_image.capture_time_ms_, &payload, encoded_image._length, nullptr, _, _)) @@ -86,12 +98,8 @@ TEST(PayloadRouterTest, SendOnOneModule) { TEST(PayloadRouterTest, SendSimulcast) { NiceMock rtp_1; NiceMock rtp_2; - std::vector modules; - modules.push_back(&rtp_1); - modules.push_back(&rtp_2); - std::vector streams(2); + std::vector modules = {&rtp_1, &rtp_2}; - int8_t payload_type = 96; uint8_t payload = 'a'; EncodedImage encoded_image; encoded_image._timeStamp = 1; @@ -100,7 +108,7 @@ TEST(PayloadRouterTest, SendSimulcast) { encoded_image._buffer = &payload; encoded_image._length = 1; - PayloadRouter payload_router(modules, payload_type); + PayloadRouter payload_router(modules, {kSsrc1, kSsrc2}, kPayloadType, {}); CodecSpecificInfo codec_info_1; memset(&codec_info_1, 0, sizeof(CodecSpecificInfo)); @@ -108,7 +116,7 @@ TEST(PayloadRouterTest, SendSimulcast) { codec_info_1.codecSpecific.VP8.simulcastIdx = 0; payload_router.SetActive(true); - EXPECT_CALL(rtp_1, SendOutgoingData(encoded_image._frameType, payload_type, + EXPECT_CALL(rtp_1, SendOutgoingData(encoded_image._frameType, kPayloadType, encoded_image._timeStamp, encoded_image.capture_time_ms_, &payload, encoded_image._length, nullptr, _, _)) @@ -124,7 +132,7 @@ TEST(PayloadRouterTest, SendSimulcast) { codec_info_2.codecType = kVideoCodecVP8; codec_info_2.codecSpecific.VP8.simulcastIdx = 1; - EXPECT_CALL(rtp_2, SendOutgoingData(encoded_image._frameType, payload_type, + EXPECT_CALL(rtp_2, SendOutgoingData(encoded_image._frameType, kPayloadType, encoded_image._timeStamp, encoded_image.capture_time_ms_, &payload, encoded_image._length, nullptr, _, _)) @@ -153,10 +161,9 @@ TEST(PayloadRouterTest, SendSimulcast) { TEST(PayloadRouterTest, SimulcastTargetBitrate) { NiceMock rtp_1; NiceMock rtp_2; - std::vector modules; - modules.push_back(&rtp_1); - modules.push_back(&rtp_2); - PayloadRouter payload_router(modules, 42); + std::vector modules = {&rtp_1, &rtp_2}; + + PayloadRouter payload_router(modules, {kSsrc1, kSsrc2}, kPayloadType, {}); payload_router.SetActive(true); BitrateAllocation bitrate; @@ -183,10 +190,8 @@ TEST(PayloadRouterTest, SimulcastTargetBitrateWithInactiveStream) { // Set up two active rtp modules. NiceMock rtp_1; NiceMock rtp_2; - std::vector modules; - modules.push_back(&rtp_1); - modules.push_back(&rtp_2); - PayloadRouter payload_router(modules, 42); + std::vector modules = {&rtp_1, &rtp_2}; + PayloadRouter payload_router(modules, {kSsrc1, kSsrc2}, kPayloadType, {}); payload_router.SetActive(true); // Create bitrate allocation with bitrate only for the first stream. @@ -204,9 +209,8 @@ TEST(PayloadRouterTest, SimulcastTargetBitrateWithInactiveStream) { TEST(PayloadRouterTest, SvcTargetBitrate) { NiceMock rtp_1; - std::vector modules; - modules.push_back(&rtp_1); - PayloadRouter payload_router(modules, 42); + std::vector modules = {&rtp_1}; + PayloadRouter payload_router(modules, {kSsrc1}, kPayloadType, {}); payload_router.SetActive(true); BitrateAllocation bitrate; @@ -220,4 +224,281 @@ TEST(PayloadRouterTest, SvcTargetBitrate) { payload_router.OnBitrateAllocationUpdated(bitrate); } +TEST(PayloadRouterTest, InfoMappedToRtpVideoHeader_Vp8) { + NiceMock rtp1; + NiceMock rtp2; + std::vector modules = {&rtp1, &rtp2}; + PayloadRouter payload_router(modules, {kSsrc1, kSsrc2}, kPayloadType, {}); + payload_router.SetActive(true); + + EncodedImage encoded_image; + encoded_image.rotation_ = kVideoRotation_90; + encoded_image.content_type_ = VideoContentType::SCREENSHARE; + + CodecSpecificInfo codec_info; + memset(&codec_info, 0, sizeof(CodecSpecificInfo)); + codec_info.codecType = kVideoCodecVP8; + codec_info.codecSpecific.VP8.simulcastIdx = 1; + codec_info.codecSpecific.VP8.pictureId = kPictureId; + codec_info.codecSpecific.VP8.temporalIdx = kTemporalIdx; + codec_info.codecSpecific.VP8.tl0PicIdx = kTl0PicIdx; + codec_info.codecSpecific.VP8.keyIdx = kNoKeyIdx; + codec_info.codecSpecific.VP8.layerSync = true; + codec_info.codecSpecific.VP8.nonReference = true; + + EXPECT_CALL(rtp2, SendOutgoingData(_, _, _, _, _, _, nullptr, _, _)) + .WillOnce(Invoke([](Unused, Unused, Unused, Unused, Unused, Unused, + Unused, const RTPVideoHeader* header, Unused) { + EXPECT_EQ(kVideoRotation_90, header->rotation); + EXPECT_EQ(VideoContentType::SCREENSHARE, header->content_type); + EXPECT_EQ(1, header->simulcastIdx); + EXPECT_EQ(kRtpVideoVp8, header->codec); + EXPECT_EQ(kPictureId, header->codecHeader.VP8.pictureId); + EXPECT_EQ(kTemporalIdx, header->codecHeader.VP8.temporalIdx); + EXPECT_EQ(kTl0PicIdx, header->codecHeader.VP8.tl0PicIdx); + EXPECT_EQ(kNoKeyIdx, header->codecHeader.VP8.keyIdx); + EXPECT_TRUE(header->codecHeader.VP8.layerSync); + EXPECT_TRUE(header->codecHeader.VP8.nonReference); + return true; + })); + + EXPECT_EQ( + EncodedImageCallback::Result::OK, + payload_router.OnEncodedImage(encoded_image, &codec_info, nullptr).error); +} + +TEST(PayloadRouterTest, InfoMappedToRtpVideoHeader_H264) { + NiceMock rtp1; + std::vector modules = {&rtp1}; + PayloadRouter payload_router(modules, {kSsrc1}, kPayloadType, {}); + payload_router.SetActive(true); + + EncodedImage encoded_image; + CodecSpecificInfo codec_info; + memset(&codec_info, 0, sizeof(CodecSpecificInfo)); + codec_info.codecType = kVideoCodecH264; + codec_info.codecSpecific.H264.packetization_mode = + H264PacketizationMode::SingleNalUnit; + + EXPECT_CALL(rtp1, SendOutgoingData(_, _, _, _, _, _, nullptr, _, _)) + .WillOnce(Invoke([](Unused, Unused, Unused, Unused, Unused, Unused, + Unused, const RTPVideoHeader* header, Unused) { + EXPECT_EQ(0, header->simulcastIdx); + EXPECT_EQ(kRtpVideoH264, header->codec); + EXPECT_EQ(H264PacketizationMode::SingleNalUnit, + header->codecHeader.H264.packetization_mode); + return true; + })); + + EXPECT_EQ( + EncodedImageCallback::Result::OK, + payload_router.OnEncodedImage(encoded_image, &codec_info, nullptr).error); +} + +TEST(PayloadRouterTest, CreateWithNoPreviousStates) { + NiceMock rtp1; + NiceMock rtp2; + std::vector modules = {&rtp1, &rtp2}; + PayloadRouter payload_router(modules, {kSsrc1, kSsrc2}, kPayloadType, {}); + payload_router.SetActive(true); + + std::map initial_states = + payload_router.GetRtpPayloadStates(); + EXPECT_EQ(2u, initial_states.size()); + EXPECT_NE(initial_states.find(kSsrc1), initial_states.end()); + EXPECT_NE(initial_states.find(kSsrc2), initial_states.end()); +} + +TEST(PayloadRouterTest, CreateWithPreviousStates) { + RtpPayloadState state1; + state1.picture_id = kInitialPictureId1; + RtpPayloadState state2; + state2.picture_id = kInitialPictureId2; + std::map states = {{kSsrc1, state1}, + {kSsrc2, state2}}; + + NiceMock rtp1; + NiceMock rtp2; + std::vector modules = {&rtp1, &rtp2}; + PayloadRouter payload_router(modules, {kSsrc1, kSsrc2}, kPayloadType, states); + payload_router.SetActive(true); + + std::map initial_states = + payload_router.GetRtpPayloadStates(); + EXPECT_EQ(2u, initial_states.size()); + EXPECT_EQ(kInitialPictureId1, initial_states[kSsrc1].picture_id); + EXPECT_EQ(kInitialPictureId2, initial_states[kSsrc2].picture_id); +} + +class PayloadRouterTest : public ::testing::Test { + public: + explicit PayloadRouterTest(const std::string& field_trials) + : override_field_trials_(field_trials) {} + virtual ~PayloadRouterTest() {} + + protected: + virtual void SetUp() { memset(&codec_info_, 0, sizeof(CodecSpecificInfo)); } + + test::ScopedFieldTrials override_field_trials_; + EncodedImage image_; + CodecSpecificInfo codec_info_; +}; + +class TestWithForcedFallbackDisabled : public PayloadRouterTest { + public: + TestWithForcedFallbackDisabled() + : PayloadRouterTest("WebRTC-VP8-Forced-Fallback-Encoder/Disabled/") {} +}; + +class TestWithForcedFallbackEnabled : public PayloadRouterTest { + public: + TestWithForcedFallbackEnabled() + : PayloadRouterTest( + "WebRTC-VP8-Forced-Fallback-Encoder/Enabled-1,2,3,4/") {} +}; + +TEST_F(TestWithForcedFallbackDisabled, PictureIdIsNotChangedForVp8) { + NiceMock rtp; + std::vector modules = {&rtp}; + PayloadRouter router(modules, {kSsrc1}, kPayloadType, {}); + router.SetActive(true); + + codec_info_.codecType = kVideoCodecVP8; + codec_info_.codecSpecific.VP8.pictureId = kPictureId; + + EXPECT_CALL(rtp, SendOutgoingData(_, _, _, _, _, _, nullptr, _, _)) + .WillOnce(Invoke([](Unused, Unused, Unused, Unused, Unused, Unused, + Unused, const RTPVideoHeader* header, Unused) { + EXPECT_EQ(kRtpVideoVp8, header->codec); + EXPECT_EQ(kPictureId, header->codecHeader.VP8.pictureId); + return true; + })); + + EXPECT_EQ(EncodedImageCallback::Result::OK, + router.OnEncodedImage(image_, &codec_info_, nullptr).error); +} + +TEST_F(TestWithForcedFallbackEnabled, PictureIdIsSetForVp8) { + RtpPayloadState state1; + state1.picture_id = kInitialPictureId1; + RtpPayloadState state2; + state2.picture_id = kInitialPictureId2; + std::map states = {{kSsrc1, state1}, + {kSsrc2, state2}}; + + NiceMock rtp1; + NiceMock rtp2; + std::vector modules = {&rtp1, &rtp2}; + PayloadRouter router(modules, {kSsrc1, kSsrc2}, kPayloadType, states); + router.SetActive(true); + + // OnEncodedImage, simulcastIdx: 0. + codec_info_.codecType = kVideoCodecVP8; + codec_info_.codecSpecific.VP8.pictureId = kPictureId; + codec_info_.codecSpecific.VP8.simulcastIdx = 0; + + EXPECT_CALL(rtp1, SendOutgoingData(_, _, _, _, _, _, nullptr, _, _)) + .WillOnce(Invoke([](Unused, Unused, Unused, Unused, Unused, Unused, + Unused, const RTPVideoHeader* header, Unused) { + EXPECT_EQ(kRtpVideoVp8, header->codec); + EXPECT_EQ(kInitialPictureId1, header->codecHeader.VP8.pictureId); + return true; + })); + + EXPECT_EQ(EncodedImageCallback::Result::OK, + router.OnEncodedImage(image_, &codec_info_, nullptr).error); + + // OnEncodedImage, simulcastIdx: 1. + codec_info_.codecSpecific.VP8.pictureId = kPictureId; + codec_info_.codecSpecific.VP8.simulcastIdx = 1; + + EXPECT_CALL(rtp2, SendOutgoingData(_, _, _, _, _, _, nullptr, _, _)) + .WillOnce(Invoke([](Unused, Unused, Unused, Unused, Unused, Unused, + Unused, const RTPVideoHeader* header, Unused) { + EXPECT_EQ(kRtpVideoVp8, header->codec); + EXPECT_EQ(kInitialPictureId2, header->codecHeader.VP8.pictureId); + return true; + })); + + EXPECT_EQ(EncodedImageCallback::Result::OK, + router.OnEncodedImage(image_, &codec_info_, nullptr).error); + + // State should hold next picture id to use. + states = router.GetRtpPayloadStates(); + EXPECT_EQ(2u, states.size()); + EXPECT_EQ(kInitialPictureId1 + 1, states[kSsrc1].picture_id); + EXPECT_EQ(kInitialPictureId2 + 1, states[kSsrc2].picture_id); +} + +TEST_F(TestWithForcedFallbackEnabled, PictureIdWraps) { + RtpPayloadState state1; + state1.picture_id = kMaxTwoBytePictureId; + + NiceMock rtp; + std::vector modules = {&rtp}; + PayloadRouter router(modules, {kSsrc1}, kPayloadType, {{kSsrc1, state1}}); + router.SetActive(true); + + codec_info_.codecType = kVideoCodecVP8; + codec_info_.codecSpecific.VP8.pictureId = kPictureId; + + EXPECT_CALL(rtp, SendOutgoingData(_, _, _, _, _, _, nullptr, _, _)) + .WillOnce(Invoke([](Unused, Unused, Unused, Unused, Unused, Unused, + Unused, const RTPVideoHeader* header, Unused) { + EXPECT_EQ(kRtpVideoVp8, header->codec); + EXPECT_EQ(kMaxTwoBytePictureId, header->codecHeader.VP8.pictureId); + return true; + })); + + EXPECT_EQ(EncodedImageCallback::Result::OK, + router.OnEncodedImage(image_, &codec_info_, nullptr).error); + + // State should hold next picture id to use. + std::map states = router.GetRtpPayloadStates(); + EXPECT_EQ(1u, states.size()); + EXPECT_EQ(0, states[kSsrc1].picture_id); // Wrapped. +} + +TEST_F(TestWithForcedFallbackEnabled, PictureIdIsNotSetIfNoPictureId) { + NiceMock rtp; + std::vector modules = {&rtp}; + PayloadRouter router(modules, {kSsrc1}, kPayloadType, {}); + router.SetActive(true); + + codec_info_.codecType = kVideoCodecVP8; + codec_info_.codecSpecific.VP8.pictureId = kNoPictureId; + + EXPECT_CALL(rtp, SendOutgoingData(_, _, _, _, _, _, nullptr, _, _)) + .WillOnce(Invoke([](Unused, Unused, Unused, Unused, Unused, Unused, + Unused, const RTPVideoHeader* header, Unused) { + EXPECT_EQ(kRtpVideoVp8, header->codec); + EXPECT_EQ(kNoPictureId, header->codecHeader.VP8.pictureId); + return true; + })); + + EXPECT_EQ(EncodedImageCallback::Result::OK, + router.OnEncodedImage(image_, &codec_info_, nullptr).error); +} + +TEST_F(TestWithForcedFallbackEnabled, PictureIdIsNotSetForVp9) { + NiceMock rtp; + std::vector modules = {&rtp}; + PayloadRouter router(modules, {kSsrc1}, kPayloadType, {}); + router.SetActive(true); + + codec_info_.codecType = kVideoCodecVP9; + codec_info_.codecSpecific.VP9.picture_id = kPictureId; + + EXPECT_CALL(rtp, SendOutgoingData(_, _, _, _, _, _, nullptr, _, _)) + .WillOnce(Invoke([](Unused, Unused, Unused, Unused, Unused, Unused, + Unused, const RTPVideoHeader* header, Unused) { + EXPECT_EQ(kRtpVideoVp9, header->codec); + EXPECT_EQ(kPictureId, header->codecHeader.VP9.picture_id); + return true; + })); + + EXPECT_EQ(EncodedImageCallback::Result::OK, + router.OnEncodedImage(image_, &codec_info_, nullptr).error); +} + } // namespace webrtc diff --git a/video/picture_id_tests.cc b/video/picture_id_tests.cc index ba99bd3ecf..3ebf8637c2 100644 --- a/video/picture_id_tests.cc +++ b/video/picture_id_tests.cc @@ -12,9 +12,10 @@ #include "modules/rtp_rtcp/source/rtp_format.h" #include "modules/video_coding/sequence_number_util.h" #include "test/call_test.h" +#include "test/field_trial.h" namespace webrtc { - +namespace { const int kFrameMaxWidth = 1280; const int kFrameMaxHeight = 720; const int kFrameRate = 30; @@ -24,6 +25,10 @@ const int kMinPacketsToObserve = 10; const int kEncoderBitrateBps = 100000; const uint32_t kPictureIdWraparound = (1 << 15); +const char kVp8ForcedFallbackEncoderEnabled[] = + "WebRTC-VP8-Forced-Fallback-Encoder/Enabled/"; +} // namespace + class PictureIdObserver : public test::RtpRtcpObserver { public: PictureIdObserver() @@ -132,9 +137,10 @@ class PictureIdObserver : public test::RtpRtcpObserver { std::set observed_ssrcs_ RTC_GUARDED_BY(crit_); }; -class PictureIdTest : public test::CallTest { +class PictureIdTest : public test::CallTest, + public ::testing::WithParamInterface { public: - PictureIdTest() {} + PictureIdTest() : scoped_field_trial_(GetParam()) {} virtual ~PictureIdTest() { EXPECT_EQ(nullptr, video_send_stream_); @@ -156,9 +162,15 @@ class PictureIdTest : public test::CallTest { const std::vector& ssrc_counts); private: + test::ScopedFieldTrials scoped_field_trial_; PictureIdObserver observer; }; +INSTANTIATE_TEST_CASE_P(TestWithForcedFallbackEncoderEnabled, + PictureIdTest, + ::testing::Values(kVp8ForcedFallbackEncoderEnabled, + "")); + // Use a special stream factory to ensure that all simulcast streams are being // sent. class VideoStreamFactory @@ -301,19 +313,19 @@ void PictureIdTest::TestPictureIdIncreaseAfterRecreateStreams( }); } -TEST_F(PictureIdTest, PictureIdContinuousAfterReconfigureVp8) { +TEST_P(PictureIdTest, PictureIdContinuousAfterReconfigureVp8) { std::unique_ptr encoder(VP8Encoder::Create()); SetupEncoder(encoder.get()); TestPictureIdContinuousAfterReconfigure({1, 3, 3, 1, 1}); } -TEST_F(PictureIdTest, PictureIdIncreasingAfterRecreateStreamVp8) { +TEST_P(PictureIdTest, PictureIdIncreasingAfterRecreateStreamVp8) { std::unique_ptr encoder(VP8Encoder::Create()); SetupEncoder(encoder.get()); TestPictureIdIncreaseAfterRecreateStreams({1, 3, 3, 1, 1}); } -TEST_F(PictureIdTest, PictureIdIncreasingAfterStreamCountChangeVp8) { +TEST_P(PictureIdTest, PictureIdIncreasingAfterStreamCountChangeVp8) { std::unique_ptr encoder(VP8Encoder::Create()); // Make sure that that the picture id is not reset if the stream count goes // down and then up. @@ -322,7 +334,7 @@ TEST_F(PictureIdTest, PictureIdIncreasingAfterStreamCountChangeVp8) { TestPictureIdContinuousAfterReconfigure(ssrc_counts); } -TEST_F(PictureIdTest, +TEST_P(PictureIdTest, PictureIdContinuousAfterReconfigureSimulcastEncoderAdapter) { cricket::InternalEncoderFactory internal_encoder_factory; SimulcastEncoderAdapter simulcast_encoder_adapter(&internal_encoder_factory); @@ -330,7 +342,7 @@ TEST_F(PictureIdTest, TestPictureIdContinuousAfterReconfigure({1, 3, 3, 1, 1}); } -TEST_F(PictureIdTest, +TEST_P(PictureIdTest, PictureIdIncreasingAfterRecreateStreamSimulcastEncoderAdapter) { cricket::InternalEncoderFactory internal_encoder_factory; SimulcastEncoderAdapter simulcast_encoder_adapter(&internal_encoder_factory); @@ -341,7 +353,7 @@ TEST_F(PictureIdTest, // When using the simulcast encoder adapter, the picture id is randomly set // when the ssrc count is reduced and then increased. This means that we are // not spec compliant in that particular case. -TEST_F( +TEST_P( PictureIdTest, DISABLED_PictureIdIncreasingAfterStreamCountChangeSimulcastEncoderAdapter) { cricket::InternalEncoderFactory internal_encoder_factory; diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index e9ee6f3a96..f0380b2f3c 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -208,18 +208,20 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, public VideoStreamEncoder::EncoderSink, public VideoBitrateAllocationObserver { public: - VideoSendStreamImpl(SendStatisticsProxy* stats_proxy, - rtc::TaskQueue* worker_queue, - CallStats* call_stats, - RtpTransportControllerSendInterface* transport, - BitrateAllocator* bitrate_allocator, - SendDelayStats* send_delay_stats, - VideoStreamEncoder* video_stream_encoder, - RtcEventLog* event_log, - const VideoSendStream::Config* config, - int initial_encoder_max_bitrate, - std::map suspended_ssrcs, - VideoEncoderConfig::ContentType content_type); + VideoSendStreamImpl( + SendStatisticsProxy* stats_proxy, + rtc::TaskQueue* worker_queue, + CallStats* call_stats, + RtpTransportControllerSendInterface* transport, + BitrateAllocator* bitrate_allocator, + SendDelayStats* send_delay_stats, + VideoStreamEncoder* video_stream_encoder, + RtcEventLog* event_log, + const VideoSendStream::Config* config, + int initial_encoder_max_bitrate, + std::map suspended_ssrcs, + std::map suspended_payload_states, + VideoEncoderConfig::ContentType content_type); ~VideoSendStreamImpl() override; // RegisterProcessThread register |module_process_thread| with those objects @@ -236,6 +238,7 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, void Stop(); VideoSendStream::RtpStateMap GetRtpStates() const; + VideoSendStream::RtpPayloadStateMap GetRtpPayloadStates() const; void EnableEncodedFrameRecording(const std::vector& files, size_t byte_limit); @@ -338,20 +341,22 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, // an object on the correct task queue. class VideoSendStream::ConstructionTask : public rtc::QueuedTask { public: - ConstructionTask(std::unique_ptr* send_stream, - rtc::Event* done_event, - SendStatisticsProxy* stats_proxy, - VideoStreamEncoder* video_stream_encoder, - ProcessThread* module_process_thread, - CallStats* call_stats, - RtpTransportControllerSendInterface* transport, - BitrateAllocator* bitrate_allocator, - SendDelayStats* send_delay_stats, - RtcEventLog* event_log, - const VideoSendStream::Config* config, - int initial_encoder_max_bitrate, - const std::map& suspended_ssrcs, - VideoEncoderConfig::ContentType content_type) + ConstructionTask( + std::unique_ptr* send_stream, + rtc::Event* done_event, + SendStatisticsProxy* stats_proxy, + VideoStreamEncoder* video_stream_encoder, + ProcessThread* module_process_thread, + CallStats* call_stats, + RtpTransportControllerSendInterface* transport, + BitrateAllocator* bitrate_allocator, + SendDelayStats* send_delay_stats, + RtcEventLog* event_log, + const VideoSendStream::Config* config, + int initial_encoder_max_bitrate, + const std::map& suspended_ssrcs, + const std::map& suspended_payload_states, + VideoEncoderConfig::ContentType content_type) : send_stream_(send_stream), done_event_(done_event), stats_proxy_(stats_proxy), @@ -364,6 +369,7 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { config_(config), initial_encoder_max_bitrate_(initial_encoder_max_bitrate), suspended_ssrcs_(suspended_ssrcs), + suspended_payload_states_(suspended_payload_states), content_type_(content_type) {} ~ConstructionTask() override { done_event_->Set(); } @@ -374,7 +380,8 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { stats_proxy_, rtc::TaskQueue::Current(), call_stats_, transport_, bitrate_allocator_, send_delay_stats_, video_stream_encoder_, event_log_, config_, initial_encoder_max_bitrate_, - std::move(suspended_ssrcs_), content_type_)); + std::move(suspended_ssrcs_), std::move(suspended_payload_states_), + content_type_)); return true; } @@ -390,15 +397,19 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { const VideoSendStream::Config* config_; int initial_encoder_max_bitrate_; std::map suspended_ssrcs_; + std::map suspended_payload_states_; const VideoEncoderConfig::ContentType content_type_; }; class VideoSendStream::DestructAndGetRtpStateTask : public rtc::QueuedTask { public: - DestructAndGetRtpStateTask(VideoSendStream::RtpStateMap* state_map, - std::unique_ptr send_stream, - rtc::Event* done_event) + DestructAndGetRtpStateTask( + VideoSendStream::RtpStateMap* state_map, + VideoSendStream::RtpPayloadStateMap* payload_state_map, + std::unique_ptr send_stream, + rtc::Event* done_event) : state_map_(state_map), + payload_state_map_(payload_state_map), send_stream_(std::move(send_stream)), done_event_(done_event) {} @@ -408,12 +419,14 @@ class VideoSendStream::DestructAndGetRtpStateTask : public rtc::QueuedTask { bool Run() override { send_stream_->Stop(); *state_map_ = send_stream_->GetRtpStates(); + *payload_state_map_ = send_stream_->GetRtpPayloadStates(); send_stream_.reset(); done_event_->Set(); return true; } VideoSendStream::RtpStateMap* state_map_; + VideoSendStream::RtpPayloadStateMap* payload_state_map_; std::unique_ptr send_stream_; rtc::Event* done_event_; }; @@ -503,7 +516,8 @@ VideoSendStream::VideoSendStream( RtcEventLog* event_log, VideoSendStream::Config config, VideoEncoderConfig encoder_config, - const std::map& suspended_ssrcs) + const std::map& suspended_ssrcs, + const std::map& suspended_payload_states) : worker_queue_(worker_queue), thread_sync_event_(false /* manual_reset */, false), stats_proxy_(Clock::GetRealTimeClock(), @@ -521,7 +535,7 @@ VideoSendStream::VideoSendStream( &send_stream_, &thread_sync_event_, &stats_proxy_, video_stream_encoder_.get(), module_process_thread, call_stats, transport, bitrate_allocator, send_delay_stats, event_log, &config_, - encoder_config.max_bitrate_bps, suspended_ssrcs, + encoder_config.max_bitrate_bps, suspended_ssrcs, suspended_payload_states, encoder_config.content_type))); // Wait for ConstructionTask to complete so that |send_stream_| can be used. @@ -597,17 +611,18 @@ void VideoSendStream::SignalNetworkState(NetworkState state) { [send_stream, state] { send_stream->SignalNetworkState(state); }); } -VideoSendStream::RtpStateMap VideoSendStream::StopPermanentlyAndGetRtpStates() { +void VideoSendStream::StopPermanentlyAndGetRtpStates( + VideoSendStream::RtpStateMap* rtp_state_map, + VideoSendStream::RtpPayloadStateMap* payload_state_map) { RTC_DCHECK_RUN_ON(&thread_checker_); video_stream_encoder_->Stop(); video_stream_encoder_->DeRegisterProcessThread(); - VideoSendStream::RtpStateMap state_map; send_stream_->DeRegisterProcessThread(); worker_queue_->PostTask( std::unique_ptr(new DestructAndGetRtpStateTask( - &state_map, std::move(send_stream_), &thread_sync_event_))); + rtp_state_map, payload_state_map, std::move(send_stream_), + &thread_sync_event_))); thread_sync_event_.Wait(rtc::Event::kForever); - return state_map; } void VideoSendStream::SetTransportOverhead( @@ -642,6 +657,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( const VideoSendStream::Config* config, int initial_encoder_max_bitrate, std::map suspended_ssrcs, + std::map suspended_payload_states, VideoEncoderConfig::ContentType content_type) : send_side_bwe_with_overhead_( webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")), @@ -682,7 +698,9 @@ VideoSendStreamImpl::VideoSendStreamImpl( config_->rtp.ssrcs.size(), transport->keepalive_config())), payload_router_(rtp_rtcp_modules_, - config_->encoder_settings.payload_type), + config_->rtp.ssrcs, + config_->encoder_settings.payload_type, + suspended_payload_states), weak_ptr_factory_(this), overhead_bytes_per_packet_(0), transport_overhead_bytes_per_packet_(0) { @@ -1123,6 +1141,12 @@ std::map VideoSendStreamImpl::GetRtpStates() const { return rtp_states; } +std::map VideoSendStreamImpl::GetRtpPayloadStates() + const { + RTC_DCHECK_RUN_ON(worker_queue_); + return payload_router_.GetRtpPayloadStates(); +} + void VideoSendStreamImpl::SignalNetworkState(NetworkState state) { RTC_DCHECK_RUN_ON(worker_queue_); for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) { diff --git a/video/video_send_stream.h b/video/video_send_stream.h index e6c0fe3bcf..998250cd6e 100644 --- a/video/video_send_stream.h +++ b/video/video_send_stream.h @@ -48,17 +48,19 @@ class VideoSendStreamImpl; // |worker_queue|. class VideoSendStream : public webrtc::VideoSendStream { public: - VideoSendStream(int num_cpu_cores, - ProcessThread* module_process_thread, - rtc::TaskQueue* worker_queue, - CallStats* call_stats, - RtpTransportControllerSendInterface* transport, - BitrateAllocator* bitrate_allocator, - SendDelayStats* send_delay_stats, - RtcEventLog* event_log, - VideoSendStream::Config config, - VideoEncoderConfig encoder_config, - const std::map& suspended_ssrcs); + VideoSendStream( + int num_cpu_cores, + ProcessThread* module_process_thread, + rtc::TaskQueue* worker_queue, + CallStats* call_stats, + RtpTransportControllerSendInterface* transport, + BitrateAllocator* bitrate_allocator, + SendDelayStats* send_delay_stats, + RtcEventLog* event_log, + VideoSendStream::Config config, + VideoEncoderConfig encoder_config, + const std::map& suspended_ssrcs, + const std::map& suspended_payload_states); ~VideoSendStream() override; @@ -76,6 +78,7 @@ class VideoSendStream : public webrtc::VideoSendStream { Stats GetStats() override; typedef std::map RtpStateMap; + typedef std::map RtpPayloadStateMap; // Takes ownership of each file, is responsible for closing them later. // Calling this method will close and finalize any current logs. @@ -86,7 +89,8 @@ class VideoSendStream : public webrtc::VideoSendStream { void EnableEncodedFrameRecording(const std::vector& files, size_t byte_limit) override; - RtpStateMap StopPermanentlyAndGetRtpStates(); + void StopPermanentlyAndGetRtpStates(RtpStateMap* rtp_state_map, + RtpPayloadStateMap* payload_state_map); void SetTransportOverhead(size_t transport_overhead_per_packet);