Simplify EncoderStateFeedback.

EncoderStateFeedback is now only connected to one encoder, so remove map
and other complexity to deliver feedback more directly.

BUG=webrtc:5494
R=danilchap@webrtc.org

Review URL: https://codereview.webrtc.org/1706803002 .

Cr-Commit-Position: refs/heads/master@{#11687}
This commit is contained in:
Peter Boström 2016-02-19 17:36:01 +01:00
parent 9674d7cb89
commit 45c44f0b94
4 changed files with 71 additions and 195 deletions

View File

@ -10,113 +10,75 @@
#include "webrtc/video/encoder_state_feedback.h"
#include <assert.h>
#include "webrtc/base/checks.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "webrtc/video/vie_encoder.h"
namespace webrtc {
// Helper class registered at the RTP module relaying callbacks to
// EncoderStatFeedback.
class EncoderStateFeedbackObserver : public RtcpIntraFrameObserver {
public:
explicit EncoderStateFeedbackObserver(EncoderStateFeedback* owner)
: owner_(owner) {}
~EncoderStateFeedbackObserver() {}
EncoderStateFeedback::EncoderStateFeedback() : vie_encoder_(nullptr) {}
// Implements RtcpIntraFrameObserver.
virtual void OnReceivedIntraFrameRequest(uint32_t ssrc) {
owner_->OnReceivedIntraFrameRequest(ssrc);
}
virtual void OnReceivedSLI(uint32_t ssrc, uint8_t picture_id) {
owner_->OnReceivedSLI(ssrc, picture_id);
}
virtual void OnReceivedRPSI(uint32_t ssrc, uint64_t picture_id) {
owner_->OnReceivedRPSI(ssrc, picture_id);
}
virtual void OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc) {
owner_->OnLocalSsrcChanged(old_ssrc, new_ssrc);
}
private:
EncoderStateFeedback* owner_;
};
EncoderStateFeedback::EncoderStateFeedback()
: observer_(new EncoderStateFeedbackObserver(this)) {}
EncoderStateFeedback::~EncoderStateFeedback() {
assert(encoders_.empty());
}
void EncoderStateFeedback::AddEncoder(const std::vector<uint32_t>& ssrcs,
ViEEncoder* encoder) {
void EncoderStateFeedback::Init(const std::vector<uint32_t>& ssrcs,
ViEEncoder* encoder) {
RTC_DCHECK(!ssrcs.empty());
rtc::CritScope lock(&crit_);
for (uint32_t ssrc : ssrcs) {
RTC_DCHECK(encoders_.find(ssrc) == encoders_.end());
encoders_[ssrc] = encoder;
}
ssrcs_ = ssrcs;
vie_encoder_ = encoder;
}
void EncoderStateFeedback::RemoveEncoder(const ViEEncoder* encoder) {
void EncoderStateFeedback::TearDown() {
rtc::CritScope lock(&crit_);
SsrcEncoderMap::iterator it = encoders_.begin();
while (it != encoders_.end()) {
if (it->second == encoder) {
encoders_.erase(it++);
} else {
++it;
}
}
RTC_DCHECK(vie_encoder_);
ssrcs_.clear();
vie_encoder_ = nullptr;
}
RtcpIntraFrameObserver* EncoderStateFeedback::GetRtcpIntraFrameObserver() {
return observer_.get();
bool EncoderStateFeedback::HasSsrc(uint32_t ssrc) {
for (uint32_t registered_ssrc : ssrcs_) {
if (registered_ssrc == ssrc)
return true;
}
return false;
}
void EncoderStateFeedback::OnReceivedIntraFrameRequest(uint32_t ssrc) {
rtc::CritScope lock(&crit_);
SsrcEncoderMap::iterator it = encoders_.find(ssrc);
if (it == encoders_.end())
if (!HasSsrc(ssrc))
return;
RTC_DCHECK(vie_encoder_);
it->second->OnReceivedIntraFrameRequest(ssrc);
vie_encoder_->OnReceivedIntraFrameRequest(ssrc);
}
void EncoderStateFeedback::OnReceivedSLI(uint32_t ssrc, uint8_t picture_id) {
rtc::CritScope lock(&crit_);
SsrcEncoderMap::iterator it = encoders_.find(ssrc);
if (it == encoders_.end())
if (!HasSsrc(ssrc))
return;
RTC_DCHECK(vie_encoder_);
it->second->OnReceivedSLI(ssrc, picture_id);
vie_encoder_->OnReceivedSLI(ssrc, picture_id);
}
void EncoderStateFeedback::OnReceivedRPSI(uint32_t ssrc, uint64_t picture_id) {
rtc::CritScope lock(&crit_);
SsrcEncoderMap::iterator it = encoders_.find(ssrc);
if (it == encoders_.end())
if (!HasSsrc(ssrc))
return;
RTC_DCHECK(vie_encoder_);
it->second->OnReceivedRPSI(ssrc, picture_id);
vie_encoder_->OnReceivedRPSI(ssrc, picture_id);
}
// Sending SSRCs for this encoder should never change since they are configured
// once and not reconfigured.
void EncoderStateFeedback::OnLocalSsrcChanged(uint32_t old_ssrc,
uint32_t new_ssrc) {
rtc::CritScope lock(&crit_);
SsrcEncoderMap::iterator it = encoders_.find(old_ssrc);
if (it == encoders_.end() || encoders_.find(new_ssrc) != encoders_.end()) {
if (!RTC_DCHECK_IS_ON)
return;
}
ViEEncoder* encoder = it->second;
encoders_.erase(it);
encoders_[new_ssrc] = encoder;
encoder->OnLocalSsrcChanged(old_ssrc, new_ssrc);
rtc::CritScope lock(&crit_);
if (ssrcs_.empty()) // Encoder not yet attached (or detached for teardown).
return;
// SSRC shouldn't change to something we haven't already registered with the
// encoder.
RTC_DCHECK(HasSsrc(new_ssrc));
}
} // namespace webrtc

View File

@ -14,56 +14,40 @@
#ifndef WEBRTC_VIDEO_ENCODER_STATE_FEEDBACK_H_
#define WEBRTC_VIDEO_ENCODER_STATE_FEEDBACK_H_
#include <map>
#include <vector>
#include "webrtc/base/constructormagic.h"
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "webrtc/typedefs.h"
namespace webrtc {
class EncoderStateFeedbackObserver;
class RtcpIntraFrameObserver;
class ViEEncoder;
class EncoderStateFeedback {
class EncoderStateFeedback : public RtcpIntraFrameObserver {
public:
friend class EncoderStateFeedbackObserver;
EncoderStateFeedback();
~EncoderStateFeedback();
// Adds an encoder to receive feedback for a set of SSRCs.
void AddEncoder(const std::vector<uint32_t>& ssrc, ViEEncoder* encoder);
void Init(const std::vector<uint32_t>& ssrc, ViEEncoder* encoder);
// Removes a registered ViEEncoder.
void RemoveEncoder(const ViEEncoder* encoder);
// Removes the registered encoder. Necessary since RTP modules outlive
// ViEEncoder.
// TODO(pbos): Make sure RTP modules are not running when tearing down
// ViEEncoder, then remove this function.
void TearDown();
// Returns an observer to register at the requesting class. The observer has
// the same lifetime as the EncoderStateFeedback instance.
RtcpIntraFrameObserver* GetRtcpIntraFrameObserver();
protected:
// Called by EncoderStateFeedbackObserver when a new key frame is requested.
void OnReceivedIntraFrameRequest(uint32_t ssrc);
void OnReceivedSLI(uint32_t ssrc, uint8_t picture_id);
void OnReceivedRPSI(uint32_t ssrc, uint64_t picture_id);
void OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc);
void OnReceivedIntraFrameRequest(uint32_t ssrc) override;
void OnReceivedSLI(uint32_t ssrc, uint8_t picture_id) override;
void OnReceivedRPSI(uint32_t ssrc, uint64_t picture_id) override;
void OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc) override;
private:
typedef std::map<uint32_t, ViEEncoder*> SsrcEncoderMap;
bool HasSsrc(uint32_t ssrc) EXCLUSIVE_LOCKS_REQUIRED(crit_);
rtc::CriticalSection crit_;
// Instance registered at the class requesting new key frames.
rtc::scoped_ptr<EncoderStateFeedbackObserver> observer_;
// Maps a unique ssrc to the given encoder.
SsrcEncoderMap encoders_;
RTC_DISALLOW_COPY_AND_ASSIGN(EncoderStateFeedback);
std::vector<uint32_t> ssrcs_ GUARDED_BY(crit_);
ViEEncoder* vie_encoder_ GUARDED_BY(crit_);
};
} // namespace webrtc

View File

@ -15,13 +15,10 @@
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
#include "webrtc/modules/pacing/paced_sender.h"
#include "webrtc/modules/pacing/packet_router.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "webrtc/modules/utility/include/mock/mock_process_thread.h"
#include "webrtc/video/payload_router.h"
#include "webrtc/video/vie_encoder.h"
using ::testing::NiceMock;
@ -51,99 +48,33 @@ class MockVieEncoder : public ViEEncoder {
void(uint32_t old_ssrc, uint32_t new_ssrc));
};
class VieKeyRequestTest : public ::testing::Test {
protected:
VieKeyRequestTest()
: pacer_(Clock::GetRealTimeClock(),
&router_,
BitrateController::kDefaultStartBitrateKbps,
PacedSender::kDefaultPaceMultiplier *
BitrateController::kDefaultStartBitrateKbps,
0) {}
virtual void SetUp() {
process_thread_.reset(new NiceMock<MockProcessThread>);
encoder_state_feedback_.reset(new EncoderStateFeedback());
}
rtc::scoped_ptr<MockProcessThread> process_thread_;
rtc::scoped_ptr<EncoderStateFeedback> encoder_state_feedback_;
PacketRouter router_;
PacedSender pacer_;
};
TEST(VieKeyRequestTest, CreateAndTriggerRequests) {
static const uint32_t kSsrc = 1234;
NiceMock<MockProcessThread> process_thread;
PacketRouter router;
PacedSender pacer(Clock::GetRealTimeClock(), &router,
BitrateController::kDefaultStartBitrateKbps,
PacedSender::kDefaultPaceMultiplier *
BitrateController::kDefaultStartBitrateKbps,
0);
MockVieEncoder encoder(&process_thread, &pacer);
TEST_F(VieKeyRequestTest, CreateAndTriggerRequests) {
const int ssrc = 1234;
MockVieEncoder encoder(process_thread_.get(), &pacer_);
encoder_state_feedback_->AddEncoder(std::vector<uint32_t>(1, ssrc), &encoder);
EncoderStateFeedback encoder_state_feedback;
encoder_state_feedback.Init(std::vector<uint32_t>(1, kSsrc), &encoder);
EXPECT_CALL(encoder, OnReceivedIntraFrameRequest(ssrc))
EXPECT_CALL(encoder, OnReceivedIntraFrameRequest(kSsrc))
.Times(1);
encoder_state_feedback_->GetRtcpIntraFrameObserver()->
OnReceivedIntraFrameRequest(ssrc);
encoder_state_feedback.OnReceivedIntraFrameRequest(kSsrc);
const uint8_t sli_picture_id = 3;
EXPECT_CALL(encoder, OnReceivedSLI(ssrc, sli_picture_id))
EXPECT_CALL(encoder, OnReceivedSLI(kSsrc, sli_picture_id))
.Times(1);
encoder_state_feedback_->GetRtcpIntraFrameObserver()->OnReceivedSLI(
ssrc, sli_picture_id);
encoder_state_feedback.OnReceivedSLI(kSsrc, sli_picture_id);
const uint64_t rpsi_picture_id = 9;
EXPECT_CALL(encoder, OnReceivedRPSI(ssrc, rpsi_picture_id))
EXPECT_CALL(encoder, OnReceivedRPSI(kSsrc, rpsi_picture_id))
.Times(1);
encoder_state_feedback_->GetRtcpIntraFrameObserver()->OnReceivedRPSI(
ssrc, rpsi_picture_id);
encoder_state_feedback_->RemoveEncoder(&encoder);
}
// Register multiple encoders and make sure the request is relayed to correct
// ViEEncoder.
TEST_F(VieKeyRequestTest, MultipleEncoders) {
const int ssrc_1 = 1234;
const int ssrc_2 = 5678;
MockVieEncoder encoder_1(process_thread_.get(), &pacer_);
MockVieEncoder encoder_2(process_thread_.get(), &pacer_);
encoder_state_feedback_->AddEncoder(std::vector<uint32_t>(1, ssrc_1),
&encoder_1);
encoder_state_feedback_->AddEncoder(std::vector<uint32_t>(1, ssrc_2),
&encoder_2);
EXPECT_CALL(encoder_1, OnReceivedIntraFrameRequest(ssrc_1))
.Times(1);
EXPECT_CALL(encoder_2, OnReceivedIntraFrameRequest(ssrc_2))
.Times(1);
encoder_state_feedback_->GetRtcpIntraFrameObserver()->
OnReceivedIntraFrameRequest(ssrc_1);
encoder_state_feedback_->GetRtcpIntraFrameObserver()->
OnReceivedIntraFrameRequest(ssrc_2);
const uint8_t sli_pid_1 = 3;
const uint8_t sli_pid_2 = 4;
EXPECT_CALL(encoder_1, OnReceivedSLI(ssrc_1, sli_pid_1))
.Times(1);
EXPECT_CALL(encoder_2, OnReceivedSLI(ssrc_2, sli_pid_2))
.Times(1);
encoder_state_feedback_->GetRtcpIntraFrameObserver()->OnReceivedSLI(
ssrc_1, sli_pid_1);
encoder_state_feedback_->GetRtcpIntraFrameObserver()->OnReceivedSLI(
ssrc_2, sli_pid_2);
const uint64_t rpsi_pid_1 = 9;
const uint64_t rpsi_pid_2 = 10;
EXPECT_CALL(encoder_1, OnReceivedRPSI(ssrc_1, rpsi_pid_1))
.Times(1);
EXPECT_CALL(encoder_2, OnReceivedRPSI(ssrc_2, rpsi_pid_2))
.Times(1);
encoder_state_feedback_->GetRtcpIntraFrameObserver()->OnReceivedRPSI(
ssrc_1, rpsi_pid_1);
encoder_state_feedback_->GetRtcpIntraFrameObserver()->OnReceivedRPSI(
ssrc_2, rpsi_pid_2);
encoder_state_feedback_->RemoveEncoder(&encoder_1);
EXPECT_CALL(encoder_2, OnReceivedIntraFrameRequest(ssrc_2))
.Times(1);
encoder_state_feedback_->GetRtcpIntraFrameObserver()->
OnReceivedIntraFrameRequest(ssrc_2);
encoder_state_feedback_->RemoveEncoder(&encoder_2);
encoder_state_feedback.OnReceivedRPSI(kSsrc, rpsi_picture_id);
}
} // namespace webrtc

View File

@ -176,7 +176,7 @@ VideoSendStream::VideoSendStream(
module_process_thread_,
&payload_router_,
nullptr,
encoder_feedback_.GetRtcpIntraFrameObserver(),
&encoder_feedback_,
congestion_controller_->GetBitrateController()
->CreateRtcpBandwidthObserver(),
congestion_controller_->GetTransportFeedbackObserver(),
@ -209,6 +209,7 @@ VideoSendStream::VideoSendStream(
RTC_DCHECK(remb_);
RTC_CHECK(vie_encoder_.Init());
encoder_feedback_.Init(config_.rtp.ssrcs, &vie_encoder_);
RTC_CHECK(vie_channel_.Init() == 0);
vcm_->RegisterProtectionCallback(vie_channel_.vcm_protection_callback());
@ -288,8 +289,6 @@ VideoSendStream::VideoSendStream(
if (config_.suspend_below_min_bitrate)
vie_encoder_.SuspendBelowMinBitrate();
encoder_feedback_.AddEncoder(config_.rtp.ssrcs, &vie_encoder_);
vie_channel_.RegisterSendChannelRtcpStatisticsCallback(&stats_proxy_);
vie_channel_.RegisterSendChannelRtpStatisticsCallback(&stats_proxy_);
vie_channel_.RegisterRtcpPacketTypeCounterObserver(&stats_proxy_);
@ -319,9 +318,9 @@ VideoSendStream::~VideoSendStream() {
rtp_module->SetREMBStatus(false);
remb_->RemoveRembSender(rtp_module);
// Remove the feedback, stop all encoding threads and processing. This must be
// done before deleting the channel.
encoder_feedback_.RemoveEncoder(&vie_encoder_);
// ViEChannel outlives ViEEncoder so remove encoder from feedback before
// destruction.
encoder_feedback_.TearDown();
congestion_controller_->GetRemoteBitrateEstimator(false)->RemoveStream(
vie_receiver_->GetRemoteSsrc());