diff --git a/webrtc/pc/peerconnection_integrationtest.cc b/webrtc/pc/peerconnection_integrationtest.cc index 8a4df9b189..b4acdaa423 100644 --- a/webrtc/pc/peerconnection_integrationtest.cc +++ b/webrtc/pc/peerconnection_integrationtest.cc @@ -245,7 +245,7 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, received_sdp_munger_ = munger; } - // Siimlar to the above, but this is run on SDP immediately after it's + // Similar to the above, but this is run on SDP immediately after it's // generated. void SetGeneratedSdpMunger( std::function munger) { @@ -2701,6 +2701,60 @@ TEST_F(PeerConnectionIntegrationTest, EndToEndConnectionTimeWithTurnTurnPair) { delete SetCalleePcWrapperAndReturnCurrent(nullptr); } +// Test that audio and video flow end-to-end when codec names don't use the +// expected casing, given that they're supposed to be case insensitive. To test +// this, all but one codec is removed from each media description, and its +// casing is changed. +// +// In the past, this has regressed and caused crashes/black video, due to the +// fact that code at some layers was doing case-insensitive comparisons and +// code at other layers was not. +TEST_F(PeerConnectionIntegrationTest, CodecNamesAreCaseInsensitive) { + ASSERT_TRUE(CreatePeerConnectionWrappers()); + ConnectFakeSignaling(); + caller()->AddAudioVideoMediaStream(); + callee()->AddAudioVideoMediaStream(); + + // Remove all but one audio/video codec (opus and VP8), and change the + // casing of the caller's generated offer. + caller()->SetGeneratedSdpMunger([](cricket::SessionDescription* description) { + cricket::AudioContentDescription* audio = + GetFirstAudioContentDescription(description); + ASSERT_NE(nullptr, audio); + auto audio_codecs = audio->codecs(); + audio_codecs.erase(std::remove_if(audio_codecs.begin(), audio_codecs.end(), + [](const cricket::AudioCodec& codec) { + return codec.name != "opus"; + }), + audio_codecs.end()); + ASSERT_EQ(1u, audio_codecs.size()); + audio_codecs[0].name = "OpUs"; + audio->set_codecs(audio_codecs); + + cricket::VideoContentDescription* video = + GetFirstVideoContentDescription(description); + ASSERT_NE(nullptr, video); + auto video_codecs = video->codecs(); + video_codecs.erase(std::remove_if(video_codecs.begin(), video_codecs.end(), + [](const cricket::VideoCodec& codec) { + return codec.name != "VP8"; + }), + video_codecs.end()); + ASSERT_EQ(1u, video_codecs.size()); + video_codecs[0].name = "vP8"; + video->set_codecs(video_codecs); + }); + + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + // Verify frames are still received end-to-end. + ExpectNewFramesReceivedWithWait( + kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount, + kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount, + kMaxWaitForFramesMs); +} + } // namespace #endif // if !defined(THREAD_SANITIZER) diff --git a/webrtc/video/send_statistics_proxy.cc b/webrtc/video/send_statistics_proxy.cc index 7cefa294e6..58751a6d82 100644 --- a/webrtc/video/send_statistics_proxy.cc +++ b/webrtc/video/send_statistics_proxy.cc @@ -18,6 +18,7 @@ #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" #include "webrtc/base/trace_event.h" +#include "webrtc/common_types.h" #include "webrtc/modules/video_coding/include/video_codec_interface.h" #include "webrtc/system_wrappers/include/metrics.h" @@ -50,15 +51,21 @@ const char* GetUmaPrefix(VideoEncoderConfig::ContentType content_type) { HistogramCodecType PayloadNameToHistogramCodecType( const std::string& payload_name) { - if (payload_name == "VP8") { - return kVideoVp8; - } else if (payload_name == "VP9") { - return kVideoVp9; - } else if (payload_name == "H264") { - return kVideoH264; - } else { + rtc::Optional codecType = + PayloadNameToCodecType(payload_name); + if (!codecType) { return kVideoUnknown; } + switch (*codecType) { + case kVideoCodecVP8: + return kVideoVp8; + case kVideoCodecVP9: + return kVideoVp9; + case kVideoCodecH264: + return kVideoH264; + default: + return kVideoUnknown; + } } void UpdateCodecTypeHistogram(const std::string& payload_name) { diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 47e8c1efad..c59af899a4 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -21,6 +21,7 @@ #include "webrtc/base/logging.h" #include "webrtc/base/optional.h" #include "webrtc/base/trace_event.h" +#include "webrtc/common_types.h" #include "webrtc/common_video/h264/profile_level_id.h" #include "webrtc/common_video/libyuv/include/webrtc_libyuv.h" #include "webrtc/modules/rtp_rtcp/include/rtp_receiver.h" @@ -142,15 +143,8 @@ VideoCodec CreateDecoderVideoCodec(const VideoReceiveStream::Decoder& decoder) { codec.plType = decoder.payload_type; strncpy(codec.plName, decoder.payload_name.c_str(), sizeof(codec.plName)); - if (decoder.payload_name == "VP8") { - codec.codecType = kVideoCodecVP8; - } else if (decoder.payload_name == "VP9") { - codec.codecType = kVideoCodecVP9; - } else if (decoder.payload_name == "H264") { - codec.codecType = kVideoCodecH264; - } else { - codec.codecType = kVideoCodecGeneric; - } + codec.codecType = + PayloadNameToCodecType(decoder.payload_name).value_or(kVideoCodecGeneric); if (codec.codecType == kVideoCodecVP8) { *(codec.VP8()) = VideoEncoder::GetDefaultVp8Settings(); diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 5f21d57919..24e7e89688 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -263,9 +263,14 @@ std::string VideoSendStream::StreamStats::ToString() const { namespace { bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name) { - if (payload_name == "VP8" || payload_name == "VP9") + rtc::Optional codecType = + PayloadNameToCodecType(payload_name); + if (codecType && + (*codecType == kVideoCodecVP8 || *codecType == kVideoCodecVP9)) { return true; - RTC_DCHECK(payload_name == "H264" || payload_name == "FAKE") + } + RTC_DCHECK((codecType && *codecType == kVideoCodecH264) || + payload_name == "FAKE") << "unknown payload_name " << payload_name; return false; }