From b7d9d8346f5e51145675636da07156f2b3c4a19f Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Tue, 15 May 2018 18:14:14 +0200 Subject: [PATCH] Implement RtpCodecParameters::parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will return all the fmtp parameters for the codecs, except for DTMF codes that don't fit the key=value pattern. Bug: webrtc:7112 Change-Id: I06a203ff64df2c3bc9bc2082cd0f374718b23510 Reviewed-on: https://webrtc-review.googlesource.com/71801 Commit-Queue: Florent Castelli Reviewed-by: Sami Kalliomäki Reviewed-by: Kári Helgason Reviewed-by: Taylor Brandstetter Reviewed-by: Steve Anton Cr-Commit-Position: refs/heads/master@{#23250} --- api/rtpparameters.h | 2 -- media/base/codec.cc | 1 + media/base/codec_unittest.cc | 12 ++++++++++-- sdk/android/api/org/webrtc/RtpParameters.java | 13 +++++++++++-- sdk/android/native_api/jni/java_types.h | 12 ++++++++++++ sdk/android/src/jni/pc/rtpparameters.cc | 6 +++++- sdk/android/src/jni/videocodecinfo.cc | 8 ++------ .../Classes/PeerConnection/RTCRtpCodecParameters.mm | 12 ++++++++++++ .../Headers/WebRTC/RTCRtpCodecParameters.h | 3 +++ 9 files changed, 56 insertions(+), 13 deletions(-) diff --git a/api/rtpparameters.h b/api/rtpparameters.h index e8ff11623c..49381e8700 100644 --- a/api/rtpparameters.h +++ b/api/rtpparameters.h @@ -498,8 +498,6 @@ struct RtpCodecParameters { // Contrary to ORTC, these parameters are named using all lowercase strings. // This helps make the mapping to SDP simpler, if an application is using // SDP. Boolean values are represented by the string "1". - // - // TODO(deadbeef): Not implemented with PeerConnection senders/receivers. std::unordered_map parameters; bool operator==(const RtpCodecParameters& o) const { diff --git a/media/base/codec.cc b/media/base/codec.cc index be3152aaf3..e050c722d2 100644 --- a/media/base/codec.cc +++ b/media/base/codec.cc @@ -143,6 +143,7 @@ webrtc::RtpCodecParameters Codec::ToCodecParameters() const { codec_params.payload_type = id; codec_params.name = name; codec_params.clock_rate = clockrate; + codec_params.parameters.insert(params.begin(), params.end()); return codec_params; } diff --git a/media/base/codec_unittest.cc b/media/base/codec_unittest.cc index d771a80281..8fa11bfd7a 100644 --- a/media/base/codec_unittest.cc +++ b/media/base/codec_unittest.cc @@ -347,19 +347,27 @@ TEST(CodecTest, TestValidateCodecFormat) { } TEST(CodecTest, TestToCodecParameters) { - const VideoCodec v(96, "V"); + VideoCodec v(96, "V"); + v.SetParam("p1", "v1"); webrtc::RtpCodecParameters codec_params_1 = v.ToCodecParameters(); EXPECT_EQ(96, codec_params_1.payload_type); EXPECT_EQ(cricket::MEDIA_TYPE_VIDEO, codec_params_1.kind); EXPECT_EQ("V", codec_params_1.name); EXPECT_EQ(cricket::kVideoCodecClockrate, codec_params_1.clock_rate); EXPECT_EQ(rtc::nullopt, codec_params_1.num_channels); + ASSERT_EQ(1, codec_params_1.parameters.size()); + EXPECT_EQ("p1", codec_params_1.parameters.begin()->first); + EXPECT_EQ("v1", codec_params_1.parameters.begin()->second); - const AudioCodec a(97, "A", 44100, 20000, 2); + AudioCodec a(97, "A", 44100, 20000, 2); + a.SetParam("p1", "a1"); webrtc::RtpCodecParameters codec_params_2 = a.ToCodecParameters(); EXPECT_EQ(97, codec_params_2.payload_type); EXPECT_EQ(cricket::MEDIA_TYPE_AUDIO, codec_params_2.kind); EXPECT_EQ("A", codec_params_2.name); EXPECT_EQ(44100, codec_params_2.clock_rate); EXPECT_EQ(2, codec_params_2.num_channels); + ASSERT_EQ(1, codec_params_2.parameters.size()); + EXPECT_EQ("p1", codec_params_2.parameters.begin()->first); + EXPECT_EQ("a1", codec_params_2.parameters.begin()->second); } diff --git a/sdk/android/api/org/webrtc/RtpParameters.java b/sdk/android/api/org/webrtc/RtpParameters.java index f2227ae53a..0e893bdc5f 100644 --- a/sdk/android/api/org/webrtc/RtpParameters.java +++ b/sdk/android/api/org/webrtc/RtpParameters.java @@ -11,8 +11,9 @@ package org.webrtc; import javax.annotation.Nullable; -import java.util.List; import java.util.ArrayList; +import java.util.List; +import java.util.Map; import org.webrtc.MediaStreamTrack; /** @@ -72,15 +73,18 @@ public class RtpParameters { public Integer clockRate; // The number of audio channels used. Set to null for video codecs. public Integer numChannels; + // The "format specific parameters" field from the "a=fmtp" line in the SDP + public Map parameters; @CalledByNative("Codec") Codec(int payloadType, String name, MediaStreamTrack.MediaType kind, Integer clockRate, - Integer numChannels) { + Integer numChannels, Map parameters) { this.payloadType = payloadType; this.name = name; this.kind = kind; this.clockRate = clockRate; this.numChannels = numChannels; + this.parameters = parameters; } @CalledByNative("Codec") @@ -107,6 +111,11 @@ public class RtpParameters { Integer getNumChannels() { return numChannels; } + + @CalledByNative("Codec") + Map getParameters() { + return parameters; + } } public final String transactionId; diff --git a/sdk/android/native_api/jni/java_types.h b/sdk/android/native_api/jni/java_types.h index ac58620a61..a84b7d9961 100644 --- a/sdk/android/native_api/jni/java_types.h +++ b/sdk/android/native_api/jni/java_types.h @@ -285,6 +285,18 @@ ScopedJavaLocalRef NativeToJavaMap(JNIEnv* env, return builder.GetJavaMap(); } +template +ScopedJavaLocalRef NativeToJavaStringMap(JNIEnv* env, + const C& container) { + JavaMapBuilder builder(env); + for (const auto& e : container) { + const auto key_value_pair = std::make_pair( + NativeToJavaString(env, e.first), NativeToJavaString(env, e.second)); + builder.put(key_value_pair.first, key_value_pair.second); + } + return builder.GetJavaMap(); +} + // Return a |jlong| that will correctly convert back to |ptr|. This is needed // because the alternative (of silently passing a 32-bit pointer to a vararg // function expecting a 64-bit param) picks up garbage in the high 32 bits. diff --git a/sdk/android/src/jni/pc/rtpparameters.cc b/sdk/android/src/jni/pc/rtpparameters.cc index a990934b01..b0b83eb811 100644 --- a/sdk/android/src/jni/pc/rtpparameters.cc +++ b/sdk/android/src/jni/pc/rtpparameters.cc @@ -35,7 +35,8 @@ ScopedJavaLocalRef NativeToJavaRtpCodecParameter( NativeToJavaString(env, codec.name), NativeToJavaMediaType(env, codec.kind), NativeToJavaInteger(env, codec.clock_rate), - NativeToJavaInteger(env, codec.num_channels)); + NativeToJavaInteger(env, codec.num_channels), + NativeToJavaStringMap(env, codec.parameters)); } } // namespace @@ -85,6 +86,9 @@ RtpParameters JavaToNativeRtpParameters(JNIEnv* jni, JavaToNativeOptionalInt(jni, Java_Codec_getClockRate(jni, j_codec)); codec.num_channels = JavaToNativeOptionalInt(jni, Java_Codec_getNumChannels(jni, j_codec)); + auto parameters_map = + JavaToNativeStringMap(jni, Java_Codec_getParameters(jni, j_codec)); + codec.parameters.insert(parameters_map.begin(), parameters_map.end()); parameters.codecs.push_back(codec); } return parameters; diff --git a/sdk/android/src/jni/videocodecinfo.cc b/sdk/android/src/jni/videocodecinfo.cc index e467e5fbe4..20a641d139 100644 --- a/sdk/android/src/jni/videocodecinfo.cc +++ b/sdk/android/src/jni/videocodecinfo.cc @@ -27,12 +27,8 @@ SdpVideoFormat VideoCodecInfoToSdpVideoFormat(JNIEnv* jni, ScopedJavaLocalRef SdpVideoFormatToVideoCodecInfo( JNIEnv* jni, const SdpVideoFormat& format) { - ScopedJavaLocalRef j_params = NativeToJavaMap( - jni, format.parameters, - [](JNIEnv* env, const std::pair& entry) { - return std::make_pair(NativeToJavaString(env, entry.first), - NativeToJavaString(env, entry.second)); - }); + ScopedJavaLocalRef j_params = + NativeToJavaStringMap(jni, format.parameters); return Java_VideoCodecInfo_Constructor( jni, NativeToJavaString(jni, format.name), j_params); } diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpCodecParameters.mm b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpCodecParameters.mm index 1a3bcf4dac..b6baee6054 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpCodecParameters.mm +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpCodecParameters.mm @@ -41,6 +41,7 @@ const NSString * const kRTCH264CodecName = @(cricket::kH264CodecName); @synthesize kind = _kind; @synthesize clockRate = _clockRate; @synthesize numChannels = _numChannels; +@synthesize parameters = _parameters; - (instancetype)init { return [super init]; @@ -68,6 +69,12 @@ const NSString * const kRTCH264CodecName = @(cricket::kH264CodecName); if (nativeParameters.num_channels) { _numChannels = [NSNumber numberWithInt:*nativeParameters.num_channels]; } + NSMutableDictionary *parameters = [NSMutableDictionary dictionary]; + for (const auto ¶meter : nativeParameters.parameters) { + [parameters setObject:[NSString stringForStdString:parameter.second] + forKey:[NSString stringForStdString:parameter.first]]; + } + _parameters = parameters; } return self; } @@ -91,6 +98,11 @@ const NSString * const kRTCH264CodecName = @(cricket::kH264CodecName); if (_numChannels != nil) { parameters.num_channels = rtc::Optional(_numChannels.intValue); } + for (NSString *paramKey in _parameters.allKeys) { + std::string key = [NSString stdStringForString:paramKey]; + std::string value = [NSString stdStringForString:_parameters[paramKey]]; + parameters.parameters[key] = value; + } return parameters; } diff --git a/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h b/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h index c7ca2f59c0..4ba03641a9 100644 --- a/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h +++ b/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h @@ -63,6 +63,9 @@ RTC_EXPORT **/ @property(nonatomic, readonly, nullable) NSNumber *numChannels; +/** The "format specific parameters" field from the "a=fmtp" line in the SDP */ +@property(nonatomic, readonly, nonnull) NSDictionary *parameters; + - (instancetype)init NS_DESIGNATED_INITIALIZER; @end