From 0181b5f8dd719f7b1f938d12875a96e59fef44f5 Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Mon, 9 Sep 2013 08:26:30 +0000 Subject: [PATCH] ExternalVideoDecoder for new VideoEngine API. Implements the ExternalVideoDecoder interface for VideoReceiveStream. Also adds a FakeDecoder used in tests, removing the overhead of running the EngineTest tests with VP8 under Memcheck/TSan, allowing us to enable them under Memcheck/TSan as well. BUG=2346,2312 R=stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/2172004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@4702 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../video_engine_tests.gtest-memcheck.txt | 2 - .../video_engine_tests.gtest-tsan.txt | 4 -- .../codecs/interface/video_codec_interface.h | 5 ++ .../main/source/generic_encoder.cc | 3 + .../internal/video_receive_stream.cc | 56 +++++++++++++---- .../internal/video_receive_stream.h | 2 + .../video_engine/test/common/fake_decoder.cc | 60 +++++++++++++++++++ .../video_engine/test/common/fake_decoder.h | 49 +++++++++++++++ .../video_engine/test/common/fake_encoder.cc | 15 ++++- .../video_engine/test/common/fake_encoder.h | 2 +- webrtc/video_engine/test/engine_tests.cc | 41 ++++++++----- webrtc/video_engine/test/send_stream_tests.cc | 3 +- webrtc/video_engine/test/tests.gypi | 2 + 13 files changed, 203 insertions(+), 41 deletions(-) delete mode 100644 tools/valgrind-webrtc/gtest_exclude/video_engine_tests.gtest-memcheck.txt create mode 100644 webrtc/video_engine/test/common/fake_decoder.cc create mode 100644 webrtc/video_engine/test/common/fake_decoder.h diff --git a/tools/valgrind-webrtc/gtest_exclude/video_engine_tests.gtest-memcheck.txt b/tools/valgrind-webrtc/gtest_exclude/video_engine_tests.gtest-memcheck.txt deleted file mode 100644 index 4289d26a8..000000000 --- a/tools/valgrind-webrtc/gtest_exclude/video_engine_tests.gtest-memcheck.txt +++ /dev/null @@ -1,2 +0,0 @@ -# https://code.google.com/p/webrtc/issues/detail?id=2346 -*EngineTest.ReceivesPliAndRecoversWithNack* diff --git a/tools/valgrind-webrtc/gtest_exclude/video_engine_tests.gtest-tsan.txt b/tools/valgrind-webrtc/gtest_exclude/video_engine_tests.gtest-tsan.txt index 3b208a41d..e3779e81d 100644 --- a/tools/valgrind-webrtc/gtest_exclude/video_engine_tests.gtest-tsan.txt +++ b/tools/valgrind-webrtc/gtest_exclude/video_engine_tests.gtest-tsan.txt @@ -1,6 +1,2 @@ # Tests that are too slow. RampUpTest/* - -# TODO(pbos): Temporary suppressions (issue 2312) -*EngineTest.ReceivesAndRetransmitsNack* -*EngineTest.ReceivesPliAndRecoversWithNack* diff --git a/webrtc/modules/video_coding/codecs/interface/video_codec_interface.h b/webrtc/modules/video_coding/codecs/interface/video_codec_interface.h index f3bee5e85..977647953 100644 --- a/webrtc/modules/video_coding/codecs/interface/video_codec_interface.h +++ b/webrtc/modules/video_coding/codecs/interface/video_codec_interface.h @@ -43,8 +43,13 @@ struct CodecSpecificInfoVP8 int8_t keyIdx; // negative value to skip keyIdx }; +struct CodecSpecificInfoGeneric { + uint8_t simulcast_idx; +}; + union CodecSpecificInfoUnion { + CodecSpecificInfoGeneric generic; CodecSpecificInfoVP8 VP8; }; diff --git a/webrtc/modules/video_coding/main/source/generic_encoder.cc b/webrtc/modules/video_coding/main/source/generic_encoder.cc index beed8bfff..139164ac9 100644 --- a/webrtc/modules/video_coding/main/source/generic_encoder.cc +++ b/webrtc/modules/video_coding/main/source/generic_encoder.cc @@ -265,6 +265,9 @@ void VCMEncodedFrameCallback::CopyCodecSpecific(const CodecSpecificInfo& info, (*rtp)->simulcastIdx = info.codecSpecific.VP8.simulcastIdx; return; } + case kVideoCodecGeneric: + (*rtp)->simulcastIdx = info.codecSpecific.generic.simulcast_idx; + return; default: { // No codec specific info. Change RTP header pointer to NULL. *rtp = NULL; diff --git a/webrtc/video_engine/internal/video_receive_stream.cc b/webrtc/video_engine/internal/video_receive_stream.cc index 69d9cd137..ed24508e8 100644 --- a/webrtc/video_engine/internal/video_receive_stream.cc +++ b/webrtc/video_engine/internal/video_receive_stream.cc @@ -18,6 +18,7 @@ #include "webrtc/video_engine/include/vie_base.h" #include "webrtc/video_engine/include/vie_capture.h" #include "webrtc/video_engine/include/vie_codec.h" +#include "webrtc/video_engine/include/vie_external_codec.h" #include "webrtc/video_engine/include/vie_network.h" #include "webrtc/video_engine/include/vie_render.h" #include "webrtc/video_engine/include/vie_rtp_rtcp.h" @@ -26,10 +27,9 @@ namespace webrtc { namespace internal { -VideoReceiveStream::VideoReceiveStream( - webrtc::VideoEngine* video_engine, - const VideoReceiveStream::Config& config, - newapi::Transport* transport) +VideoReceiveStream::VideoReceiveStream(webrtc::VideoEngine* video_engine, + const VideoReceiveStream::Config& config, + newapi::Transport* transport) : transport_(transport), config_(config), channel_(-1) { video_engine_base_ = ViEBase::GetInterface(video_engine); // TODO(mflodman): Use the other CreateChannel method. @@ -41,8 +41,7 @@ VideoReceiveStream::VideoReceiveStream( // TODO(pbos): This is not fine grained enough... rtp_rtcp_->SetNACKStatus(channel_, config_.rtp.nack.rtp_history_ms > 0); - rtp_rtcp_->SetKeyFrameRequestMethod(channel_, - kViEKeyFrameRequestPliRtcp); + rtp_rtcp_->SetKeyFrameRequestMethod(channel_, kViEKeyFrameRequestPliRtcp); assert(config_.rtp.ssrc != 0); @@ -61,6 +60,21 @@ VideoReceiveStream::VideoReceiveStream( } } + external_codec_ = ViEExternalCodec::GetInterface(video_engine); + for (size_t i = 0; i < config_.external_decoders.size(); ++i) { + ExternalVideoDecoder* decoder = &config_.external_decoders[i]; + if (external_codec_->RegisterExternalReceiveCodec( + channel_, + decoder->payload_type, + decoder->decoder, + decoder->renderer, + decoder->expected_delay_ms) != + 0) { + // TODO(pbos): Abort gracefully? Can this be a runtime error? + abort(); + } + } + render_ = webrtc::ViERender::GetInterface(video_engine); assert(render_ != NULL); @@ -72,9 +86,15 @@ VideoReceiveStream::VideoReceiveStream( } VideoReceiveStream::~VideoReceiveStream() { + for (size_t i = 0; i < config_.external_decoders.size(); ++i) { + external_codec_->DeRegisterExternalReceiveCodec( + channel_, config_.external_decoders[i].payload_type); + } + network_->DeregisterSendTransport(channel_); video_engine_base_->Release(); + external_codec_->Release(); codec_->Release(); network_->Release(); render_->Release(); @@ -111,15 +131,18 @@ bool VideoReceiveStream::DeliverRtp(const uint8_t* packet, size_t length) { return network_->ReceivedRTPPacket(channel_, packet, length) == 0; } -int VideoReceiveStream::FrameSizeChange(unsigned int width, unsigned int height, +int VideoReceiveStream::FrameSizeChange(unsigned int width, + unsigned int height, unsigned int /*number_of_streams*/) { width_ = width; height_ = height; return 0; } -int VideoReceiveStream::DeliverFrame(uint8_t* frame, int buffer_size, - uint32_t timestamp, int64_t render_time, +int VideoReceiveStream::DeliverFrame(uint8_t* frame, + int buffer_size, + uint32_t timestamp, + int64_t render_time, void* /*handle*/) { if (config_.renderer == NULL) { return 0; @@ -127,8 +150,15 @@ int VideoReceiveStream::DeliverFrame(uint8_t* frame, int buffer_size, I420VideoFrame video_frame; video_frame.CreateEmptyFrame(width_, height_, width_, height_, height_); - ConvertToI420(kI420, frame, 0, 0, width_, height_, buffer_size, - webrtc::kRotateNone, &video_frame); + ConvertToI420(kI420, + frame, + 0, + 0, + width_, + height_, + buffer_size, + webrtc::kRotateNone, + &video_frame); video_frame.set_timestamp(timestamp); video_frame.set_render_time_ms(render_time); @@ -138,8 +168,8 @@ int VideoReceiveStream::DeliverFrame(uint8_t* frame, int buffer_size, if (config_.renderer != NULL) { // TODO(pbos): Add timing to RenderFrame call - config_.renderer - ->RenderFrame(video_frame, render_time - clock_->TimeInMilliseconds()); + config_.renderer->RenderFrame(video_frame, + render_time - clock_->TimeInMilliseconds()); } return 0; diff --git a/webrtc/video_engine/internal/video_receive_stream.h b/webrtc/video_engine/internal/video_receive_stream.h index 0de5e6cbd..e13a2e169 100644 --- a/webrtc/video_engine/internal/video_receive_stream.h +++ b/webrtc/video_engine/internal/video_receive_stream.h @@ -23,6 +23,7 @@ namespace webrtc { class VideoEngine; class ViEBase; class ViECodec; +class ViEExternalCodec; class ViENetwork; class ViERender; class ViERTP_RTCP; @@ -66,6 +67,7 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, ViEBase* video_engine_base_; ViECodec* codec_; + ViEExternalCodec* external_codec_; ViENetwork* network_; ViERender* render_; ViERTP_RTCP* rtp_rtcp_; diff --git a/webrtc/video_engine/test/common/fake_decoder.cc b/webrtc/video_engine/test/common/fake_decoder.cc new file mode 100644 index 000000000..8125e5f39 --- /dev/null +++ b/webrtc/video_engine/test/common/fake_decoder.cc @@ -0,0 +1,60 @@ +/* + * Copyright (c) 2013 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "webrtc/video_engine/test/common/fake_decoder.h" + +#include "testing/gtest/include/gtest/gtest.h" + +namespace webrtc { +namespace test { + +FakeDecoder::FakeDecoder() : callback_(NULL) {} + +int32_t FakeDecoder::InitDecode(const VideoCodec* config, + int32_t number_of_cores) { + config_ = *config; + size_t width = config->width; + size_t height = config->height; + frame_.CreateEmptyFrame(static_cast(width), + static_cast(height), + static_cast(width), + static_cast((width + 1) / 2), + static_cast((width + 1) / 2)); + return WEBRTC_VIDEO_CODEC_OK; +} + +int32_t FakeDecoder::Decode(const EncodedImage& input, + bool missing_frames, + const RTPFragmentationHeader* fragmentation, + const CodecSpecificInfo* codec_specific_info, + int64_t render_time_ms) { + frame_.set_timestamp(input._timeStamp); + frame_.set_render_time_ms(render_time_ms); + + callback_->Decoded(frame_); + + return WEBRTC_VIDEO_CODEC_OK; +} + +int32_t FakeDecoder::RegisterDecodeCompleteCallback( + DecodedImageCallback* callback) { + callback_ = callback; + return WEBRTC_VIDEO_CODEC_OK; +} + +int32_t FakeDecoder::Release() { + return WEBRTC_VIDEO_CODEC_OK; +} +int32_t FakeDecoder::Reset() { + return WEBRTC_VIDEO_CODEC_OK; +} + +} // namespace test +} // namespace webrtc diff --git a/webrtc/video_engine/test/common/fake_decoder.h b/webrtc/video_engine/test/common/fake_decoder.h new file mode 100644 index 000000000..10559528f --- /dev/null +++ b/webrtc/video_engine/test/common/fake_decoder.h @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2013 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef WEBRTC_VIDEO_ENGINE_TEST_COMMON_FAKE_DECODER_H_ +#define WEBRTC_VIDEO_ENGINE_TEST_COMMON_FAKE_DECODER_H_ + +#include + +#include "webrtc/modules/video_coding/codecs/interface/video_codec_interface.h" +#include "webrtc/system_wrappers/interface/clock.h" + +namespace webrtc { +namespace test { + +class FakeDecoder : public VideoDecoder { + public: + FakeDecoder(); + + virtual int32_t InitDecode(const VideoCodec* config, + int32_t number_of_cores) OVERRIDE; + + virtual int32_t Decode(const EncodedImage& input, + bool missing_frames, + const RTPFragmentationHeader* fragmentation, + const CodecSpecificInfo* codec_specific_info, + int64_t render_time_ms) OVERRIDE; + + virtual int32_t RegisterDecodeCompleteCallback( + DecodedImageCallback* callback) OVERRIDE; + + virtual int32_t Release() OVERRIDE; + virtual int32_t Reset() OVERRIDE; + + private: + VideoCodec config_; + I420VideoFrame frame_; + DecodedImageCallback* callback_; +}; +} // namespace test +} // namespace webrtc + +#endif // WEBRTC_VIDEO_ENGINE_TEST_COMMON_FAKE_DECODER_H_ diff --git a/webrtc/video_engine/test/common/fake_encoder.cc b/webrtc/video_engine/test/common/fake_encoder.cc index e355698f9..c5e58f53b 100644 --- a/webrtc/video_engine/test/common/fake_encoder.cc +++ b/webrtc/video_engine/test/common/fake_encoder.cc @@ -25,7 +25,7 @@ FakeEncoder::FakeEncoder(Clock* clock) FakeEncoder::~FakeEncoder() {} -void FakeEncoder::SetCodecStreamSettings(VideoCodec* codec, +void FakeEncoder::SetCodecSettings(VideoCodec* codec, size_t num_streams) { assert(num_streams > 0); assert(num_streams <= kMaxSimulcastStreams); @@ -54,6 +54,10 @@ void FakeEncoder::SetCodecStreamSettings(VideoCodec* codec, 2; codec->minBitrate = stream_settings[0].minBitrate; codec->maxBitrate = sum_of_max_bitrates; + + codec->codecType = kVideoCodecGeneric; + strcpy(codec->plName, "FAKE"); + codec->plType = 125; } int32_t FakeEncoder::InitEncode(const VideoCodec* config, @@ -78,13 +82,17 @@ int32_t FakeEncoder::Encode( } int bits_available = target_bitrate_kbps_ * delta_since_last_encode; + int min_bits = + config_.simulcastStream[0].minBitrate * delta_since_last_encode; + if (bits_available < min_bits) + bits_available = min_bits; last_encode_time_ms_ = time_now_ms; for (int i = 0; i < config_.numberOfSimulcastStreams; ++i) { CodecSpecificInfo specifics; memset(&specifics, 0, sizeof(specifics)); - specifics.codecType = kVideoCodecVP8; - specifics.codecSpecific.VP8.simulcastIdx = i; + specifics.codecType = kVideoCodecGeneric; + specifics.codecSpecific.generic.simulcast_idx = i; int min_stream_bits = config_.simulcastStream[i].minBitrate * delta_since_last_encode; int max_stream_bits = config_.simulcastStream[i].maxBitrate * @@ -100,6 +108,7 @@ int32_t FakeEncoder::Encode( encoded_buffer_, stream_bytes, sizeof(encoded_buffer_)); encoded._timeStamp = input_image.timestamp(); encoded.capture_time_ms_ = input_image.render_time_ms(); + encoded._frameType = (*frame_types)[i]; if (min_stream_bits > bits_available) { encoded._length = 0; encoded._frameType = kSkipFrame; diff --git a/webrtc/video_engine/test/common/fake_encoder.h b/webrtc/video_engine/test/common/fake_encoder.h index d4e19314a..c57a4dceb 100644 --- a/webrtc/video_engine/test/common/fake_encoder.h +++ b/webrtc/video_engine/test/common/fake_encoder.h @@ -24,7 +24,7 @@ class FakeEncoder : public VideoEncoder { explicit FakeEncoder(Clock* clock); virtual ~FakeEncoder(); - static void SetCodecStreamSettings(VideoCodec* codec, size_t num_streams); + static void SetCodecSettings(VideoCodec* codec, size_t num_streams); virtual int32_t InitEncode(const VideoCodec* config, int32_t number_of_cores, diff --git a/webrtc/video_engine/test/engine_tests.cc b/webrtc/video_engine/test/engine_tests.cc index 3d6a81e15..3978c5241 100644 --- a/webrtc/video_engine/test/engine_tests.cc +++ b/webrtc/video_engine/test/engine_tests.cc @@ -23,6 +23,7 @@ #include "webrtc/system_wrappers/interface/event_wrapper.h" #include "webrtc/video_engine/new_include/video_call.h" #include "webrtc/video_engine/test/common/direct_transport.h" +#include "webrtc/video_engine/test/common/fake_decoder.h" #include "webrtc/video_engine/test/common/fake_encoder.h" #include "webrtc/video_engine/test/common/frame_generator.h" #include "webrtc/video_engine/test/common/frame_generator_capturer.h" @@ -151,8 +152,7 @@ TEST_P(RampUpTest, RampUpWithPadding) { test::FakeEncoder encoder(Clock::GetRealTimeClock()); send_config.encoder = &encoder; send_config.internal_source = false; - test::FakeEncoder::SetCodecStreamSettings(&send_config.codec, 3); - send_config.codec.plType = 100; + test::FakeEncoder::SetCodecSettings(&send_config.codec, 3); send_config.pacing = GetParam(); test::GenerateRandomSsrcs(&send_config, &reserved_ssrcs_); @@ -200,7 +200,10 @@ struct EngineTestParams { class EngineTest : public ::testing::TestWithParam { public: - EngineTest() : send_stream_(NULL), receive_stream_(NULL) {} + EngineTest() + : send_stream_(NULL), + receive_stream_(NULL), + fake_encoder_(Clock::GetRealTimeClock()) {} ~EngineTest() { EXPECT_EQ(NULL, send_stream_); @@ -217,17 +220,20 @@ class EngineTest : public ::testing::TestWithParam { } void CreateTestConfigs() { - EngineTestParams params = GetParam(); send_config_ = sender_call_->GetDefaultSendConfig(); receive_config_ = receiver_call_->GetDefaultReceiveConfig(); test::GenerateRandomSsrcs(&send_config_, &reserved_ssrcs_); - send_config_.codec.width = static_cast(params.width); - send_config_.codec.height = static_cast(params.height); - send_config_.codec.minBitrate = params.bitrate.min; - send_config_.codec.startBitrate = params.bitrate.start; - send_config_.codec.maxBitrate = params.bitrate.max; + send_config_.encoder = &fake_encoder_; + send_config_.internal_source = false; + test::FakeEncoder::SetCodecSettings(&send_config_.codec, 1); + receive_config_.codecs.clear(); + receive_config_.codecs.push_back(send_config_.codec); + ExternalVideoDecoder decoder; + decoder.decoder = &fake_decoder_; + decoder.payload_type = send_config_.codec.plType; + receive_config_.external_decoders.push_back(decoder); receive_config_.rtp.ssrc = send_config_.rtp.ssrcs[0]; } @@ -240,11 +246,11 @@ class EngineTest : public ::testing::TestWithParam { } void CreateFrameGenerator() { - EngineTestParams params = GetParam(); frame_generator_capturer_.reset(test::FrameGeneratorCapturer::Create( send_stream_->Input(), - test::FrameGenerator::Create( - params.width, params.height, Clock::GetRealTimeClock()), + test::FrameGenerator::Create(send_config_.codec.width, + send_config_.codec.height, + Clock::GetRealTimeClock()), 30)); } @@ -284,6 +290,9 @@ class EngineTest : public ::testing::TestWithParam { scoped_ptr frame_generator_capturer_; + test::FakeEncoder fake_encoder_; + test::FakeDecoder fake_decoder_; + std::map reserved_ssrcs_; }; @@ -428,9 +437,9 @@ TEST_P(EngineTest, ReceivesAndRetransmitsNack) { StopSending(); - DestroyStreams(); - observer.StopSending(); + + DestroyStreams(); } class PliObserver : public test::RtpRtcpObserver { @@ -543,9 +552,9 @@ void EngineTest::ReceivesPliAndRecovers(int rtp_history_ms) { StopSending(); - DestroyStreams(); - observer.StopSending(); + + DestroyStreams(); } TEST_P(EngineTest, ReceivesPliAndRecoversWithNack) { diff --git a/webrtc/video_engine/test/send_stream_tests.cc b/webrtc/video_engine/test/send_stream_tests.cc index 0b142ae59..1e1ac27c3 100644 --- a/webrtc/video_engine/test/send_stream_tests.cc +++ b/webrtc/video_engine/test/send_stream_tests.cc @@ -68,8 +68,7 @@ class VideoSendStreamTest : public ::testing::Test { VideoSendStream::Config config = call->GetDefaultSendConfig(); config.encoder = &fake_encoder_; config.internal_source = false; - test::FakeEncoder::SetCodecStreamSettings(&config.codec, 1); - config.codec.plType = 100; + test::FakeEncoder::SetCodecSettings(&config.codec, 1); return config; } diff --git a/webrtc/video_engine/test/tests.gypi b/webrtc/video_engine/test/tests.gypi index f0e8e5fd6..8a730ec4c 100644 --- a/webrtc/video_engine/test/tests.gypi +++ b/webrtc/video_engine/test/tests.gypi @@ -14,6 +14,8 @@ 'sources': [ 'common/direct_transport.cc', 'common/direct_transport.h', + 'common/fake_decoder.cc', + 'common/fake_decoder.h', 'common/fake_encoder.cc', 'common/fake_encoder.h', 'common/file_capturer.cc',