zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Modify BufferedFrameDecryptor to perform fine grained key requests.

Browse Source 
The current Key Frame request system doesn't take into account failed
decryptions and this can lead to WebRTC spamming new key frame requests when
the issue is actually in the decryptor layer. To prevent this if frame
decryption is required for the PeerConnection key frame requests will not be
sent at 200ms intervals but will wait until the stream is decryptable before
utilizing this logic.

Bug: webrtc:10330
Change-Id: I188a21dfd142dec6175d9def95f39a2bc517017c
Reviewed-on: https://webrtc-review.googlesource.com/c/123414
Commit-Queue: Benjamin Wright <benwright@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26931}
This commit is contained in:
Benjamin Wright
2019-03-01 11:01:59 -08:00
committed by Commit Bot
parent e4bd9a13d8
commit 52426edef1
6 changed files with 74 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
video/buffered_frame_decryptor.cc
View File

@ -20,11 +20,13 @@ namespace webrtc {
BufferedFrameDecryptor::BufferedFrameDecryptor(
OnDecryptedFrameCallback* decrypted_frame_callback,
OnDecryptionStatusChangeCallback* decryption_status_change_callback,
rtc::scoped_refptr<FrameDecryptorInterface> 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;

16
video/buffered_frame_decryptor.h
View File

@ -32,6 +32,19 @@ class OnDecryptedFrameCallback {
std::unique_ptr<video_coding::RtpFrameObject> 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<FrameDecryptorInterface> 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<FrameDecryptorInterface> frame_decryptor_;
OnDecryptedFrameCallback* const decrypted_frame_callback_;
OnDecryptionStatusChangeCallback* const decryption_status_change_callback_;
std::deque<std::unique_ptr<video_coding::RtpFrameObject>> stashed_frames_;
};

18
video/buffered_frame_decryptor_unittest.cc
View File

@ -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<video_coding::RtpFrameObject> frame) override {}
@ -98,10 +103,11 @@ class BufferedFrameDecryptorTest
void SetUp() override {
fake_packet_data_ = std::vector<uint8_t>(100);
decrypted_frame_call_count_ = 0;
decryption_status_change_count_ = 0;
seq_num_ = 0;
mock_frame_decryptor_ = new rtc::RefCountedObject<MockFrameDecryptor>();
buffered_frame_decryptor_ = absl::make_unique<BufferedFrameDecryptor>(
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<MockFrameDecryptor> mock_frame_decryptor_;
std::unique_ptr<BufferedFrameDecryptor> 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<size_t>(1));
EXPECT_EQ(decryption_status_change_count_, static_cast<size_t>(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<size_t>(0));
EXPECT_EQ(decryption_status_change_count_, static_cast<size_t>(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<size_t>(0));
EXPECT_EQ(decryption_status_change_count_, static_cast<size_t>(1));
// The second call will succeed playing back both frames.
buffered_frame_decryptor_->ManageEncryptedFrame(CreateRtpFrameObject(false));
EXPECT_EQ(decrypted_frame_call_count_, static_cast<size_t>(2));
EXPECT_EQ(decryption_status_change_count_, static_cast<size_t>(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<size_t>(0));
EXPECT_EQ(decryption_status_change_count_, static_cast<size_t>(1));
// The second call will succeed playing back both frames.
buffered_frame_decryptor_->ManageEncryptedFrame(CreateRtpFrameObject(false));
EXPECT_EQ(decrypted_frame_call_count_, static_cast<size_t>(2));
EXPECT_EQ(decryption_status_change_count_, static_cast<size_t>(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<size_t>(2));
EXPECT_EQ(decryption_status_change_count_, static_cast<size_t>(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<size_t>(0));
EXPECT_EQ(decryption_status_change_count_, static_cast<size_t>(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<size_t>(2));
}
} // namespace webrtc

13
video/rtp_video_stream_receiver.cc
View File

@ -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<BufferedFrameDecryptor>(this, frame_decryptor);
absl::make_unique<BufferedFrameDecryptor>(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);

14
video/rtp_video_stream_receiver.h
View File

@ -11,6 +11,7 @@
#ifndef VIDEO_RTP_VIDEO_STREAM_RECEIVER_H_
#define VIDEO_RTP_VIDEO_STREAM_RECEIVER_H_
#include <atomic>
#include <list>
#include <map>
#include <memory>
@ -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<uint16_t>& sequence_numbers);
@ -145,6 +153,9 @@ class RtpVideoStreamReceiver : public LossNotificationSender,
void OnDecryptedFrame(
std::unique_ptr<video_coding::RtpFrameObject> 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<BufferedFrameDecryptor> buffered_frame_decryptor_
RTC_PT_GUARDED_BY(network_tc_);
std::atomic<bool> frames_decryptable_;
absl::optional<ColorSpace> last_color_space_;
};

4
video/video_receive_stream.cc
View File

@ -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();