From 5dfac5681351242808798e7c74a5c52189fae151 Mon Sep 17 00:00:00 2001 From: magjed Date: Fri, 25 Nov 2016 03:56:37 -0800 Subject: [PATCH] Keep all codec parameters in VideoReceiveStream::Decoder It will be necessary to keep the H264 profile information in VideoReceiveStream::Decoder. I think it will be easier now and for the future to just store all of the codec parameters unmodified in VideoReceiveStream::Decoder instead of extracting a subset of them to an ad hoc class. BUG=webrtc:6743,webrtc:5948 Review-Url: https://codereview.webrtc.org/2523773003 Cr-Commit-Position: refs/heads/master@{#15239} --- webrtc/config.cc | 4 ---- webrtc/config.h | 11 --------- webrtc/media/base/mediaconstants.cc | 1 + webrtc/media/base/mediaconstants.h | 1 + webrtc/media/engine/webrtcvideoengine2.cc | 16 +------------ .../engine/webrtcvideoengine2_unittest.cc | 23 +++++++++---------- webrtc/video/video_receive_stream.cc | 7 +++--- webrtc/video_receive_stream.h | 5 +++- 8 files changed, 21 insertions(+), 47 deletions(-) diff --git a/webrtc/config.cc b/webrtc/config.cc index f8e3602851..cb6dcb24a1 100644 --- a/webrtc/config.cc +++ b/webrtc/config.cc @@ -243,8 +243,4 @@ void VideoEncoderConfig::Vp9EncoderSpecificSettings::FillVideoCodecVp9( *vp9_settings = specifics_; } -DecoderSpecificSettings::DecoderSpecificSettings() = default; - -DecoderSpecificSettings::~DecoderSpecificSettings() = default; - } // namespace webrtc diff --git a/webrtc/config.h b/webrtc/config.h index ee81617d8d..e0883e671d 100644 --- a/webrtc/config.h +++ b/webrtc/config.h @@ -250,17 +250,6 @@ class VideoEncoderConfig { VideoEncoderConfig(const VideoEncoderConfig&); }; -struct VideoDecoderH264Settings { - std::string sprop_parameter_sets; -}; - -class DecoderSpecificSettings { - public: - DecoderSpecificSettings(); - virtual ~DecoderSpecificSettings(); - rtc::Optional h264_extra_settings; -}; - } // namespace webrtc #endif // WEBRTC_CONFIG_H_ diff --git a/webrtc/media/base/mediaconstants.cc b/webrtc/media/base/mediaconstants.cc index bce252195a..0d8512b9f3 100644 --- a/webrtc/media/base/mediaconstants.cc +++ b/webrtc/media/base/mediaconstants.cc @@ -106,6 +106,7 @@ const char kH264CodecName[] = "H264"; const char kH264FmtpProfileLevelId[] = "profile-level-id"; const char kH264FmtpLevelAsymmetryAllowed[] = "level-asymmetry-allowed"; const char kH264FmtpPacketizationMode[] = "packetization-mode"; +const char kH264FmtpSpropParameterSets[] = "sprop-parameter-sets"; const char kH264ProfileLevelConstrainedBaseline[] = "42e01f"; const int kDefaultVideoMaxFramerate = 60; diff --git a/webrtc/media/base/mediaconstants.h b/webrtc/media/base/mediaconstants.h index 70080c3a08..547839afdb 100644 --- a/webrtc/media/base/mediaconstants.h +++ b/webrtc/media/base/mediaconstants.h @@ -128,6 +128,7 @@ extern const char kH264CodecName[]; extern const char kH264FmtpProfileLevelId[]; extern const char kH264FmtpLevelAsymmetryAllowed[]; extern const char kH264FmtpPacketizationMode[]; +extern const char kH264FmtpSpropParameterSets[]; extern const char kH264ProfileLevelConstrainedBaseline[]; extern const int kDefaultVideoMaxFramerate; diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 596e7091f7..55e6374a2b 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -2276,20 +2276,6 @@ WebRtcVideoChannel2::WebRtcVideoReceiveStream::CreateOrReuseVideoDecoder( webrtc::kVideoCodecUnknown, false); } -void ConfigureDecoderSpecifics(webrtc::VideoReceiveStream::Decoder* decoder, - const cricket::VideoCodec& recv_video_codec) { - if (recv_video_codec.name.compare("H264") == 0) { - auto it = recv_video_codec.params.find("sprop-parameter-sets"); - if (it != recv_video_codec.params.end()) { - decoder->decoder_specific.h264_extra_settings = - rtc::Optional( - webrtc::VideoDecoderH264Settings()); - decoder->decoder_specific.h264_extra_settings->sprop_parameter_sets = - it->second; - } - } -} - void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ConfigureCodecs( const std::vector& recv_codecs, std::vector* old_decoders) { @@ -2305,7 +2291,7 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ConfigureCodecs( decoder.decoder = allocated_decoder.decoder; decoder.payload_type = recv_codecs[i].codec.id; decoder.payload_name = recv_codecs[i].codec.name; - ConfigureDecoderSpecifics(&decoder, recv_codecs[i].codec); + decoder.codec_params = recv_codecs[i].codec.params; config_.decoders.push_back(decoder); } diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index ea384e0222..af811216f9 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -19,6 +19,7 @@ #include "webrtc/base/stringutils.h" #include "webrtc/common_video/h264/profile_level_id.h" #include "webrtc/logging/rtc_event_log/rtc_event_log.h" +#include "webrtc/media/base/mediaconstants.h" #include "webrtc/media/base/testutils.h" #include "webrtc/media/base/videoengine_unittest.h" #include "webrtc/media/engine/fakewebrtccall.h" @@ -3749,10 +3750,10 @@ TEST_F(WebRtcVideoChannel2Test, DISABLED_GetRtpReceiveFmtpSprop) { #endif cricket::VideoRecvParameters parameters; cricket::VideoCodec kH264sprop1(101, "H264"); - kH264sprop1.SetParam("sprop-parameter-sets", "uvw"); + kH264sprop1.SetParam(kH264FmtpSpropParameterSets, "uvw"); parameters.codecs.push_back(kH264sprop1); cricket::VideoCodec kH264sprop2(102, "H264"); - kH264sprop2.SetParam("sprop-parameter-sets", "xyz"); + kH264sprop2.SetParam(kH264FmtpSpropParameterSets, "xyz"); parameters.codecs.push_back(kH264sprop2); EXPECT_TRUE(channel_->SetRecvParameters(parameters)); @@ -3765,19 +3766,17 @@ TEST_F(WebRtcVideoChannel2Test, DISABLED_GetRtpReceiveFmtpSprop) { ASSERT_EQ(2u, cfg.decoders.size()); EXPECT_EQ(101, cfg.decoders[0].payload_type); EXPECT_EQ("H264", cfg.decoders[0].payload_name); - std::string sprop; - const webrtc::DecoderSpecificSettings* decoder_specific; - decoder_specific = &cfg.decoders[0].decoder_specific; - ASSERT_TRUE(static_cast(decoder_specific->h264_extra_settings)); - sprop = decoder_specific->h264_extra_settings->sprop_parameter_sets; - EXPECT_EQ("uvw", sprop); + const auto it0 = + cfg.decoders[0].codec_params.find(kH264FmtpSpropParameterSets); + ASSERT_TRUE(it0 != cfg.decoders[0].codec_params.end()); + EXPECT_EQ("uvw", it0->second); EXPECT_EQ(102, cfg.decoders[1].payload_type); EXPECT_EQ("H264", cfg.decoders[1].payload_name); - decoder_specific = &cfg.decoders[1].decoder_specific; - ASSERT_TRUE(static_cast(decoder_specific->h264_extra_settings)); - sprop = decoder_specific->h264_extra_settings->sprop_parameter_sets; - EXPECT_EQ("xyz", sprop); + const auto it1 = + cfg.decoders[1].codec_params.find(kH264FmtpSpropParameterSets); + ASSERT_TRUE(it1 != cfg.decoders[1].codec_params.end()); + EXPECT_EQ("xyz", it1->second); } // Test that RtpParameters for receive stream has one encoding and it has diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index ea0289a3d1..cc14525654 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -51,10 +51,9 @@ std::string VideoReceiveStream::Decoder::ToString() const { ss << "{decoder: " << (decoder ? "(VideoDecoder)" : "nullptr"); ss << ", payload_type: " << payload_type; ss << ", payload_name: " << payload_name; - ss << ", decoder_specific: {"; - ss << " h264_extra_settings: " - << (decoder_specific.h264_extra_settings ? "(h264_extra_settings)" - : "nullptr"); + ss << ", codec_params: {"; + for (const auto& it : codec_params) + ss << it.first << ": " << it.second; ss << '}'; ss << '}'; diff --git a/webrtc/video_receive_stream.h b/webrtc/video_receive_stream.h index 848267fc49..abeb83b202 100644 --- a/webrtc/video_receive_stream.h +++ b/webrtc/video_receive_stream.h @@ -45,7 +45,10 @@ class VideoReceiveStream { // used to unpack incoming packets. std::string payload_name; - DecoderSpecificSettings decoder_specific; + // This map contains the codec specific parameters from SDP, i.e. the "fmtp" + // parameters. It is the same as cricket::CodecParameterMap used in + // cricket::VideoCodec. + std::map codec_params; }; struct Stats {