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}
This commit is contained in:
Harald Alvestrand 2024-09-12 14:02:21 +00:00 committed by WebRTC LUCI CQ
parent dfd8f570cd
commit 6aab4ccf42
6 changed files with 85 additions and 17 deletions

View File

@ -898,6 +898,7 @@ if (rtc_include_tests) {
":media_channel", ":media_channel",
":media_constants", ":media_constants",
":media_engine", ":media_engine",
":rid_description",
":rtc_audio_video", ":rtc_audio_video",
":rtc_internal_video_codecs", ":rtc_internal_video_codecs",
":rtc_media", ":rtc_media",
@ -929,6 +930,7 @@ if (rtc_include_tests) {
"../api:rtp_parameters", "../api:rtp_parameters",
"../api:scoped_refptr", "../api:scoped_refptr",
"../api:simulcast_test_fixture_api", "../api:simulcast_test_fixture_api",
"../api:transport_api",
"../api/audio:audio_processing", "../api/audio:audio_processing",
"../api/audio_codecs:audio_codecs_api", "../api/audio_codecs:audio_codecs_api",
"../api/audio_codecs:builtin_audio_decoder_factory", "../api/audio_codecs:builtin_audio_decoder_factory",
@ -946,11 +948,14 @@ if (rtc_include_tests) {
"../api/units:time_delta", "../api/units:time_delta",
"../api/units:timestamp", "../api/units:timestamp",
"../api/video:builtin_video_bitrate_allocator_factory", "../api/video:builtin_video_bitrate_allocator_factory",
"../api/video:recordable_encoded_frame",
"../api/video:resolution", "../api/video:resolution",
"../api/video:video_bitrate_allocation", "../api/video:video_bitrate_allocation",
"../api/video:video_bitrate_allocator_factory",
"../api/video:video_codec_constants", "../api/video:video_codec_constants",
"../api/video:video_frame", "../api/video:video_frame",
"../api/video:video_rtp_headers", "../api/video:video_rtp_headers",
"../api/video_codecs:scalability_mode",
"../api/video_codecs:video_codecs_api", "../api/video_codecs:video_codecs_api",
"../api/video_codecs:video_decoder_factory_template", "../api/video_codecs:video_decoder_factory_template",
"../api/video_codecs:video_decoder_factory_template_dav1d_adapter", "../api/video_codecs:video_decoder_factory_template_dav1d_adapter",
@ -964,6 +969,7 @@ if (rtc_include_tests) {
"../api/video_codecs:video_encoder_factory_template_open_h264_adapter", "../api/video_codecs:video_encoder_factory_template_open_h264_adapter",
"../audio", "../audio",
"../call:call_interfaces", "../call:call_interfaces",
"../call:video_stream_api",
"../common_video", "../common_video",
"../modules/audio_device:mock_audio_device", "../modules/audio_device:mock_audio_device",
"../modules/audio_mixer:audio_mixer_impl", "../modules/audio_mixer:audio_mixer_impl",
@ -989,6 +995,7 @@ if (rtc_include_tests) {
"../rtc_base:rtc_base_tests_utils", "../rtc_base:rtc_base_tests_utils",
"../rtc_base:rtc_event", "../rtc_base:rtc_event",
"../rtc_base:safe_conversions", "../rtc_base:safe_conversions",
"../rtc_base:socket",
"../rtc_base:stringutils", "../rtc_base:stringutils",
"../rtc_base:threading", "../rtc_base:threading",
"../rtc_base:timeutils", "../rtc_base:timeutils",
@ -1005,8 +1012,10 @@ if (rtc_include_tests) {
"../test:test_support", "../test:test_support",
"../test:video_test_common", "../test:video_test_common",
"../test/time_controller", "../test/time_controller",
"../video/config:encoder_config",
"../video/config:streams_config", "../video/config:streams_config",
"//third_party/abseil-cpp/absl/algorithm:container", "//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/memory",
"//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/strings",
"//third_party/abseil-cpp/absl/strings:string_view", "//third_party/abseil-cpp/absl/strings:string_view",

View File

@ -197,15 +197,15 @@ Codec::Codec(Type type,
bitrate(0), bitrate(0),
channels(channels) {} 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::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; params = c.parameters;
} }
Codec::Codec(const webrtc::SdpVideoFormat& c) Codec::Codec(const webrtc::SdpVideoFormat& c)
: Codec(Type::kVideo, 0, c.name, kVideoCodecClockrate) { : Codec(Type::kVideo, kIdNotSet, c.name, kVideoCodecClockrate) {
params = c.parameters; params = c.parameters;
scalability_modes = c.scalability_modes; scalability_modes = c.scalability_modes;
} }
@ -539,7 +539,7 @@ Codec CreateAudioCodec(const webrtc::SdpAudioFormat& c) {
} }
Codec CreateVideoCodec(const std::string& name) { Codec CreateVideoCodec(const std::string& name) {
return CreateVideoCodec(0, name); return CreateVideoCodec(Codec::kIdNotSet, name);
} }
Codec CreateVideoCodec(int id, const std::string& name) { Codec CreateVideoCodec(int id, const std::string& name) {

View File

@ -79,6 +79,8 @@ struct RTC_EXPORT Codec {
kFlexfec, kFlexfec,
kRtx, kRtx,
}; };
// Value of "id" if it's not explicitly set. Exposed for tests.
static const int kIdNotSet = -1;
Type type; Type type;
int id; int id;

View File

@ -10,13 +10,18 @@
#include "media/base/codec.h" #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/h264_profile_level_id.h"
#include "api/video_codecs/sdp_video_format.h"
#include "api/video_codecs/vp9_profile.h" #include "api/video_codecs/vp9_profile.h"
#include "media/base/media_constants.h"
#include "modules/video_coding/codecs/h264/include/h264.h" #include "modules/video_coding/codecs/h264/include/h264.h"
#include "rtc_base/gunit.h" #include "test/gtest.h"
using cricket::Codec; using cricket::Codec;
using cricket::FeedbackParam; using cricket::FeedbackParam;
@ -62,7 +67,7 @@ TEST(CodecTest, TestCodecOperators) {
EXPECT_TRUE(c0 != c1); EXPECT_TRUE(c0 != c1);
TestCodec c5; TestCodec c5;
TestCodec c6(0, "", 0); TestCodec c6(Codec::kIdNotSet, "", 0);
EXPECT_TRUE(c5 == c6); EXPECT_TRUE(c5 == c6);
} }

View File

@ -11,32 +11,58 @@
#include "media/engine/webrtc_video_engine.h" #include "media/engine/webrtc_video_engine.h"
#include <algorithm> #include <algorithm>
#include <cstddef>
#include <cstdint> #include <cstdint>
#include <cstdlib>
#include <cstring>
#include <map> #include <map>
#include <memory> #include <memory>
#include <optional>
#include <set>
#include <string> #include <string>
#include <tuple> #include <tuple>
#include <utility> #include <utility>
#include <vector> #include <vector>
#include "absl/algorithm/container.h" #include "absl/algorithm/container.h"
#include "absl/container/inlined_vector.h"
#include "absl/strings/match.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.h"
#include "api/environment/environment_factory.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/rtp_parameters.h"
#include "api/scoped_refptr.h"
#include "api/test/mock_encoder_selector.h" #include "api/test/mock_encoder_selector.h"
#include "api/test/mock_video_bitrate_allocator.h" #include "api/test/mock_video_bitrate_allocator.h"
#include "api/test/mock_video_bitrate_allocator_factory.h" #include "api/test/mock_video_bitrate_allocator_factory.h"
#include "api/test/mock_video_decoder_factory.h" #include "api/test/mock_video_decoder_factory.h"
#include "api/test/mock_video_encoder_factory.h" #include "api/test/mock_video_encoder_factory.h"
#include "api/test/video/function_video_decoder_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/field_trial_based_config.h"
#include "api/transport/rtp/rtp_source.h"
#include "api/units/time_delta.h" #include "api/units/time_delta.h"
#include "api/units/timestamp.h" #include "api/units/timestamp.h"
#include "api/video/builtin_video_bitrate_allocator_factory.h" #include "api/video/builtin_video_bitrate_allocator_factory.h"
#include "api/video/i420_buffer.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/h264_profile_level_id.h"
#include "api/video_codecs/scalability_mode.h"
#include "api/video_codecs/sdp_video_format.h" #include "api/video_codecs/sdp_video_format.h"
#include "api/video_codecs/video_codec.h" #include "api/video_codecs/video_codec.h"
#include "api/video_codecs/video_decoder_factory.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_vp8_adapter.h"
#include "api/video_codecs/video_encoder_factory_template_libvpx_vp9_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 "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/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_frame_source.h"
#include "media/base/fake_network_interface.h" #include "media/base/fake_network_interface.h"
#include "media/base/fake_video_renderer.h" #include "media/base/fake_video_renderer.h"
#include "media/base/media_channel.h" #include "media/base/media_channel.h"
#include "media/base/media_config.h"
#include "media/base/media_constants.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/test_utils.h"
#include "media/base/video_common.h"
#include "media/engine/fake_webrtc_call.h" #include "media/engine/fake_webrtc_call.h"
#include "media/engine/fake_webrtc_video_engine.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/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.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/video_coding/svc/scalability_mode_util.h" #include "modules/video_coding/svc/scalability_mode_util.h"
#include "rtc_base/arraysize.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/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/numerics/safe_conversions.h"
#include "rtc_base/socket.h"
#include "rtc_base/time_utils.h" #include "rtc_base/time_utils.h"
#include "test/fake_decoder.h" #include "test/fake_decoder.h"
#include "test/frame_forwarder.h" #include "test/frame_forwarder.h"
#include "test/gmock.h" #include "test/gmock.h"
#include "test/gtest.h"
#include "test/rtcp_packet_parser.h" #include "test/rtcp_packet_parser.h"
#include "test/scoped_key_value_config.h" #include "test/scoped_key_value_config.h"
#include "test/time_controller/simulated_time_controller.h" #include "test/time_controller/simulated_time_controller.h"
#include "video/config/encoder_stream_factory.h" #include "video/config/encoder_stream_factory.h"
#include "video/config/simulcast.h" #include "video/config/simulcast.h"
#include "video/config/video_encoder_config.h"
using ::testing::_; using ::testing::_;
using ::testing::Combine; using ::testing::Combine;
@ -8031,7 +8073,10 @@ TEST_P(WebRtcVideoChannelScaleResolutionDownByTest, ScaleResolutionDownBy) {
// Set up WebRtcVideoChannel for 3-layer simulcast. // Set up WebRtcVideoChannel for 3-layer simulcast.
encoder_factory_->AddSupportedVideoCodecType(codec_name); encoder_factory_->AddSupportedVideoCodecType(codec_name);
VideoSenderParameters parameters; 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)); ASSERT_TRUE(send_channel_->SetSenderParameters(parameters));
FakeVideoSendStream* stream = SetUpSimulcast(true, /*with_rtx=*/false); FakeVideoSendStream* stream = SetUpSimulcast(true, /*with_rtx=*/false);
webrtc::test::FrameForwarder frame_forwarder; webrtc::test::FrameForwarder frame_forwarder;
@ -9351,7 +9396,7 @@ class WebRtcVideoChannelSimulcastTest : public ::testing::Test {
} }
protected: protected:
void VerifySimulcastSettings(const Codec& codec, void VerifySimulcastSettings(const Codec& codec_in,
int capture_width, int capture_width,
int capture_height, int capture_height,
size_t num_configured_streams, size_t num_configured_streams,
@ -9359,6 +9404,9 @@ class WebRtcVideoChannelSimulcastTest : public ::testing::Test {
bool screenshare, bool screenshare,
bool conference_mode) { bool conference_mode) {
cricket::VideoSenderParameters parameters; 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.codecs.push_back(codec);
parameters.conference_mode = conference_mode; parameters.conference_mode = conference_mode;
ASSERT_TRUE(send_channel_->SetSenderParameters(parameters)); ASSERT_TRUE(send_channel_->SetSenderParameters(parameters));

View File

@ -25,6 +25,8 @@
#include "absl/strings/str_replace.h" #include "absl/strings/str_replace.h"
#include "absl/strings/string_view.h" #include "absl/strings/string_view.h"
#include "api/array_view.h" #include "api/array_view.h"
#include "api/candidate.h"
#include "api/jsep.h"
#include "api/jsep_session_description.h" #include "api/jsep_session_description.h"
#include "api/media_types.h" #include "api/media_types.h"
#include "api/rtp_parameters.h" #include "api/rtp_parameters.h"
@ -2281,6 +2283,8 @@ TEST_F(WebRtcSdpTest, ParseSslTcpCandidate) {
TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithH264) { TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithH264) {
cricket::Codec h264_codec = cricket::CreateVideoCodec("H264"); 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("profile-level-id", "42e01f");
h264_codec.SetParam("level-asymmetry-allowed", "1"); h264_codec.SetParam("level-asymmetry-allowed", "1");
h264_codec.SetParam("packetization-mode", "1"); h264_codec.SetParam("packetization-mode", "1");