From b5bf54c4e7efa1bd37ec42b9150f9746b015cd79 Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Fri, 5 Apr 2013 13:27:38 +0000 Subject: [PATCH] Permit arbitrary payload names for kVideoCodecGeneric. BUG=1575 Review URL: https://webrtc-codereview.appspot.com/1282005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@3768 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../rtp_rtcp/source/rtp_payload_registry.cc | 7 ++-- .../source/rtp_payload_registry_unittest.cc | 18 +++++++++++ .../rtp_rtcp/source/rtp_sender_video.cc | 4 +-- .../auto_test/source/vie_autotest_codec.cc | 32 +++++++++++++++++++ .../auto_test/source/vie_autotest_loopback.cc | 5 ++- webrtc/video_engine/vie_codec_impl.cc | 6 ++-- 6 files changed, 60 insertions(+), 12 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc index 2f27e12370..13b9b77bf7 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -39,6 +39,7 @@ WebRtc_Word32 RTPPayloadRegistry::RegisterReceivePayload( const WebRtc_UWord8 channels, const WebRtc_UWord32 rate, bool* created_new_payload) { + assert(payload_type >= 0); assert(payload_name); *created_new_payload = false; @@ -61,6 +62,7 @@ WebRtc_Word32 RTPPayloadRegistry::RegisterReceivePayload( default: break; } + size_t payload_name_length = strlen(payload_name); ModuleRTPUtility::PayloadTypeMap::iterator it = @@ -318,13 +320,10 @@ class RTPPayloadVideoStrategy : public RTPPayloadStrategy { videoType = kRtpVp8Video; } else if (ModuleRTPUtility::StringCompare(payloadName, "I420", 4)) { videoType = kRtpGenericVideo; - } else if (ModuleRTPUtility::StringCompare(payloadName, "GENERIC", 7)) { - videoType = kRtpGenericVideo; } else if (ModuleRTPUtility::StringCompare(payloadName, "ULPFEC", 6)) { videoType = kRtpFecVideo; } else { - assert(false); - return NULL; + videoType = kRtpGenericVideo; } ModuleRTPUtility::Payload* payload = new ModuleRTPUtility::Payload; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc index d9c0961f92..fa79b4a5e4 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc @@ -233,4 +233,22 @@ INSTANTIATE_TEST_CASE_P(TestKnownBadPayloadTypes, ParameterizedRtpPayloadRegistryTest, testing::Values(64, 72, 73, 74, 75, 76, 77, 78, 79)); +class RtpPayloadRegistryGenericTest : + public RtpPayloadRegistryTest, + public ::testing::WithParamInterface { +}; + +TEST_P(RtpPayloadRegistryGenericTest, RegisterGenericReceivePayloadType) { + int payload_type = GetParam(); + + bool ignored; + + EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload("generic-codec", + static_cast(payload_type), + 19, 1, 17, &ignored)); // dummy values, except for payload_type +} + +INSTANTIATE_TEST_CASE_P(TestDynamicRange, RtpPayloadRegistryGenericTest, + testing::Range(96, 127+1)); + } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc index 204f4e6d47..59d4403fa9 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -95,10 +95,8 @@ WebRtc_Word32 RTPSenderVideo::RegisterVideoPayload( videoType = kRtpVp8Video; } else if (ModuleRTPUtility::StringCompare(payloadName, "I420", 4)) { videoType = kRtpGenericVideo; - } else if (ModuleRTPUtility::StringCompare(payloadName, "GENERIC", 7)) { - videoType = kRtpGenericVideo; } else { - return -1; + videoType = kRtpGenericVideo; } payload = new ModuleRTPUtility::Payload; payload->name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; diff --git a/webrtc/video_engine/test/auto_test/source/vie_autotest_codec.cc b/webrtc/video_engine/test/auto_test/source/vie_autotest_codec.cc index a1e0f8d2b4..53c57f5d7c 100644 --- a/webrtc/video_engine/test/auto_test/source/vie_autotest_codec.cc +++ b/webrtc/video_engine/test/auto_test/source/vie_autotest_codec.cc @@ -10,6 +10,7 @@ #include "webrtc/common_types.h" #include "webrtc/engine_configurations.h" +#include "webrtc/modules/video_coding/codecs/i420/main/interface/i420.h" #include "webrtc/system_wrappers/interface/scoped_ptr.h" #include "webrtc/test/channel_transport/include/channel_transport.h" #include "webrtc/video_engine/test/auto_test/interface/vie_autotest_defines.h" @@ -497,12 +498,43 @@ void ViEAutoTest::ViECodecAPITest() { break; } } + memset(&video_codec, 0, sizeof(video_codec)); EXPECT_EQ(0, codec->GetSendCodec(video_channel, video_codec)); EXPECT_EQ(webrtc::kVideoCodecI420, video_codec.codecType); + // Register a generic codec + memset(&video_codec, 0, sizeof(video_codec)); + video_codec.codecType = webrtc::kVideoCodecGeneric; + strcpy(video_codec.plName, "generic-codec"); + uint8_t payload_type = 127; + video_codec.plType = payload_type; + video_codec.minBitrate = 100; + video_codec.startBitrate = 500; + video_codec.maxBitrate = 10000; + video_codec.width = 1920; + video_codec.height = 1080; + video_codec.maxFramerate = 30; + video_codec.qpMax = 50; + + webrtc::ViEExternalCodec* external_codec = + webrtc::ViEExternalCodec::GetInterface(video_engine); + EXPECT_TRUE(external_codec != NULL); + + // Any encoder will do. + webrtc::I420Encoder encoder; + EXPECT_EQ(0, external_codec->RegisterExternalSendCodec(video_channel, + payload_type, &encoder, + false)); + EXPECT_EQ(0, codec->SetSendCodec(video_channel, video_codec)); + + memset(&video_codec, 0, sizeof(video_codec)); + EXPECT_EQ(0, codec->GetSendCodec(video_channel, video_codec)); + EXPECT_EQ(webrtc::kVideoCodecGeneric, video_codec.codecType); + EXPECT_EQ(0, base->DeleteChannel(video_channel)); + EXPECT_EQ(0, external_codec->Release()); EXPECT_EQ(0, codec->Release()); EXPECT_EQ(0, base->Release()); EXPECT_TRUE(webrtc::VideoEngine::Delete(video_engine)); diff --git a/webrtc/video_engine/test/auto_test/source/vie_autotest_loopback.cc b/webrtc/video_engine/test/auto_test/source/vie_autotest_loopback.cc index 354535d027..ba0da62da3 100644 --- a/webrtc/video_engine/test/auto_test/source/vie_autotest_loopback.cc +++ b/webrtc/video_engine/test/auto_test/source/vie_autotest_loopback.cc @@ -342,6 +342,7 @@ int VideoEngineSampleCode(void* window1, void* window2) getchar(); codecIdx = codecIdx - 1; // Compensate for idx start at 1. #endif + // VP8 over generic transport gets this special one. if (codecIdx == ptrViECodec->NumberOfCodecs()) { for (codecIdx = 0; codecIdx < ptrViECodec->NumberOfCodecs(); ++codecIdx) { error = ptrViECodec->GetCodec(codecIdx, videoCodec); @@ -351,7 +352,9 @@ int VideoEngineSampleCode(void* window1, void* window2) } assert(videoCodec.codecType == webrtc::kVideoCodecVP8); videoCodec.codecType = webrtc::kVideoCodecGeneric; - strcpy(videoCodec.plName, "GENERIC"); + + // Any plName should work with generic + strcpy(videoCodec.plName, "VP8-GENERIC"); uint8_t pl_type = 127; videoCodec.plType = pl_type; webrtc::ViEExternalCodec* external_codec = webrtc::ViEExternalCodec diff --git a/webrtc/video_engine/vie_codec_impl.cc b/webrtc/video_engine/vie_codec_impl.cc index 42f1a876bd..7d2d95d27d 100644 --- a/webrtc/video_engine/vie_codec_impl.cc +++ b/webrtc/video_engine/vie_codec_impl.cc @@ -741,11 +741,9 @@ bool ViECodecImpl::CodecValid(const VideoCodec& video_codec) { } else if ((video_codec.codecType == kVideoCodecVP8 && strncmp(video_codec.plName, "VP8", 4) == 0) || (video_codec.codecType == kVideoCodecI420 && - strncmp(video_codec.plName, "I420", 4) == 0) || - (video_codec.codecType == kVideoCodecGeneric && - strncmp(video_codec.plName, "GENERIC", 7) == 0)) { + strncmp(video_codec.plName, "I420", 4) == 0)) { // OK. - } else { + } else if (video_codec.codecType != kVideoCodecGeneric) { WEBRTC_TRACE(kTraceError, kTraceVideo, -1, "Codec type doesn't match pl_name", video_codec.plType); return false;