From 257dc39841c4d29e6a360f2dd10af336a45072a3 Mon Sep 17 00:00:00 2001 From: hta Date: Tue, 25 Oct 2016 09:05:06 -0700 Subject: [PATCH] Refactoring: Hide VideoCodec.codecSpecific as "private" This refactoring allows runtime checks that functions that access codec specific information are using the correct union member. The API also allows replacing the union with another implementation without changes at calling sites. BUG=webrtc:6603 Review-Url: https://codereview.webrtc.org/2001533003 Cr-Commit-Position: refs/heads/master@{#14775} --- webrtc/common_types.cc | 49 +++++++++++++++++++ webrtc/common_types.h | 25 ++++++++-- webrtc/modules/video_coding/codec_database.cc | 21 ++++---- .../test/videoprocessor_integrationtest.cc | 33 +++++-------- .../codecs/tools/video_quality_measurement.cc | 3 +- .../codecs/vp8/simulcast_encoder_adapter.cc | 16 +++--- .../vp8/simulcast_encoder_adapter_unittest.cc | 43 +++++++--------- .../codecs/vp8/simulcast_unittest.h | 18 +++---- .../codecs/vp8/test/vp8_impl_unittest.cc | 6 +-- .../video_coding/codecs/vp8/vp8_impl.cc | 42 +++++++--------- .../codecs/vp8/vp8_sequence_coder.cc | 4 +- .../video_coding/codecs/vp9/vp9_impl.cc | 36 +++++++------- webrtc/modules/video_coding/video_sender.cc | 4 +- .../video_coding/video_sender_unittest.cc | 4 +- webrtc/video/video_receive_stream.cc | 6 +-- webrtc/video/video_send_stream_tests.cc | 16 +++--- webrtc/video_decoder.h | 2 +- webrtc/video_encoder.h | 2 +- 18 files changed, 185 insertions(+), 145 deletions(-) diff --git a/webrtc/common_types.cc b/webrtc/common_types.cc index 8f117e1153..e78a4aacca 100644 --- a/webrtc/common_types.cc +++ b/webrtc/common_types.cc @@ -8,6 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include "webrtc/base/checks.h" #include "webrtc/common_types.h" #include @@ -42,4 +43,52 @@ RTPHeader::RTPHeader() payload_type_frequency(0), extension() {} +VideoCodec::VideoCodec() + : codecType(kVideoCodecUnknown), + plName(), + plType(0), + width(0), + height(0), + startBitrate(0), + maxBitrate(0), + minBitrate(0), + targetBitrate(0), + maxFramerate(0), + qpMax(0), + numberOfSimulcastStreams(0), + simulcastStream(), + spatialLayers(), + mode(kRealtimeVideo), + codecSpecific() {} + +VideoCodecVP8* VideoCodec::VP8() { + RTC_DCHECK_EQ(codecType, kVideoCodecVP8); + return &codecSpecific.VP8; +} + +const VideoCodecVP8& VideoCodec::VP8() const { + RTC_DCHECK_EQ(codecType, kVideoCodecVP8); + return codecSpecific.VP8; +} + +VideoCodecVP9* VideoCodec::VP9() { + RTC_DCHECK_EQ(codecType, kVideoCodecVP9); + return &codecSpecific.VP9; +} + +const VideoCodecVP9& VideoCodec::VP9() const { + RTC_DCHECK_EQ(codecType, kVideoCodecVP9); + return codecSpecific.VP9; +} + +VideoCodecH264* VideoCodec::H264() { + RTC_DCHECK_EQ(codecType, kVideoCodecH264); + return &codecSpecific.H264; +} + +const VideoCodecH264& VideoCodec::H264() const { + RTC_DCHECK_EQ(codecType, kVideoCodecH264); + return codecSpecific.H264; +} + } // namespace webrtc diff --git a/webrtc/common_types.h b/webrtc/common_types.h index b1842ef595..c3e9a84c3c 100644 --- a/webrtc/common_types.h +++ b/webrtc/common_types.h @@ -593,7 +593,11 @@ struct SpatialLayer { enum VideoCodecMode { kRealtimeVideo, kScreensharing }; // Common video codec properties -struct VideoCodec { +class VideoCodec { + public: + VideoCodec(); + + // Public variables. TODO(hta): Make them private with accessors. VideoCodecType codecType; char plName[kPayloadNameSize]; unsigned char plType; @@ -608,8 +612,6 @@ struct VideoCodec { unsigned char maxFramerate; - VideoCodecUnion codecSpecific; - unsigned int qpMax; unsigned char numberOfSimulcastStreams; SimulcastStream simulcastStream[kMaxSimulcastStreams]; @@ -620,6 +622,23 @@ struct VideoCodec { bool operator==(const VideoCodec& other) const = delete; bool operator!=(const VideoCodec& other) const = delete; + + // Accessors for codec specific information. + // There is a const version of each that returns a reference, + // and a non-const version that returns a pointer, in order + // to allow modification of the parameters. + VideoCodecVP8* VP8(); + const VideoCodecVP8& VP8() const; + VideoCodecVP9* VP9(); + const VideoCodecVP9& VP9() const; + VideoCodecH264* H264(); + const VideoCodecH264& H264() const; + + // This variable will be declared private and renamed to codec_specific_ + // once Chromium is not accessing it. + // TODO(hta): Consider replacing the union with a pointer type. + // This will allow removing the VideoCodec* types from this file. + VideoCodecUnion codecSpecific; }; // Bandwidth over-use detector options. These are used to drive diff --git a/webrtc/modules/video_coding/codec_database.cc b/webrtc/modules/video_coding/codec_database.cc index 72a1ec322f..48ca1fbe25 100644 --- a/webrtc/modules/video_coding/codec_database.cc +++ b/webrtc/modules/video_coding/codec_database.cc @@ -129,7 +129,7 @@ void VCMCodecDataBase::Codec(VideoCodecType codec_type, VideoCodec* settings) { settings->height = VCM_DEFAULT_CODEC_HEIGHT; settings->numberOfSimulcastStreams = 0; settings->qpMax = 56; - settings->codecSpecific.VP8 = VideoEncoder::GetDefaultVp8Settings(); + *(settings->VP8()) = VideoEncoder::GetDefaultVp8Settings(); return; case kVideoCodecVP9: strncpy(settings->plName, "VP9", 4); @@ -144,7 +144,7 @@ void VCMCodecDataBase::Codec(VideoCodecType codec_type, VideoCodec* settings) { settings->height = VCM_DEFAULT_CODEC_HEIGHT; settings->numberOfSimulcastStreams = 0; settings->qpMax = 56; - settings->codecSpecific.VP9 = VideoEncoder::GetDefaultVp9Settings(); + *(settings->VP9()) = VideoEncoder::GetDefaultVp9Settings(); return; case kVideoCodecH264: strncpy(settings->plName, "H264", 5); @@ -159,7 +159,7 @@ void VCMCodecDataBase::Codec(VideoCodecType codec_type, VideoCodec* settings) { settings->height = VCM_DEFAULT_CODEC_HEIGHT; settings->numberOfSimulcastStreams = 0; settings->qpMax = 56; - settings->codecSpecific.H264 = VideoEncoder::GetDefaultH264Settings(); + *(settings->H264()) = VideoEncoder::GetDefaultH264Settings(); return; case kVideoCodecI420: strncpy(settings->plName, "I420", 5); @@ -328,23 +328,20 @@ bool VCMCodecDataBase::RequiresEncoderReset(const VideoCodec& new_send_codec) { switch (new_send_codec.codecType) { case kVideoCodecVP8: - if (memcmp(&new_send_codec.codecSpecific.VP8, - &send_codec_.codecSpecific.VP8, - sizeof(new_send_codec.codecSpecific.VP8)) != 0) { + if (memcmp(&new_send_codec.VP8(), send_codec_.VP8(), + sizeof(new_send_codec.VP8())) != 0) { return true; } break; case kVideoCodecVP9: - if (memcmp(&new_send_codec.codecSpecific.VP9, - &send_codec_.codecSpecific.VP9, - sizeof(new_send_codec.codecSpecific.VP9)) != 0) { + if (memcmp(&new_send_codec.VP9(), send_codec_.VP9(), + sizeof(new_send_codec.VP9())) != 0) { return true; } break; case kVideoCodecH264: - if (memcmp(&new_send_codec.codecSpecific.H264, - &send_codec_.codecSpecific.H264, - sizeof(new_send_codec.codecSpecific.H264)) != 0) { + if (memcmp(&new_send_codec.H264(), send_codec_.H264(), + sizeof(new_send_codec.H264())) != 0) { return true; } break; diff --git a/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc b/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc index 5ca9e55ab3..9c99a0a973 100644 --- a/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc +++ b/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc @@ -191,34 +191,27 @@ class VideoProcessorIntegrationTest : public testing::Test { // These features may be set depending on the test. switch (config_.codec_settings->codecType) { case kVideoCodecH264: - config_.codec_settings->codecSpecific.H264.frameDroppingOn = - frame_dropper_on_; - config_.codec_settings->codecSpecific.H264.keyFrameInterval = + config_.codec_settings->H264()->frameDroppingOn = frame_dropper_on_; + config_.codec_settings->H264()->keyFrameInterval = kBaseKeyFrameInterval; break; case kVideoCodecVP8: - config_.codec_settings->codecSpecific.VP8.errorConcealmentOn = + config_.codec_settings->VP8()->errorConcealmentOn = error_concealment_on_; - config_.codec_settings->codecSpecific.VP8.denoisingOn = denoising_on_; - config_.codec_settings->codecSpecific.VP8.numberOfTemporalLayers = + config_.codec_settings->VP8()->denoisingOn = denoising_on_; + config_.codec_settings->VP8()->numberOfTemporalLayers = num_temporal_layers_; - config_.codec_settings->codecSpecific.VP8.frameDroppingOn = - frame_dropper_on_; - config_.codec_settings->codecSpecific.VP8.automaticResizeOn = - spatial_resize_on_; - config_.codec_settings->codecSpecific.VP8.keyFrameInterval = - kBaseKeyFrameInterval; + config_.codec_settings->VP8()->frameDroppingOn = frame_dropper_on_; + config_.codec_settings->VP8()->automaticResizeOn = spatial_resize_on_; + config_.codec_settings->VP8()->keyFrameInterval = kBaseKeyFrameInterval; break; case kVideoCodecVP9: - config_.codec_settings->codecSpecific.VP9.denoisingOn = denoising_on_; - config_.codec_settings->codecSpecific.VP9.numberOfTemporalLayers = + config_.codec_settings->VP9()->denoisingOn = denoising_on_; + config_.codec_settings->VP9()->numberOfTemporalLayers = num_temporal_layers_; - config_.codec_settings->codecSpecific.VP9.frameDroppingOn = - frame_dropper_on_; - config_.codec_settings->codecSpecific.VP9.automaticResizeOn = - spatial_resize_on_; - config_.codec_settings->codecSpecific.VP9.keyFrameInterval = - kBaseKeyFrameInterval; + config_.codec_settings->VP9()->frameDroppingOn = frame_dropper_on_; + config_.codec_settings->VP9()->automaticResizeOn = spatial_resize_on_; + config_.codec_settings->VP9()->keyFrameInterval = kBaseKeyFrameInterval; break; default: assert(false); diff --git a/webrtc/modules/video_coding/codecs/tools/video_quality_measurement.cc b/webrtc/modules/video_coding/codecs/tools/video_quality_measurement.cc index 37fad483f7..18c48dbfcd 100644 --- a/webrtc/modules/video_coding/codecs/tools/video_quality_measurement.cc +++ b/webrtc/modules/video_coding/codecs/tools/video_quality_measurement.cc @@ -215,8 +215,7 @@ int HandleCommandLineFlags(webrtc::test::TestConfig* config) { FLAGS_temporal_layers); return 13; } - config->codec_settings->codecSpecific.VP8.numberOfTemporalLayers = - FLAGS_temporal_layers; + config->codec_settings->VP8()->numberOfTemporalLayers = FLAGS_temporal_layers; // Check the bit rate. if (FLAGS_bitrate <= 0) { diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc index 1c8037a60e..880f45fa7e 100644 --- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc +++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc @@ -74,12 +74,10 @@ int VerifyCodec(const webrtc::VideoCodec* inst) { if (inst->width <= 1 || inst->height <= 1) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; } - if (inst->codecSpecific.VP8.feedbackModeOn && - inst->numberOfSimulcastStreams > 1) { + if (inst->VP8().feedbackModeOn && inst->numberOfSimulcastStreams > 1) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; } - if (inst->codecSpecific.VP8.automaticResizeOn && - inst->numberOfSimulcastStreams > 1) { + if (inst->VP8().automaticResizeOn && inst->numberOfSimulcastStreams > 1) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; } return WEBRTC_VIDEO_CODEC_OK; @@ -182,7 +180,7 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, // Special mode when screensharing on a single stream. if (number_of_streams == 1 && inst->mode == kScreensharing) { screensharing_tl_factory_.reset(new ScreenshareTemporalLayersFactory()); - codec_.codecSpecific.VP8.tl_factory = screensharing_tl_factory_.get(); + codec_.VP8()->tl_factory = screensharing_tl_factory_.get(); } std::string implementation_name; @@ -386,7 +384,7 @@ int SimulcastEncoderAdapter::SetRates(uint32_t new_bitrate_kbit, // the target we still allow it to overshoot up to the max before dropping // frames. This hack should be improved. if (codec_.targetBitrate > 0 && - (codec_.codecSpecific.VP8.numberOfTemporalLayers == 2 || + (codec_.VP8()->numberOfTemporalLayers == 2 || codec_.simulcastStream[0].numberOfTemporalLayers == 2)) { stream_bitrate_kbps = std::min(codec_.maxBitrate, stream_bitrate_kbps); // TODO(ronghuawu): Can't change max bitrate via the VideoEncoder @@ -425,7 +423,7 @@ void SimulcastEncoderAdapter::PopulateStreamCodec( *stream_codec = *inst; // Stream specific settings. - stream_codec->codecSpecific.VP8.numberOfTemporalLayers = + stream_codec->VP8()->numberOfTemporalLayers = inst->simulcastStream[stream_index].numberOfTemporalLayers; stream_codec->numberOfSimulcastStreams = 0; stream_codec->width = inst->simulcastStream[stream_index].width; @@ -443,10 +441,10 @@ void SimulcastEncoderAdapter::PopulateStreamCodec( // kComplexityHigher, which maps to cpu_used = -4. int pixels_per_frame = stream_codec->width * stream_codec->height; if (pixels_per_frame < 352 * 288) { - stream_codec->codecSpecific.VP8.complexity = webrtc::kComplexityHigher; + stream_codec->VP8()->complexity = webrtc::kComplexityHigher; } // Turn off denoising for all streams but the highest resolution. - stream_codec->codecSpecific.VP8.denoisingOn = false; + stream_codec->VP8()->denoisingOn = false; } // TODO(ronghuawu): what to do with targetBitrate. diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc index 0ccbb54855..8bc3b489ff 100644 --- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc +++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc @@ -289,28 +289,19 @@ class TestSimulcastEncoderAdapterFake : public ::testing::Test, EXPECT_EQ(ref.maxBitrate, target.maxBitrate); EXPECT_EQ(ref.minBitrate, target.minBitrate); EXPECT_EQ(ref.maxFramerate, target.maxFramerate); - EXPECT_EQ(ref.codecSpecific.VP8.pictureLossIndicationOn, - target.codecSpecific.VP8.pictureLossIndicationOn); - EXPECT_EQ(ref.codecSpecific.VP8.feedbackModeOn, - target.codecSpecific.VP8.feedbackModeOn); - EXPECT_EQ(ref.codecSpecific.VP8.complexity, - target.codecSpecific.VP8.complexity); - EXPECT_EQ(ref.codecSpecific.VP8.resilience, - target.codecSpecific.VP8.resilience); - EXPECT_EQ(ref.codecSpecific.VP8.numberOfTemporalLayers, - target.codecSpecific.VP8.numberOfTemporalLayers); - EXPECT_EQ(ref.codecSpecific.VP8.denoisingOn, - target.codecSpecific.VP8.denoisingOn); - EXPECT_EQ(ref.codecSpecific.VP8.errorConcealmentOn, - target.codecSpecific.VP8.errorConcealmentOn); - EXPECT_EQ(ref.codecSpecific.VP8.automaticResizeOn, - target.codecSpecific.VP8.automaticResizeOn); - EXPECT_EQ(ref.codecSpecific.VP8.frameDroppingOn, - target.codecSpecific.VP8.frameDroppingOn); - EXPECT_EQ(ref.codecSpecific.VP8.keyFrameInterval, - target.codecSpecific.VP8.keyFrameInterval); - EXPECT_EQ(ref.codecSpecific.VP8.tl_factory, - target.codecSpecific.VP8.tl_factory); + EXPECT_EQ(ref.VP8().pictureLossIndicationOn, + target.VP8().pictureLossIndicationOn); + EXPECT_EQ(ref.VP8().feedbackModeOn, target.VP8().feedbackModeOn); + EXPECT_EQ(ref.VP8().complexity, target.VP8().complexity); + EXPECT_EQ(ref.VP8().resilience, target.VP8().resilience); + EXPECT_EQ(ref.VP8().numberOfTemporalLayers, + target.VP8().numberOfTemporalLayers); + EXPECT_EQ(ref.VP8().denoisingOn, target.VP8().denoisingOn); + EXPECT_EQ(ref.VP8().errorConcealmentOn, target.VP8().errorConcealmentOn); + EXPECT_EQ(ref.VP8().automaticResizeOn, target.VP8().automaticResizeOn); + EXPECT_EQ(ref.VP8().frameDroppingOn, target.VP8().frameDroppingOn); + EXPECT_EQ(ref.VP8().keyFrameInterval, target.VP8().keyFrameInterval); + EXPECT_EQ(ref.VP8().tl_factory, target.VP8().tl_factory); EXPECT_EQ(ref.qpMax, target.qpMax); EXPECT_EQ(0, target.numberOfSimulcastStreams); EXPECT_EQ(ref.mode, target.mode); @@ -321,7 +312,7 @@ class TestSimulcastEncoderAdapterFake : public ::testing::Test, void InitRefCodec(int stream_index, VideoCodec* ref_codec) { *ref_codec = codec_; - ref_codec->codecSpecific.VP8.numberOfTemporalLayers = + ref_codec->VP8()->numberOfTemporalLayers = kTestTemporalLayerProfile[stream_index]; ref_codec->width = codec_.simulcastStream[stream_index].width; ref_codec->height = codec_.simulcastStream[stream_index].height; @@ -337,14 +328,14 @@ class TestSimulcastEncoderAdapterFake : public ::testing::Test, // stream 0, the lowest resolution stream. InitRefCodec(0, &ref_codec); ref_codec.qpMax = 45; - ref_codec.codecSpecific.VP8.complexity = webrtc::kComplexityHigher; - ref_codec.codecSpecific.VP8.denoisingOn = false; + ref_codec.VP8()->complexity = webrtc::kComplexityHigher; + ref_codec.VP8()->denoisingOn = false; ref_codec.startBitrate = 100; // Should equal to the target bitrate. VerifyCodec(ref_codec, 0); // stream 1 InitRefCodec(1, &ref_codec); - ref_codec.codecSpecific.VP8.denoisingOn = false; + ref_codec.VP8()->denoisingOn = false; // The start bitrate (300kbit) minus what we have for the lower layers // (100kbit). ref_codec.startBitrate = 200; diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h b/webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h index e2d5bb8738..56ad3a710d 100644 --- a/webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h +++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h @@ -153,7 +153,7 @@ class SkipEncodingUnusedStreamsTest { VideoCodec* settings, uint32_t target_bitrate) { SpyingTemporalLayersFactory spy_factory; - settings->codecSpecific.VP8.tl_factory = &spy_factory; + settings->VP8()->tl_factory = &spy_factory; EXPECT_EQ(0, encoder->InitEncode(settings, 1, 1200)); encoder->SetRates(target_bitrate, 30); @@ -288,13 +288,13 @@ class TestVp8Simulcast : public ::testing::Test { ConfigureStream(kDefaultWidth, kDefaultHeight, kMaxBitrates[2], kMinBitrates[2], kTargetBitrates[2], &settings->simulcastStream[2], temporal_layer_profile[2]); - settings->codecSpecific.VP8.resilience = kResilientStream; - settings->codecSpecific.VP8.denoisingOn = true; - settings->codecSpecific.VP8.errorConcealmentOn = false; - settings->codecSpecific.VP8.automaticResizeOn = false; - settings->codecSpecific.VP8.feedbackModeOn = false; - settings->codecSpecific.VP8.frameDroppingOn = true; - settings->codecSpecific.VP8.keyFrameInterval = 3000; + settings->VP8()->resilience = kResilientStream; + settings->VP8()->denoisingOn = true; + settings->VP8()->errorConcealmentOn = false; + settings->VP8()->automaticResizeOn = false; + settings->VP8()->feedbackModeOn = false; + settings->VP8()->frameDroppingOn = true; + settings->VP8()->keyFrameInterval = 3000; } static void ConfigureStream(int width, @@ -563,7 +563,7 @@ class TestVp8Simulcast : public ::testing::Test { void SwitchingToOneStream(int width, int height) { // Disable all streams except the last and set the bitrate of the last to // 100 kbps. This verifies the way GTP switches to screenshare mode. - settings_.codecSpecific.VP8.numberOfTemporalLayers = 1; + settings_.VP8()->numberOfTemporalLayers = 1; settings_.maxBitrate = 100; settings_.startBitrate = 100; settings_.width = width; diff --git a/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc b/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc index 63be85afdc..487a9da846 100644 --- a/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc +++ b/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc @@ -160,7 +160,7 @@ class TestVp8Impl : public ::testing::Test { codec_inst_.startBitrate = 300; codec_inst_.maxBitrate = 4000; codec_inst_.qpMax = 56; - codec_inst_.codecSpecific.VP8.denoisingOn = true; + codec_inst_.VP8()->denoisingOn = true; EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, encoder_->InitEncode(&codec_inst_, 1, 1440)); @@ -213,8 +213,8 @@ TEST_F(TestVp8Impl, EncoderParameterTest) { codec_inst_.maxFramerate = 30; codec_inst_.startBitrate = 300; codec_inst_.qpMax = 56; - codec_inst_.codecSpecific.VP8.complexity = kComplexityNormal; - codec_inst_.codecSpecific.VP8.numberOfTemporalLayers = 1; + codec_inst_.VP8()->complexity = kComplexityNormal; + codec_inst_.VP8()->numberOfTemporalLayers = 1; // Calls before InitEncode(). EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, encoder_->Release()); int bit_rate = 300; diff --git a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc index 439f6afa3e..715f8ff575 100644 --- a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc +++ b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc @@ -247,7 +247,7 @@ int VP8EncoderImpl::SetRates(uint32_t new_bitrate_kbit, // the target we still allow it to overshoot up to the max before dropping // frames. This hack should be improved. if (codec_.targetBitrate > 0 && - (codec_.codecSpecific.VP8.numberOfTemporalLayers == 2 || + (codec_.VP8()->numberOfTemporalLayers == 2 || codec_.simulcastStream[0].numberOfTemporalLayers == 2)) { int tl0_bitrate = std::min(codec_.targetBitrate, target_bitrate); max_bitrate = std::min(codec_.maxBitrate, target_bitrate); @@ -286,7 +286,7 @@ void VP8EncoderImpl::SetupTemporalLayers(int num_streams, int num_temporal_layers, const VideoCodec& codec) { TemporalLayersFactory default_factory; - const TemporalLayersFactory* tl_factory = codec.codecSpecific.VP8.tl_factory; + const TemporalLayersFactory* tl_factory = codec.VP8().tl_factory; if (!tl_factory) tl_factory = &default_factory; if (num_streams == 1) { @@ -328,12 +328,10 @@ int VP8EncoderImpl::InitEncode(const VideoCodec* inst, if (number_of_cores < 1) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; } - if (inst->codecSpecific.VP8.feedbackModeOn && - inst->numberOfSimulcastStreams > 1) { + if (inst->VP8().feedbackModeOn && inst->numberOfSimulcastStreams > 1) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; } - if (inst->codecSpecific.VP8.automaticResizeOn && - inst->numberOfSimulcastStreams > 1) { + if (inst->VP8().automaticResizeOn && inst->numberOfSimulcastStreams > 1) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; } int retVal = Release(); @@ -350,14 +348,14 @@ int VP8EncoderImpl::InitEncode(const VideoCodec* inst, int num_temporal_layers = doing_simulcast ? inst->simulcastStream[0].numberOfTemporalLayers - : inst->codecSpecific.VP8.numberOfTemporalLayers; + : inst->VP8().numberOfTemporalLayers; // TODO(andresp): crash if num temporal layers is bananas. if (num_temporal_layers < 1) num_temporal_layers = 1; SetupTemporalLayers(number_of_streams, num_temporal_layers, *inst); - feedback_mode_ = inst->codecSpecific.VP8.feedbackModeOn; + feedback_mode_ = inst->VP8().feedbackModeOn; timestamp_ = 0; codec_ = *inst; @@ -419,7 +417,7 @@ int VP8EncoderImpl::InitEncode(const VideoCodec* inst, configurations_[0].g_lag_in_frames = 0; // 0- no frame lagging // Set the error resilience mode according to user settings. - switch (inst->codecSpecific.VP8.resilience) { + switch (inst->VP8().resilience) { case kResilienceOff: // TODO(marpan): We should set keep error resilience off for this mode, // independent of temporal layer settings, and make sure we set @@ -437,8 +435,7 @@ int VP8EncoderImpl::InitEncode(const VideoCodec* inst, } // rate control settings - configurations_[0].rc_dropframe_thresh = - inst->codecSpecific.VP8.frameDroppingOn ? 30 : 0; + configurations_[0].rc_dropframe_thresh = inst->VP8().frameDroppingOn ? 30 : 0; configurations_[0].rc_end_usage = VPX_CBR; configurations_[0].g_pass = VPX_RC_ONE_PASS; // TODO(hellner): investigate why the following two lines produce @@ -449,7 +446,7 @@ int VP8EncoderImpl::InitEncode(const VideoCodec* inst, // inst->codecSpecific.VP8.automaticResizeOn ? 1 : 0; configurations_[0].rc_resize_allowed = 0; // Handle resizing outside of libvpx when doing single-stream. - if (inst->codecSpecific.VP8.automaticResizeOn && number_of_streams > 1) { + if (inst->VP8().automaticResizeOn && number_of_streams > 1) { configurations_[0].rc_resize_allowed = 1; } configurations_[0].rc_min_quantizer = 2; @@ -470,15 +467,15 @@ int VP8EncoderImpl::InitEncode(const VideoCodec* inst, // Disable periodic key frames if we get feedback from the decoder // through SLI and RPSI. configurations_[0].kf_mode = VPX_KF_DISABLED; - } else if (inst->codecSpecific.VP8.keyFrameInterval > 0) { + } else if (inst->VP8().keyFrameInterval > 0) { configurations_[0].kf_mode = VPX_KF_AUTO; - configurations_[0].kf_max_dist = inst->codecSpecific.VP8.keyFrameInterval; + configurations_[0].kf_max_dist = inst->VP8().keyFrameInterval; } else { configurations_[0].kf_mode = VPX_KF_DISABLED; } // Allow the user to set the complexity for the base stream. - switch (inst->codecSpecific.VP8.complexity) { + switch (inst->VP8().complexity) { case kComplexityHigh: cpu_speed_[0] = -5; break; @@ -563,7 +560,7 @@ int VP8EncoderImpl::InitEncode(const VideoCodec* inst, // use frame drops as a signal and is only applicable when we drop frames. quality_scaler_enabled_ = encoders_.size() == 1 && configurations_[0].rc_dropframe_thresh > 0 && - codec_.codecSpecific.VP8.automaticResizeOn; + codec_.VP8()->automaticResizeOn; return InitAndSetControlSettings(); } @@ -576,7 +573,7 @@ int VP8EncoderImpl::SetCpuSpeed(int width, int height) { #else // For non-ARM, increase encoding complexity (i.e., use lower speed setting) // if resolution is below CIF. Otherwise, keep the default/user setting - // (|cpu_speed_default_|) set on InitEncode via codecSpecific.VP8.complexity. + // (|cpu_speed_default_|) set on InitEncode via VP8().complexity. if (width * height < 352 * 288) return (cpu_speed_default_ < -4) ? -4 : cpu_speed_default_; else @@ -644,13 +641,12 @@ int VP8EncoderImpl::InitAndSetControlSettings() { #else denoiser_state = kDenoiserOnAdaptive; #endif - vpx_codec_control( - &encoders_[0], VP8E_SET_NOISE_SENSITIVITY, - codec_.codecSpecific.VP8.denoisingOn ? denoiser_state : kDenoiserOff); + vpx_codec_control(&encoders_[0], VP8E_SET_NOISE_SENSITIVITY, + codec_.VP8()->denoisingOn ? denoiser_state : kDenoiserOff); if (encoders_.size() > 2) { vpx_codec_control( &encoders_[1], VP8E_SET_NOISE_SENSITIVITY, - codec_.codecSpecific.VP8.denoisingOn ? denoiser_state : kDenoiserOff); + codec_.VP8()->denoisingOn ? denoiser_state : kDenoiserOff); } for (size_t i = 0; i < encoders_.size(); ++i) { // Allow more screen content to be detected as static. @@ -780,7 +776,7 @@ int VP8EncoderImpl::Encode(const VideoFrame& frame, // Adapt the size of the key frame when in screenshare with 1 temporal // layer. if (encoders_.size() == 1 && codec_.mode == kScreensharing && - codec_.codecSpecific.VP8.numberOfTemporalLayers <= 1) { + codec_.VP8()->numberOfTemporalLayers <= 1) { const uint32_t forceKeyFrameIntraTh = 100; vpx_codec_control(&(encoders_[0]), VP8E_SET_MAX_INTRA_BITRATE_PCT, forceKeyFrameIntraTh); @@ -1073,7 +1069,7 @@ int VP8DecoderImpl::InitDecode(const VideoCodec* inst, int number_of_cores) { decoder_ = new vpx_codec_ctx_t; } if (inst && inst->codecType == kVideoCodecVP8) { - feedback_mode_ = inst->codecSpecific.VP8.feedbackModeOn; + feedback_mode_ = inst->VP8().feedbackModeOn; } vpx_codec_dec_cfg_t cfg; // Setting number of threads to a constant value (1) diff --git a/webrtc/modules/video_coding/codecs/vp8/vp8_sequence_coder.cc b/webrtc/modules/video_coding/codecs/vp8/vp8_sequence_coder.cc index 006ff65eee..857976d5d2 100644 --- a/webrtc/modules/video_coding/codecs/vp8/vp8_sequence_coder.cc +++ b/webrtc/modules/video_coding/codecs/vp8/vp8_sequence_coder.cc @@ -135,8 +135,8 @@ int SequenceCoder(webrtc::test::CommandLineParser* parser) { webrtc::VP8Encoder* encoder = webrtc::VP8Encoder::Create(); webrtc::VP8Decoder* decoder = webrtc::VP8Decoder::Create(); inst.codecType = webrtc::kVideoCodecVP8; - inst.codecSpecific.VP8.feedbackModeOn = false; - inst.codecSpecific.VP8.denoisingOn = true; + inst.VP8()->feedbackModeOn = false; + inst.VP8()->denoisingOn = true; inst.maxFramerate = framerate; inst.startBitrate = target_bitrate; inst.maxBitrate = 8000; diff --git a/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc b/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc index aa54bdf0a5..3a4efb3c3a 100644 --- a/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -242,11 +242,11 @@ int VP9EncoderImpl::InitEncode(const VideoCodec* inst, if (number_of_cores < 1) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; } - if (inst->codecSpecific.VP9.numberOfTemporalLayers > 3) { + if (inst->VP9().numberOfTemporalLayers > 3) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; } // libvpx currently supports only one or two spatial layers. - if (inst->codecSpecific.VP9.numberOfSpatialLayers > 2) { + if (inst->VP9().numberOfSpatialLayers > 2) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; } @@ -265,8 +265,8 @@ int VP9EncoderImpl::InitEncode(const VideoCodec* inst, codec_ = *inst; } - num_spatial_layers_ = inst->codecSpecific.VP9.numberOfSpatialLayers; - num_temporal_layers_ = inst->codecSpecific.VP9.numberOfTemporalLayers; + num_spatial_layers_ = inst->VP9().numberOfSpatialLayers; + num_temporal_layers_ = inst->VP9().numberOfTemporalLayers; if (num_temporal_layers_ == 0) num_temporal_layers_ = 1; @@ -298,8 +298,7 @@ int VP9EncoderImpl::InitEncode(const VideoCodec* inst, config_->g_lag_in_frames = 0; // 0- no frame lagging config_->g_threads = 1; // Rate control settings. - config_->rc_dropframe_thresh = - inst->codecSpecific.VP9.frameDroppingOn ? 30 : 0; + config_->rc_dropframe_thresh = inst->VP9().frameDroppingOn ? 30 : 0; config_->rc_end_usage = VPX_CBR; config_->g_pass = VPX_RC_ONE_PASS; config_->rc_min_quantizer = 2; @@ -311,17 +310,16 @@ int VP9EncoderImpl::InitEncode(const VideoCodec* inst, config_->rc_buf_sz = 1000; // Set the maximum target size of any key-frame. rc_max_intra_target_ = MaxIntraTarget(config_->rc_buf_optimal_sz); - if (inst->codecSpecific.VP9.keyFrameInterval > 0) { + if (inst->VP9().keyFrameInterval > 0) { config_->kf_mode = VPX_KF_AUTO; - config_->kf_max_dist = inst->codecSpecific.VP9.keyFrameInterval; + config_->kf_max_dist = inst->VP9().keyFrameInterval; // Needs to be set (in svc mode) to get correct periodic key frame interval // (will have no effect in non-svc). config_->kf_min_dist = config_->kf_max_dist; } else { config_->kf_mode = VPX_KF_DISABLED; } - config_->rc_resize_allowed = - inst->codecSpecific.VP9.automaticResizeOn ? 1 : 0; + config_->rc_resize_allowed = inst->VP9().automaticResizeOn ? 1 : 0; // Determine number of threads based on the image size and #cores. config_->g_threads = NumberOfThreads(config_->g_w, config_->g_h, number_of_cores); @@ -330,7 +328,7 @@ int VP9EncoderImpl::InitEncode(const VideoCodec* inst, // TODO(asapersson): Check configuration of temporal switch up and increase // pattern length. - is_flexible_mode_ = inst->codecSpecific.VP9.flexibleMode; + is_flexible_mode_ = inst->VP9().flexibleMode; if (is_flexible_mode_) { config_->temporal_layering_mode = VP9E_TEMPORAL_LAYERING_MODE_BYPASS; config_->ts_number_layers = num_temporal_layers_; @@ -424,7 +422,7 @@ int VP9EncoderImpl::InitAndSetControlSettings(const VideoCodec* inst) { vpx_codec_control(encoder_, VP8E_SET_MAX_INTRA_BITRATE_PCT, rc_max_intra_target_); vpx_codec_control(encoder_, VP9E_SET_AQ_MODE, - inst->codecSpecific.VP9.adaptiveQpMode ? 3 : 0); + inst->VP9().adaptiveQpMode ? 3 : 0); vpx_codec_control( encoder_, VP9E_SET_SVC, @@ -448,9 +446,9 @@ int VP9EncoderImpl::InitAndSetControlSettings(const VideoCodec* inst) { #if !defined(WEBRTC_ARCH_ARM) && !defined(WEBRTC_ARCH_ARM64) && \ !defined(ANDROID) // Note denoiser is still off by default until further testing/optimization, - // i.e., codecSpecific.VP9.denoisingOn == 0. + // i.e., VP9().denoisingOn == 0. vpx_codec_control(encoder_, VP9E_SET_NOISE_SENSITIVITY, - inst->codecSpecific.VP9.denoisingOn ? 1 : 0); + inst->VP9().denoisingOn ? 1 : 0); #endif if (codec_.mode == kScreensharing) { // Adjust internal parameters to screen content. @@ -565,11 +563,11 @@ void VP9EncoderImpl::PopulateCodecSpecific(CodecSpecificInfo* codec_specific, // TODO(asapersson): Set correct value. vp9_info->inter_pic_predicted = (pkt.data.frame.flags & VPX_FRAME_IS_KEY) ? false : true; - vp9_info->flexible_mode = codec_.codecSpecific.VP9.flexibleMode; - vp9_info->ss_data_available = ((pkt.data.frame.flags & VPX_FRAME_IS_KEY) && - !codec_.codecSpecific.VP9.flexibleMode) - ? true - : false; + vp9_info->flexible_mode = codec_.VP9()->flexibleMode; + vp9_info->ss_data_available = + ((pkt.data.frame.flags & VPX_FRAME_IS_KEY) && !codec_.VP9()->flexibleMode) + ? true + : false; vpx_svc_layer_id_t layer_id = {0}; vpx_codec_control(encoder_, VP9E_GET_SVC_LAYER_ID, &layer_id); diff --git a/webrtc/modules/video_coding/video_sender.cc b/webrtc/modules/video_coding/video_sender.cc index 0f2da583a0..b7664a719b 100644 --- a/webrtc/modules/video_coding/video_sender.cc +++ b/webrtc/modules/video_coding/video_sender.cc @@ -103,9 +103,9 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec, int numLayers; if (sendCodec->codecType == kVideoCodecVP8) { - numLayers = sendCodec->codecSpecific.VP8.numberOfTemporalLayers; + numLayers = sendCodec->VP8().numberOfTemporalLayers; } else if (sendCodec->codecType == kVideoCodecVP9) { - numLayers = sendCodec->codecSpecific.VP9.numberOfTemporalLayers; + numLayers = sendCodec->VP9().numberOfTemporalLayers; } else { numLayers = 1; } diff --git a/webrtc/modules/video_coding/video_sender_unittest.cc b/webrtc/modules/video_coding/video_sender_unittest.cc index f66221e558..71602f0ea1 100644 --- a/webrtc/modules/video_coding/video_sender_unittest.cc +++ b/webrtc/modules/video_coding/video_sender_unittest.cc @@ -412,7 +412,7 @@ class TestVideoSenderWithVp8 : public TestVideoSender { VideoCodingModule::Codec(kVideoCodecVP8, &codec); codec.width = width; codec.height = height; - codec.codecSpecific.VP8.numberOfTemporalLayers = temporal_layers; + codec.VP8()->numberOfTemporalLayers = temporal_layers; return codec; } @@ -477,7 +477,7 @@ TEST_F(TestVideoSenderWithVp8, MAYBE_FixedTemporalLayersStrategy) { TEST_F(TestVideoSenderWithVp8, MAYBE_RealTimeTemporalLayersStrategy) { VideoCodec codec = MakeVp8VideoCodec(352, 288, 3); RealTimeTemporalLayersFactory realtime_tl_factory; - codec.codecSpecific.VP8.tl_factory = &realtime_tl_factory; + codec.VP8()->tl_factory = &realtime_tl_factory; codec.minBitrate = 10; codec.startBitrate = codec_bitrate_kbps_; codec.maxBitrate = codec_bitrate_kbps_; diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index e25546a193..7d191c8179 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -160,11 +160,11 @@ VideoCodec CreateDecoderVideoCodec(const VideoReceiveStream::Decoder& decoder) { } if (codec.codecType == kVideoCodecVP8) { - codec.codecSpecific.VP8 = VideoEncoder::GetDefaultVp8Settings(); + *(codec.VP8()) = VideoEncoder::GetDefaultVp8Settings(); } else if (codec.codecType == kVideoCodecVP9) { - codec.codecSpecific.VP9 = VideoEncoder::GetDefaultVp9Settings(); + *(codec.VP9()) = VideoEncoder::GetDefaultVp9Settings(); } else if (codec.codecType == kVideoCodecH264) { - codec.codecSpecific.H264 = VideoEncoder::GetDefaultH264Settings(); + *(codec.H264()) = VideoEncoder::GetDefaultH264Settings(); } codec.width = 320; diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index cc73fa809b..9d00e007de 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -1992,8 +1992,8 @@ class VideoCodecConfigObserver : public test::SendTest, template <> void VideoCodecConfigObserver::VerifyCodecSpecifics( const VideoCodec& config) const { - EXPECT_EQ(0, memcmp(&config.codecSpecific.H264, &encoder_settings_, - sizeof(encoder_settings_))); + EXPECT_EQ( + 0, memcmp(&config.H264(), &encoder_settings_, sizeof(encoder_settings_))); } template <> @@ -2009,7 +2009,7 @@ void VideoCodecConfigObserver::VerifyCodecSpecifics( // Check that the number of temporal layers has propagated properly to // VideoCodec. EXPECT_EQ(kVideoCodecConfigObserverNumberOfTemporalLayers, - config.codecSpecific.VP8.numberOfTemporalLayers); + config.VP8().numberOfTemporalLayers); for (unsigned char i = 0; i < config.numberOfSimulcastStreams; ++i) { EXPECT_EQ(kVideoCodecConfigObserverNumberOfTemporalLayers, @@ -2021,8 +2021,8 @@ void VideoCodecConfigObserver::VerifyCodecSpecifics( VideoCodecVP8 encoder_settings = encoder_settings_; encoder_settings.numberOfTemporalLayers = kVideoCodecConfigObserverNumberOfTemporalLayers; - EXPECT_EQ(0, memcmp(&config.codecSpecific.VP8, &encoder_settings, - sizeof(encoder_settings_))); + EXPECT_EQ( + 0, memcmp(&config.VP8(), &encoder_settings, sizeof(encoder_settings_))); } template <> @@ -2038,7 +2038,7 @@ void VideoCodecConfigObserver::VerifyCodecSpecifics( // Check that the number of temporal layers has propagated properly to // VideoCodec. EXPECT_EQ(kVideoCodecConfigObserverNumberOfTemporalLayers, - config.codecSpecific.VP9.numberOfTemporalLayers); + config.VP9().numberOfTemporalLayers); for (unsigned char i = 0; i < config.numberOfSimulcastStreams; ++i) { EXPECT_EQ(kVideoCodecConfigObserverNumberOfTemporalLayers, @@ -2050,8 +2050,8 @@ void VideoCodecConfigObserver::VerifyCodecSpecifics( VideoCodecVP9 encoder_settings = encoder_settings_; encoder_settings.numberOfTemporalLayers = kVideoCodecConfigObserverNumberOfTemporalLayers; - EXPECT_EQ(0, memcmp(&config.codecSpecific.VP9, &encoder_settings, - sizeof(encoder_settings_))); + EXPECT_EQ( + 0, memcmp(&(config.VP9()), &encoder_settings, sizeof(encoder_settings_))); } template <> diff --git a/webrtc/video_decoder.h b/webrtc/video_decoder.h index 4f25e1fdce..7a41840ba8 100644 --- a/webrtc/video_decoder.h +++ b/webrtc/video_decoder.h @@ -24,7 +24,7 @@ namespace webrtc { class RTPFragmentationHeader; // TODO(pbos): Expose these through a public (root) header or change these APIs. struct CodecSpecificInfo; -struct VideoCodec; +class VideoCodec; class DecodedImageCallback { public: diff --git a/webrtc/video_encoder.h b/webrtc/video_encoder.h index d28533b9fd..d8b7921bc0 100644 --- a/webrtc/video_encoder.h +++ b/webrtc/video_encoder.h @@ -24,7 +24,7 @@ namespace webrtc { class RTPFragmentationHeader; // TODO(pbos): Expose these through a public (root) header or change these APIs. struct CodecSpecificInfo; -struct VideoCodec; +class VideoCodec; class EncodedImageCallback { public: