Change cricket::Codec default id from 0 to -1
This allows detecting if it has been set reliably.
0 is a valid payload type.
Bug: webrtc:360058654
Change-Id: Ic3646abe20d0247592145ad27549fa46ddb7ec90
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362261
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43016}
diff --git a/media/BUILD.gn b/media/BUILD.gn
index 74d935d..08e924a 100644
--- a/media/BUILD.gn
+++ b/media/BUILD.gn
@@ -898,6 +898,7 @@
":media_channel",
":media_constants",
":media_engine",
+ ":rid_description",
":rtc_audio_video",
":rtc_internal_video_codecs",
":rtc_media",
@@ -929,6 +930,7 @@
"../api:rtp_parameters",
"../api:scoped_refptr",
"../api:simulcast_test_fixture_api",
+ "../api:transport_api",
"../api/audio:audio_processing",
"../api/audio_codecs:audio_codecs_api",
"../api/audio_codecs:builtin_audio_decoder_factory",
@@ -946,11 +948,14 @@
"../api/units:time_delta",
"../api/units:timestamp",
"../api/video:builtin_video_bitrate_allocator_factory",
+ "../api/video:recordable_encoded_frame",
"../api/video:resolution",
"../api/video:video_bitrate_allocation",
+ "../api/video:video_bitrate_allocator_factory",
"../api/video:video_codec_constants",
"../api/video:video_frame",
"../api/video:video_rtp_headers",
+ "../api/video_codecs:scalability_mode",
"../api/video_codecs:video_codecs_api",
"../api/video_codecs:video_decoder_factory_template",
"../api/video_codecs:video_decoder_factory_template_dav1d_adapter",
@@ -964,6 +969,7 @@
"../api/video_codecs:video_encoder_factory_template_open_h264_adapter",
"../audio",
"../call:call_interfaces",
+ "../call:video_stream_api",
"../common_video",
"../modules/audio_device:mock_audio_device",
"../modules/audio_mixer:audio_mixer_impl",
@@ -989,6 +995,7 @@
"../rtc_base:rtc_base_tests_utils",
"../rtc_base:rtc_event",
"../rtc_base:safe_conversions",
+ "../rtc_base:socket",
"../rtc_base:stringutils",
"../rtc_base:threading",
"../rtc_base:timeutils",
@@ -1005,8 +1012,10 @@
"../test:test_support",
"../test:video_test_common",
"../test/time_controller",
+ "../video/config:encoder_config",
"../video/config:streams_config",
"//third_party/abseil-cpp/absl/algorithm:container",
+ "//third_party/abseil-cpp/absl/container:inlined_vector",
"//third_party/abseil-cpp/absl/memory",
"//third_party/abseil-cpp/absl/strings",
"//third_party/abseil-cpp/absl/strings:string_view",
diff --git a/media/base/codec.cc b/media/base/codec.cc
index e994f0e..0116821 100644
--- a/media/base/codec.cc
+++ b/media/base/codec.cc
@@ -197,15 +197,15 @@
bitrate(0),
channels(channels) {}
-Codec::Codec(Type type) : Codec(type, 0, "", 0) {}
+Codec::Codec(Type type) : Codec(type, kIdNotSet, "", 0) {}
Codec::Codec(const webrtc::SdpAudioFormat& c)
- : Codec(Type::kAudio, 0, c.name, c.clockrate_hz, c.num_channels) {
+ : Codec(Type::kAudio, kIdNotSet, c.name, c.clockrate_hz, c.num_channels) {
params = c.parameters;
}
Codec::Codec(const webrtc::SdpVideoFormat& c)
- : Codec(Type::kVideo, 0, c.name, kVideoCodecClockrate) {
+ : Codec(Type::kVideo, kIdNotSet, c.name, kVideoCodecClockrate) {
params = c.parameters;
scalability_modes = c.scalability_modes;
}
@@ -539,7 +539,7 @@
}
Codec CreateVideoCodec(const std::string& name) {
- return CreateVideoCodec(0, name);
+ return CreateVideoCodec(Codec::kIdNotSet, name);
}
Codec CreateVideoCodec(int id, const std::string& name) {
diff --git a/media/base/codec.h b/media/base/codec.h
index 583a14e..75bbc88 100644
--- a/media/base/codec.h
+++ b/media/base/codec.h
@@ -79,6 +79,8 @@
kFlexfec,
kRtx,
};
+ // Value of "id" if it's not explicitly set. Exposed for tests.
+ static const int kIdNotSet = -1;
Type type;
int id;
diff --git a/media/base/codec_unittest.cc b/media/base/codec_unittest.cc
index 62fbd76..84aa9ff 100644
--- a/media/base/codec_unittest.cc
+++ b/media/base/codec_unittest.cc
@@ -10,13 +10,18 @@
#include "media/base/codec.h"
-#include <tuple>
+#include <optional>
+#include <string>
+#include <vector>
-#include "api/video_codecs/av1_profile.h"
+#include "api/media_types.h"
+#include "api/rtp_parameters.h"
#include "api/video_codecs/h264_profile_level_id.h"
+#include "api/video_codecs/sdp_video_format.h"
#include "api/video_codecs/vp9_profile.h"
+#include "media/base/media_constants.h"
#include "modules/video_coding/codecs/h264/include/h264.h"
-#include "rtc_base/gunit.h"
+#include "test/gtest.h"
using cricket::Codec;
using cricket::FeedbackParam;
@@ -62,7 +67,7 @@
EXPECT_TRUE(c0 != c1);
TestCodec c5;
- TestCodec c6(0, "", 0);
+ TestCodec c6(Codec::kIdNotSet, "", 0);
EXPECT_TRUE(c5 == c6);
}
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 52981dd..a495f4a 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -11,32 +11,58 @@
#include "media/engine/webrtc_video_engine.h"
#include <algorithm>
+#include <cstddef>
#include <cstdint>
+#include <cstdlib>
+#include <cstring>
#include <map>
#include <memory>
+#include <optional>
+#include <set>
#include <string>
#include <tuple>
#include <utility>
#include <vector>
#include "absl/algorithm/container.h"
+#include "absl/container/inlined_vector.h"
#include "absl/strings/match.h"
+#include "api/call/transport.h"
+#include "api/crypto/crypto_options.h"
#include "api/environment/environment.h"
#include "api/environment/environment_factory.h"
+#include "api/make_ref_counted.h"
+#include "api/media_types.h"
+#include "api/priority.h"
+#include "api/rtc_error.h"
+#include "api/rtp_headers.h"
#include "api/rtp_parameters.h"
+#include "api/scoped_refptr.h"
#include "api/test/mock_encoder_selector.h"
#include "api/test/mock_video_bitrate_allocator.h"
#include "api/test/mock_video_bitrate_allocator_factory.h"
#include "api/test/mock_video_decoder_factory.h"
#include "api/test/mock_video_encoder_factory.h"
#include "api/test/video/function_video_decoder_factory.h"
+#include "api/transport/bitrate_settings.h"
#include "api/transport/field_trial_based_config.h"
+#include "api/transport/rtp/rtp_source.h"
#include "api/units/time_delta.h"
#include "api/units/timestamp.h"
#include "api/video/builtin_video_bitrate_allocator_factory.h"
#include "api/video/i420_buffer.h"
-#include "api/video/video_bitrate_allocation.h"
+#include "api/video/recordable_encoded_frame.h"
+#include "api/video/resolution.h"
+#include "api/video/video_bitrate_allocator_factory.h"
+#include "api/video/video_codec_constants.h"
+#include "api/video/video_codec_type.h"
+#include "api/video/video_content_type.h"
+#include "api/video/video_frame_buffer.h"
+#include "api/video/video_rotation.h"
+#include "api/video/video_sink_interface.h"
+#include "api/video/video_source_interface.h"
#include "api/video_codecs/h264_profile_level_id.h"
+#include "api/video_codecs/scalability_mode.h"
#include "api/video_codecs/sdp_video_format.h"
#include "api/video_codecs/video_codec.h"
#include "api/video_codecs/video_decoder_factory.h"
@@ -52,37 +78,53 @@
#include "api/video_codecs/video_encoder_factory_template_libvpx_vp8_adapter.h"
#include "api/video_codecs/video_encoder_factory_template_libvpx_vp9_adapter.h"
#include "api/video_codecs/video_encoder_factory_template_open_h264_adapter.h"
+#include "call/call.h"
+#include "call/call_config.h"
#include "call/flexfec_receive_stream.h"
+#include "call/video_receive_stream.h"
+#include "call/video_send_stream.h"
+#include "common_video/include/quality_limitation_reason.h"
+#include "media/base/codec.h"
#include "media/base/fake_frame_source.h"
#include "media/base/fake_network_interface.h"
#include "media/base/fake_video_renderer.h"
#include "media/base/media_channel.h"
+#include "media/base/media_config.h"
#include "media/base/media_constants.h"
-#include "media/base/rtp_utils.h"
+#include "media/base/media_engine.h"
+#include "media/base/rid_description.h"
+#include "media/base/stream_params.h"
#include "media/base/test_utils.h"
+#include "media/base/video_common.h"
#include "media/engine/fake_webrtc_call.h"
#include "media/engine/fake_webrtc_video_engine.h"
-#include "media/engine/webrtc_voice_engine.h"
+#include "modules/rtp_rtcp/include/report_block_data.h"
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
-#include "modules/rtp_rtcp/source/rtcp_packet.h"
+#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/source/rtcp_packet/report_block.h"
+#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "modules/rtp_rtcp/source/rtp_packet.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/video_coding/svc/scalability_mode_util.h"
#include "rtc_base/arraysize.h"
-#include "rtc_base/event.h"
+#include "rtc_base/async_packet_socket.h"
+#include "rtc_base/checks.h"
+#include "rtc_base/copy_on_write_buffer.h"
+#include "rtc_base/dscp.h"
#include "rtc_base/experiments/min_video_bitrate_experiment.h"
-#include "rtc_base/fake_clock.h"
-#include "rtc_base/gunit.h"
#include "rtc_base/numerics/safe_conversions.h"
+#include "rtc_base/socket.h"
#include "rtc_base/time_utils.h"
#include "test/fake_decoder.h"
#include "test/frame_forwarder.h"
#include "test/gmock.h"
+#include "test/gtest.h"
#include "test/rtcp_packet_parser.h"
#include "test/scoped_key_value_config.h"
#include "test/time_controller/simulated_time_controller.h"
#include "video/config/encoder_stream_factory.h"
#include "video/config/simulcast.h"
+#include "video/config/video_encoder_config.h"
using ::testing::_;
using ::testing::Combine;
@@ -8031,7 +8073,10 @@
// Set up WebRtcVideoChannel for 3-layer simulcast.
encoder_factory_->AddSupportedVideoCodecType(codec_name);
VideoSenderParameters parameters;
- parameters.codecs.push_back(cricket::CreateVideoCodec(codec_name));
+ cricket::Codec codec = cricket::CreateVideoCodec(codec_name);
+ // Codec ID does not matter, but must be valid.
+ codec.id = 123;
+ parameters.codecs.push_back(codec);
ASSERT_TRUE(send_channel_->SetSenderParameters(parameters));
FakeVideoSendStream* stream = SetUpSimulcast(true, /*with_rtx=*/false);
webrtc::test::FrameForwarder frame_forwarder;
@@ -9351,7 +9396,7 @@
}
protected:
- void VerifySimulcastSettings(const Codec& codec,
+ void VerifySimulcastSettings(const Codec& codec_in,
int capture_width,
int capture_height,
size_t num_configured_streams,
@@ -9359,6 +9404,9 @@
bool screenshare,
bool conference_mode) {
cricket::VideoSenderParameters parameters;
+ // The codec ID does not matter, but must be valid.
+ Codec codec = codec_in;
+ codec.id = 123;
parameters.codecs.push_back(codec);
parameters.conference_mode = conference_mode;
ASSERT_TRUE(send_channel_->SetSenderParameters(parameters));
diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc
index 6c08bf4..9cca26a 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -25,6 +25,8 @@
#include "absl/strings/str_replace.h"
#include "absl/strings/string_view.h"
#include "api/array_view.h"
+#include "api/candidate.h"
+#include "api/jsep.h"
#include "api/jsep_session_description.h"
#include "api/media_types.h"
#include "api/rtp_parameters.h"
@@ -2281,6 +2283,8 @@
TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithH264) {
cricket::Codec h264_codec = cricket::CreateVideoCodec("H264");
+ // Id must be valid, but value doesn't matter.
+ h264_codec.id = 123;
h264_codec.SetParam("profile-level-id", "42e01f");
h264_codec.SetParam("level-asymmetry-allowed", "1");
h264_codec.SetParam("packetization-mode", "1");
