diff --git a/video/buffered_frame_decryptor.cc b/video/buffered_frame_decryptor.cc index 99dc74254b..71414af142 100644 --- a/video/buffered_frame_decryptor.cc +++ b/video/buffered_frame_decryptor.cc @@ -20,11 +20,13 @@ namespace webrtc { BufferedFrameDecryptor::BufferedFrameDecryptor( OnDecryptedFrameCallback* decrypted_frame_callback, + OnDecryptionStatusChangeCallback* decryption_status_change_callback, rtc::scoped_refptr frame_decryptor) : generic_descriptor_auth_experiment_( field_trial::IsEnabled("WebRTC-GenericDescriptorAuth")), frame_decryptor_(std::move(frame_decryptor)), - decrypted_frame_callback_(decrypted_frame_callback) {} + decrypted_frame_callback_(decrypted_frame_callback), + decryption_status_change_callback_(decryption_status_change_callback) {} BufferedFrameDecryptor::~BufferedFrameDecryptor() {} @@ -78,9 +80,17 @@ BufferedFrameDecryptor::FrameDecision BufferedFrameDecryptor::DecryptFrame( // Attempt to decrypt the video frame. size_t bytes_written = 0; - if (frame_decryptor_->Decrypt( - cricket::MEDIA_TYPE_VIDEO, /*csrcs=*/{}, additional_data, *frame, - inline_decrypted_bitstream, &bytes_written) != 0) { + const int status = frame_decryptor_->Decrypt( + cricket::MEDIA_TYPE_VIDEO, /*csrcs=*/{}, additional_data, *frame, + inline_decrypted_bitstream, &bytes_written); + + // Optionally call the callback if there was a change in status + if (status != last_status_) { + last_status_ = status; + decryption_status_change_callback_->OnDecryptionStatusChange(status); + } + + if (status != 0) { // Only stash frames if we have never decrypted a frame before. return first_frame_decrypted_ ? FrameDecision::kDrop : FrameDecision::kStash; diff --git a/video/buffered_frame_decryptor.h b/video/buffered_frame_decryptor.h index 82732a2ea8..dd82fb10fa 100644 --- a/video/buffered_frame_decryptor.h +++ b/video/buffered_frame_decryptor.h @@ -32,6 +32,19 @@ class OnDecryptedFrameCallback { std::unique_ptr frame) = 0; }; +// This callback is called each time there is a status change in the decryption +// stream. For example going from a none state to a first decryption or going +// frome a decryptable state to a non decryptable state. +class OnDecryptionStatusChangeCallback { + public: + virtual ~OnDecryptionStatusChangeCallback() = default; + // Called each time the decryption stream status changes. This call is + // blocking so the caller must relinquish the callback quickly. This status + // must match what is specified in the FrameDecryptorInterface file. Notably + // 0 must indicate success and any positive integer is a failure. + virtual void OnDecryptionStatusChange(int status) = 0; +}; + // The BufferedFrameDecryptor is responsible for deciding when to pass // decrypted received frames onto the OnDecryptedFrameCallback. Frames can be // delayed when frame encryption is enabled but the key hasn't arrived yet. In @@ -45,6 +58,7 @@ class BufferedFrameDecryptor final { // Constructs a new BufferedFrameDecryptor that can hold explicit BufferedFrameDecryptor( OnDecryptedFrameCallback* decrypted_frame_callback, + OnDecryptionStatusChangeCallback* decryption_status_change_callback, rtc::scoped_refptr frame_decryptor); ~BufferedFrameDecryptor(); // This object cannot be copied. @@ -71,8 +85,10 @@ class BufferedFrameDecryptor final { const bool generic_descriptor_auth_experiment_; bool first_frame_decrypted_ = false; + int last_status_ = -1; const rtc::scoped_refptr frame_decryptor_; OnDecryptedFrameCallback* const decrypted_frame_callback_; + OnDecryptionStatusChangeCallback* const decryption_status_change_callback_; std::deque> stashed_frames_; }; diff --git a/video/buffered_frame_decryptor_unittest.cc b/video/buffered_frame_decryptor_unittest.cc index 9230b47c88..0719caf142 100644 --- a/video/buffered_frame_decryptor_unittest.cc +++ b/video/buffered_frame_decryptor_unittest.cc @@ -60,6 +60,7 @@ class FakePacketBuffer : public video_coding::PacketBuffer { class BufferedFrameDecryptorTest : public ::testing::Test, public OnDecryptedFrameCallback, + public OnDecryptionStatusChangeCallback, public video_coding::OnAssembledFrameCallback { public: // Implements the OnDecryptedFrameCallbackInterface @@ -68,6 +69,10 @@ class BufferedFrameDecryptorTest decrypted_frame_call_count_++; } + void OnDecryptionStatusChange(int status) { + ++decryption_status_change_count_; + } + // Implements the OnAssembledFrameCallback interface. void OnAssembledFrame( std::unique_ptr frame) override {} @@ -98,10 +103,11 @@ class BufferedFrameDecryptorTest void SetUp() override { fake_packet_data_ = std::vector(100); decrypted_frame_call_count_ = 0; + decryption_status_change_count_ = 0; seq_num_ = 0; mock_frame_decryptor_ = new rtc::RefCountedObject(); buffered_frame_decryptor_ = absl::make_unique( - this, mock_frame_decryptor_.get()); + this, this, mock_frame_decryptor_.get()); } static const size_t kMaxStashedFrames; @@ -111,6 +117,7 @@ class BufferedFrameDecryptorTest rtc::scoped_refptr mock_frame_decryptor_; std::unique_ptr buffered_frame_decryptor_; size_t decrypted_frame_call_count_; + size_t decryption_status_change_count_ = 0; uint16_t seq_num_; }; @@ -124,6 +131,7 @@ TEST_F(BufferedFrameDecryptorTest, CallbackCalledOnSuccessfulDecryption) { .WillOnce(Return(0)); buffered_frame_decryptor_->ManageEncryptedFrame(CreateRtpFrameObject(true)); EXPECT_EQ(decrypted_frame_call_count_, static_cast(1)); + EXPECT_EQ(decryption_status_change_count_, static_cast(1)); } // An initial fail to decrypt should not trigger the callback. @@ -134,6 +142,7 @@ TEST_F(BufferedFrameDecryptorTest, CallbackNotCalledOnFailedDecryption) { .WillOnce(Return(0)); buffered_frame_decryptor_->ManageEncryptedFrame(CreateRtpFrameObject(true)); EXPECT_EQ(decrypted_frame_call_count_, static_cast(0)); + EXPECT_EQ(decryption_status_change_count_, static_cast(1)); } // Initial failures should be stored and retried after the first successful @@ -151,9 +160,11 @@ TEST_F(BufferedFrameDecryptorTest, DelayedCallbackOnBufferedFrames) { // The first decrypt will fail stashing the first frame. buffered_frame_decryptor_->ManageEncryptedFrame(CreateRtpFrameObject(true)); EXPECT_EQ(decrypted_frame_call_count_, static_cast(0)); + EXPECT_EQ(decryption_status_change_count_, static_cast(1)); // The second call will succeed playing back both frames. buffered_frame_decryptor_->ManageEncryptedFrame(CreateRtpFrameObject(false)); EXPECT_EQ(decrypted_frame_call_count_, static_cast(2)); + EXPECT_EQ(decryption_status_change_count_, static_cast(2)); } // Subsequent failure to decrypts after the first successful decryption should @@ -172,13 +183,16 @@ TEST_F(BufferedFrameDecryptorTest, FTDDiscardedAfterFirstSuccess) { // The first decrypt will fail stashing the first frame. buffered_frame_decryptor_->ManageEncryptedFrame(CreateRtpFrameObject(true)); EXPECT_EQ(decrypted_frame_call_count_, static_cast(0)); + EXPECT_EQ(decryption_status_change_count_, static_cast(1)); // The second call will succeed playing back both frames. buffered_frame_decryptor_->ManageEncryptedFrame(CreateRtpFrameObject(false)); EXPECT_EQ(decrypted_frame_call_count_, static_cast(2)); + EXPECT_EQ(decryption_status_change_count_, static_cast(2)); // A new failure call will not result in an additional decrypted frame // callback. buffered_frame_decryptor_->ManageEncryptedFrame(CreateRtpFrameObject(true)); EXPECT_EQ(decrypted_frame_call_count_, static_cast(2)); + EXPECT_EQ(decryption_status_change_count_, static_cast(3)); } // Validate that the maximum number of stashed frames cannot be exceeded even if @@ -195,12 +209,14 @@ TEST_F(BufferedFrameDecryptorTest, MaximumNumberOfFramesStored) { buffered_frame_decryptor_->ManageEncryptedFrame(CreateRtpFrameObject(true)); } EXPECT_EQ(decrypted_frame_call_count_, static_cast(0)); + EXPECT_EQ(decryption_status_change_count_, static_cast(1)); EXPECT_CALL(*mock_frame_decryptor_, Decrypt) .Times(kMaxStashedFrames + 1) .WillRepeatedly(Return(0)); buffered_frame_decryptor_->ManageEncryptedFrame(CreateRtpFrameObject(true)); EXPECT_EQ(decrypted_frame_call_count_, kMaxStashedFrames + 1); + EXPECT_EQ(decryption_status_change_count_, static_cast(2)); } } // namespace webrtc diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index f40df3ecf6..5032f0e980 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -111,7 +111,8 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( packet_router)), complete_frame_callback_(complete_frame_callback), keyframe_request_sender_(keyframe_request_sender), - has_received_frame_(false) { + has_received_frame_(false), + frames_decryptable_(false) { constexpr bool remb_candidate = true; packet_router_->AddReceiveRtpModule(rtp_rtcp_.get(), remb_candidate); @@ -174,7 +175,7 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( if (frame_decryptor != nullptr || config_.crypto_options.sframe.require_frame_encryption) { buffered_frame_decryptor_ = - absl::make_unique(this, frame_decryptor); + absl::make_unique(this, this, frame_decryptor); } } @@ -398,6 +399,10 @@ void RtpVideoStreamReceiver::RequestPacketRetransmit( rtp_rtcp_->SendNack(sequence_numbers); } +bool RtpVideoStreamReceiver::IsDecryptable() const { + return frames_decryptable_.load(); +} + int32_t RtpVideoStreamReceiver::ResendPackets(const uint16_t* sequence_numbers, uint16_t length) { return rtp_rtcp_->SendNACK(sequence_numbers, length); @@ -449,6 +454,10 @@ void RtpVideoStreamReceiver::OnDecryptedFrame( reference_finder_->ManageFrame(std::move(frame)); } +void RtpVideoStreamReceiver::OnDecryptionStatusChange(int status) { + frames_decryptable_.store(status == 0); +} + void RtpVideoStreamReceiver::UpdateRtt(int64_t max_rtt_ms) { if (nack_module_) nack_module_->UpdateRtt(max_rtt_ms); diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h index 16ea612de4..9e50dd3b7b 100644 --- a/video/rtp_video_stream_receiver.h +++ b/video/rtp_video_stream_receiver.h @@ -11,6 +11,7 @@ #ifndef VIDEO_RTP_VIDEO_STREAM_RECEIVER_H_ #define VIDEO_RTP_VIDEO_STREAM_RECEIVER_H_ +#include #include #include #include @@ -62,7 +63,8 @@ class RtpVideoStreamReceiver : public LossNotificationSender, public VCMPacketRequestCallback, public video_coding::OnAssembledFrameCallback, public video_coding::OnCompleteFrameCallback, - public OnDecryptedFrameCallback { + public OnDecryptedFrameCallback, + public OnDecryptionStatusChangeCallback { public: RtpVideoStreamReceiver( Transport* transport, @@ -126,6 +128,12 @@ class RtpVideoStreamReceiver : public LossNotificationSender, bool IsUlpfecEnabled() const; bool IsRetransmissionsEnabled() const; + + // Returns true if a decryptor is attached and frames can be decrypted. + // Updated by OnDecryptionStatusChangeCallback. Note this refers to Frame + // Decryption not SRTP. + bool IsDecryptable() const; + // Don't use, still experimental. void RequestPacketRetransmit(const std::vector& sequence_numbers); @@ -145,6 +153,9 @@ class RtpVideoStreamReceiver : public LossNotificationSender, void OnDecryptedFrame( std::unique_ptr frame) override; + // Implements OnDecryptionStatusChangeCallback. + void OnDecryptionStatusChange(int status) override; + // Called by VideoReceiveStream when stats are updated. void UpdateRtt(int64_t max_rtt_ms); @@ -230,6 +241,7 @@ class RtpVideoStreamReceiver : public LossNotificationSender, // rtp_reference_finder if they are decryptable. std::unique_ptr buffered_frame_decryptor_ RTC_PT_GUARDED_BY(network_tc_); + std::atomic frames_decryptable_; absl::optional last_color_space_; }; diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index 868ae05e8c..9d84fd3147 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -620,7 +620,9 @@ bool VideoReceiveStream::Decode() { last_keyframe_packet_ms && now_ms - *last_keyframe_packet_ms < kMaxWaitForKeyFrameMs; - if (stream_is_active && !receiving_keyframe) { + if (stream_is_active && !receiving_keyframe && + (!config_.crypto_options.sframe.require_frame_encryption || + rtp_video_stream_receiver_.IsDecryptable())) { RTC_LOG(LS_WARNING) << "No decodable frame in " << wait_ms << " ms, requesting keyframe."; RequestKeyFrame();