Fixing some case-sensitive codec name comparisons.
As specified in RFC 4288, Section 4.2, and RFC 4855, Section 3, these
names should be case-insensitive. They already were being treated as
case-insensitive in some other places.
This bug was resulting in either a crash or no decoded video, depending
on the platform.
BUG=webrtc:6439, webrtc:7027
Review-Url: https://codereview.webrtc.org/2782273002
Cr-Commit-Position: refs/heads/master@{#17515}
diff --git a/webrtc/pc/peerconnection_integrationtest.cc b/webrtc/pc/peerconnection_integrationtest.cc
index 8a4df9b..b4acdaa 100644
--- a/webrtc/pc/peerconnection_integrationtest.cc
+++ b/webrtc/pc/peerconnection_integrationtest.cc
@@ -245,7 +245,7 @@
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<void(cricket::SessionDescription*)> munger) {
@@ -2701,6 +2701,60 @@
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 7cefa29..58751a6 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 @@
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<VideoCodecType> 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 47e8c1e..c59af89 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 @@
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 5f21d57..24e7e89 100644
--- a/webrtc/video/video_send_stream.cc
+++ b/webrtc/video/video_send_stream.cc
@@ -263,9 +263,14 @@
namespace {
bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name) {
- if (payload_name == "VP8" || payload_name == "VP9")
+ rtc::Optional<VideoCodecType> 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;
}