From 4bece9a4f0b524b9f6ab53471d5e7a86a865ed79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85sa=20Persson?= Date: Fri, 6 Oct 2017 10:04:04 +0200 Subject: [PATCH] Set RTPVideoHeader picture id in PayloadRouter if forced fallback for VP8 is enabled. The SW and HW encoder have separate picture id sequences. Set picture id to not cause sequence discontinuties at encoder changes. Bug: webrtc:6634 Change-Id: Ie47168791399303d88cbec3ef6ae8ef8c16ced30 Reviewed-on: https://webrtc-review.googlesource.com/5481 Commit-Queue: Stefan Holmer Reviewed-by: Stefan Holmer Reviewed-by: Rasmus Brandt Cr-Commit-Position: refs/heads/master@{#20188} --- call/call.cc | 22 +- common_types.h | 5 + video/payload_router.cc | 67 ++++++- video/payload_router.h | 12 +- video/payload_router_unittest.cc | 333 ++++++++++++++++++++++++++++--- video/picture_id_tests.cc | 30 ++- video/video_send_stream.cc | 98 +++++---- video/video_send_stream.h | 28 +-- 8 files changed, 501 insertions(+), 94 deletions(-) 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);