Remove RegisterExternal{De,En}coder error codes.

Also adds a RTC_CHECK in VideoReceiveStream that verifies that decoders
aren't null, since this will attempt to deregister a codec which would
previously fail with an obscure stack trace not indicating what actually
was wrong.

BUG=webrtc:5249
R=stefan@webrtc.org

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

Cr-Commit-Position: refs/heads/master@{#10821}
This commit is contained in:
Peter Boström
2015-11-27 14:09:07 +01:00
parent 34873b5bb0
commit 795dbe4e0f
12 changed files with 37 additions and 58 deletions

View File

@ -437,19 +437,15 @@ bool VCMCodecDataBase::DeregisterExternalDecoder(uint8_t payload_type) {
// Add the external encoder object to the list of external decoders.
// Won't be registered as a receive codec until RegisterReceiveCodec is called.
bool VCMCodecDataBase::RegisterExternalDecoder(
void VCMCodecDataBase::RegisterExternalDecoder(
VideoDecoder* external_decoder,
uint8_t payload_type,
bool internal_render_timing) {
// Check if payload value already exists, if so - erase old and insert new.
VCMExtDecoderMapItem* ext_decoder = new VCMExtDecoderMapItem(
external_decoder, payload_type, internal_render_timing);
if (!ext_decoder) {
return false;
}
DeregisterExternalDecoder(payload_type);
dec_external_map_[payload_type] = ext_decoder;
return true;
}
bool VCMCodecDataBase::DecoderRegistered() const {

View File

@ -98,7 +98,7 @@ class VCMCodecDataBase {
// |internal_render_timing| is set to true if the |external_decoder| has
// built in rendering which is able to obey the render timestamps of the
// encoded frames.
bool RegisterExternalDecoder(VideoDecoder* external_decoder,
void RegisterExternalDecoder(VideoDecoder* external_decoder,
uint8_t payload_type,
bool internal_render_timing);

View File

@ -181,6 +181,7 @@ public:
//
// Return value : VCM_OK, on success.
// < 0, on error.
// TODO(pbos): Remove return type when unused elsewhere.
virtual int32_t RegisterExternalEncoder(VideoEncoder* externalEncoder,
uint8_t payloadType,
bool internalSource = false) = 0;
@ -334,12 +335,9 @@ public:
// registered to.
// - internalRenderTiming : True if the internal renderer (if any) of the decoder
// object can make sure to render at a given time in ms.
//
// Return value : VCM_OK, on success.
// < 0, on error.
virtual int32_t RegisterExternalDecoder(VideoDecoder* externalDecoder,
uint8_t payloadType,
bool internalRenderTiming) = 0;
virtual void RegisterExternalDecoder(VideoDecoder* externalDecoder,
uint8_t payloadType,
bool internalRenderTiming) = 0;
// Register a receive callback. Will be called whenever there is a new frame ready
// for rendering.

View File

@ -127,8 +127,9 @@ class VideoCodingModuleImpl : public VideoCodingModule {
int32_t RegisterExternalEncoder(VideoEncoder* externalEncoder,
uint8_t payloadType,
bool internalSource) override {
return sender_.RegisterExternalEncoder(externalEncoder, payloadType,
internalSource);
sender_.RegisterExternalEncoder(externalEncoder, payloadType,
internalSource);
return 0;
}
int Bitrate(unsigned int* bitrate) const override {
@ -195,11 +196,11 @@ class VideoCodingModuleImpl : public VideoCodingModule {
requireKeyFrame);
}
int32_t RegisterExternalDecoder(VideoDecoder* externalDecoder,
uint8_t payloadType,
bool internalRenderTiming) override {
return receiver_.RegisterExternalDecoder(externalDecoder, payloadType,
internalRenderTiming);
void RegisterExternalDecoder(VideoDecoder* externalDecoder,
uint8_t payloadType,
bool internalRenderTiming) override {
receiver_.RegisterExternalDecoder(externalDecoder, payloadType,
internalRenderTiming);
}
int32_t RegisterReceiveCallback(

View File

@ -82,9 +82,9 @@ class VideoSender {
// Same as SendCodecBlocking. Try to use GetSendCodec() instead.
VideoCodecType SendCodecBlocking() const;
int32_t RegisterExternalEncoder(VideoEncoder* externalEncoder,
uint8_t payloadType,
bool internalSource);
void RegisterExternalEncoder(VideoEncoder* externalEncoder,
uint8_t payloadType,
bool internalSource);
int Bitrate(unsigned int* bitrate) const;
int FrameRate(unsigned int* framerate) const;
@ -150,9 +150,9 @@ class VideoReceiver {
int32_t numberOfCores,
bool requireKeyFrame);
int32_t RegisterExternalDecoder(VideoDecoder* externalDecoder,
uint8_t payloadType,
bool internalRenderTiming);
void RegisterExternalDecoder(VideoDecoder* externalDecoder,
uint8_t payloadType,
bool internalRenderTiming);
int32_t RegisterReceiveCallback(VCMReceiveCallback* receiveCallback);
int32_t RegisterReceiveStatisticsCallback(
VCMReceiveStatisticsCallback* receiveStats);

View File

@ -44,9 +44,7 @@ class VCMRobustnessTest : public ::testing::Test {
ASSERT_EQ(0, vcm_->RegisterPacketRequestCallback(&request_callback_));
ASSERT_EQ(VCM_OK, vcm_->Codec(kVideoCodecVP8, &video_codec_));
ASSERT_EQ(VCM_OK, vcm_->RegisterReceiveCodec(&video_codec_, 1));
ASSERT_EQ(VCM_OK, vcm_->RegisterExternalDecoder(&decoder_,
video_codec_.plType,
true));
vcm_->RegisterExternalDecoder(&decoder_, video_codec_.plType, true);
}
virtual void TearDown() {

View File

@ -237,19 +237,17 @@ int32_t VideoReceiver::RegisterDecoderTimingCallback(
// Register an externally defined decoder/render object.
// Can be a decoder only or a decoder coupled with a renderer.
int32_t VideoReceiver::RegisterExternalDecoder(VideoDecoder* externalDecoder,
void VideoReceiver::RegisterExternalDecoder(VideoDecoder* externalDecoder,
uint8_t payloadType,
bool internalRenderTiming) {
CriticalSectionScoped cs(_receiveCritSect);
if (externalDecoder == NULL) {
// Make sure the VCM updates the decoder next time it decodes.
_decoder = NULL;
return _codecDataBase.DeregisterExternalDecoder(payloadType) ? 0 : -1;
RTC_CHECK(_codecDataBase.DeregisterExternalDecoder(payloadType));
}
return _codecDataBase.RegisterExternalDecoder(
externalDecoder, payloadType, internalRenderTiming)
? 0
: -1;
_codecDataBase.RegisterExternalDecoder(externalDecoder, payloadType,
internalRenderTiming);
}
// Register a frame type request callback.

View File

@ -34,8 +34,7 @@ class TestVideoReceiver : public ::testing::Test {
virtual void SetUp() {
receiver_.reset(new VideoReceiver(&clock_, &event_factory_));
EXPECT_EQ(0, receiver_->RegisterExternalDecoder(&decoder_,
kUnusedPayloadType, true));
receiver_->RegisterExternalDecoder(&decoder_, kUnusedPayloadType, true);
const size_t kMaxNackListSize = 250;
const int kMaxPacketAgeToNack = 450;
receiver_->SetNackSettings(kMaxNackListSize, kMaxPacketAgeToNack, 0);

View File

@ -157,7 +157,7 @@ VideoCodecType VideoSender::SendCodecBlocking() const {
// Register an external decoder object.
// This can not be used together with external decoder callbacks.
int32_t VideoSender::RegisterExternalEncoder(VideoEncoder* externalEncoder,
void VideoSender::RegisterExternalEncoder(VideoEncoder* externalEncoder,
uint8_t payloadType,
bool internalSource /*= false*/) {
RTC_DCHECK(main_thread_.CalledOnValidThread());
@ -166,17 +166,16 @@ int32_t VideoSender::RegisterExternalEncoder(VideoEncoder* externalEncoder,
if (externalEncoder == nullptr) {
bool wasSendCodec = false;
const bool ret =
_codecDataBase.DeregisterExternalEncoder(payloadType, &wasSendCodec);
RTC_CHECK(
_codecDataBase.DeregisterExternalEncoder(payloadType, &wasSendCodec));
if (wasSendCodec) {
// Make sure the VCM doesn't use the de-registered codec
_encoder = nullptr;
}
return ret ? 0 : -1;
return;
}
_codecDataBase.RegisterExternalEncoder(
externalEncoder, payloadType, internalSource);
return 0;
}
// Get encode bitrate

View File

@ -207,9 +207,7 @@ class TestVideoSenderWithMockEncoder : public TestVideoSender {
void SetUp() override {
TestVideoSender::SetUp();
EXPECT_EQ(
0,
sender_->RegisterExternalEncoder(&encoder_, kUnusedPayloadType, false));
sender_->RegisterExternalEncoder(&encoder_, kUnusedPayloadType, false);
memset(&settings_, 0, sizeof(settings_));
EXPECT_EQ(0, VideoCodingModule::Codec(kVideoCodecVP8, &settings_));
settings_.numberOfSimulcastStreams = kNumberOfStreams;
@ -300,11 +298,9 @@ TEST_F(TestVideoSenderWithMockEncoder, TestIntraRequests) {
TEST_F(TestVideoSenderWithMockEncoder, TestIntraRequestsInternalCapture) {
// De-register current external encoder.
EXPECT_EQ(0,
sender_->RegisterExternalEncoder(NULL, kUnusedPayloadType, false));
sender_->RegisterExternalEncoder(nullptr, kUnusedPayloadType, false);
// Register encoder with internal capture.
EXPECT_EQ(
0, sender_->RegisterExternalEncoder(&encoder_, kUnusedPayloadType, true));
sender_->RegisterExternalEncoder(&encoder_, kUnusedPayloadType, true);
EXPECT_EQ(0, sender_->RegisterSendCodec(&settings_, 1, 1200));
ExpectIntraRequest(0);
EXPECT_EQ(0, sender_->IntraFrameRequest(0));
@ -384,8 +380,7 @@ class TestVideoSenderWithVp8 : public TestVideoSender {
codec_.startBitrate = codec_bitrate_kbps_;
codec_.maxBitrate = codec_bitrate_kbps_;
encoder_.reset(VP8Encoder::Create());
ASSERT_EQ(0, sender_->RegisterExternalEncoder(encoder_.get(), codec_.plType,
false));
sender_->RegisterExternalEncoder(encoder_.get(), codec_.plType, false);
EXPECT_EQ(0, sender_->RegisterSendCodec(&codec_, 1, 1200));
}

View File

@ -261,6 +261,7 @@ VideoReceiveStream::VideoReceiveStream(
RTC_DCHECK(!config_.decoders.empty());
for (size_t i = 0; i < config_.decoders.size(); ++i) {
const Decoder& decoder = config_.decoders[i];
RTC_CHECK(decoder.decoder);
RTC_CHECK_EQ(0,
vie_channel_->RegisterExternalDecoder(
decoder.payload_type, decoder.decoder, decoder.is_renderer,

View File

@ -435,11 +435,7 @@ int32_t ViEChannel::RegisterExternalDecoder(const uint8_t pl_type,
bool buffered_rendering,
int32_t render_delay) {
RTC_DCHECK(!sender_);
int32_t result;
result = vcm_->RegisterExternalDecoder(decoder, pl_type, buffered_rendering);
if (result != VCM_OK) {
return result;
}
vcm_->RegisterExternalDecoder(decoder, pl_type, buffered_rendering);
return vcm_->SetRenderDelay(render_delay);
}
@ -448,9 +444,7 @@ int32_t ViEChannel::DeRegisterExternalDecoder(const uint8_t pl_type) {
VideoCodec current_receive_codec;
int32_t result = 0;
result = vcm_->ReceiveCodec(&current_receive_codec);
if (vcm_->RegisterExternalDecoder(NULL, pl_type, false) != VCM_OK) {
return -1;
}
vcm_->RegisterExternalDecoder(NULL, pl_type, false);
if (result == 0 && current_receive_codec.plType == pl_type) {
result = vcm_->RegisterReceiveCodec(&current_receive_codec,