admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Implement read-only codecPayloadType in RtpParameters"

Browse Source 
This reverts commit 806e06d1366b58878ced05cdd8d1d56394982fe6.

Reason for revert: Breaks WebRTC roll to Chromium. https://chromium-review.googlesource.com/c/chromium/src/+/1375538

02:52:35.346 7748   [6936:11248:1213/025234.206:ERROR:mediaengine.cc(80)] Attempted to set RtpParameters with modified codecPayloadType (INVALID_MODIFICATION)

Original change's description:
> Implement read-only codecPayloadType in RtpParameters
> 
> Bug: webrtc:7580
> Change-Id: I6d901afa97262b6c6d9fe6c7366df465ec77bfb3
> Reviewed-on: https://webrtc-review.googlesource.com/c/113944
> Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
> Reviewed-by: Seth Hampson <shampson@webrtc.org>
> Reviewed-by: Anders Carlsson <andersc@webrtc.org>
> Reviewed-by: Steve Anton <steveanton@webrtc.org>
> Commit-Queue: Florent Castelli <orphis@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#25993}

TBR=steveanton@webrtc.org,sakal@webrtc.org,andersc@webrtc.org,shampson@webrtc.org,orphis@webrtc.org

Change-Id: I157f9a79ae7133395431891e15e2c053559d359b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:7580
Reviewed-on: https://webrtc-review.googlesource.com/c/114300
Reviewed-by: Henrik Grunell <henrikg@webrtc.org>
Commit-Queue: Henrik Grunell <henrikg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26000}
This commit is contained in:
Henrik Grunell 2018-12-13 12:13:22 +00:00 committed by Commit Bot
parent 94c0f2645e
commit e1301a8b3a
13 changed files with 38 additions and 108 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
api/rtpparameters.h
View File

@ -377,7 +377,12 @@ struct RtpEncodingParameters {
// unset SSRC acts as a "wildcard" SSRC.
absl::optional<uint32_t> ssrc;
// Read-only parameter indicating the payload type of the codec being used.
// Can be used to reference a codec in the |codecs| member of the
// RtpParameters that contains this RtpEncodingParameters. If unset, the
// implementation will choose the first possible codec (if a sender), or
// prepare to receive any codec (for a receiver).
// TODO(deadbeef): Not implemented. Implementation of RtpSender will always
// choose the first codec from the list.
absl::optional<int> codec_payload_type;
// Specifies the FEC mechanism, if set.

6
media/base/mediaengine.cc
View File

@ -73,12 +73,6 @@ webrtc::RTCError ValidateRtpParameters(
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION,
"Attempted to set RtpParameters with modified SSRC");
}
if (rtp_parameters.encodings[i].codec_payload_type !=
old_rtp_parameters.encodings[i].codec_payload_type) {
LOG_AND_RETURN_ERROR(
RTCErrorType::INVALID_MODIFICATION,
"Attempted to set RtpParameters with modified codecPayloadType");
}
if (rtp_parameters.encodings[i].bitrate_priority <= 0) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_RANGE,
"Attempted to set RtpParameters bitrate_priority to "

4
media/engine/webrtcvideoengine.cc
View File

@ -1765,10 +1765,6 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetCodec(
parameters_.codec_settings = codec_settings;
for (auto& encoding : rtp_parameters_.encodings) {
encoding.codec_payload_type = codec_settings.codec.id;
}
// TODO(nisse): Avoid recreation, it should be enough to call
// ReconfigureEncoder.
RTC_LOG(LS_INFO) << "RecreateWebRtcStream (send) because of SetCodec.";

3
media/engine/webrtcvoiceengine.cc
View File

@ -1050,9 +1050,6 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
max_send_bitrate_bps_, rtp_parameters_.encodings[0].max_bitrate_bps,
*audio_codec_spec_);
rtp_parameters_.encodings[0].codec_payload_type =
send_codec_spec.payload_type;
UpdateAllowedBitrateRange();
}

6
pc/peerconnection.cc
View File

@ -1443,12 +1443,6 @@ PeerConnection::AddTransceiver(
RTCErrorType::UNSUPPORTED_PARAMETER,
"Attempted to set an unimplemented parameter of RtpParameters.");
}
if (encoding.codec_payload_type.has_value()) {
LOG_AND_RETURN_ERROR(
RTCErrorType::INVALID_MODIFICATION,
"Attempted to set a read-only value in RtpParameters.");
}
}
RtpParameters parameters;

28
pc/peerconnection_integrationtest.cc
View File

@ -4586,34 +4586,6 @@ TEST_P(PeerConnectionIntegrationTest, GetSourcesVideo) {
EXPECT_EQ(webrtc::RtpSourceType::SSRC, sources[0].source_type());
}
TEST_P(PeerConnectionIntegrationTest, GetParametersCodecPayloadTypeAudio) {
ASSERT_TRUE(CreatePeerConnectionWrappers());
ConnectFakeSignaling();
caller()->AddAudioTrack();
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
ASSERT_EQ(caller()->pc()->GetSenders().size(), 1u);
auto sender = caller()->pc()->GetSenders()[0];
ASSERT_EQ(sender->media_type(), cricket::MEDIA_TYPE_AUDIO);
ASSERT_GT(sender->GetParameters().encodings.size(), 0u);
EXPECT_TRUE(
sender->GetParameters().encodings[0].codec_payload_type.has_value());
}
TEST_P(PeerConnectionIntegrationTest, GetParametersCodecPayloadTypeVideo) {
ASSERT_TRUE(CreatePeerConnectionWrappers());
ConnectFakeSignaling();
caller()->AddVideoTrack();
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
ASSERT_EQ(caller()->pc()->GetSenders().size(), 1u);
auto sender = caller()->pc()->GetSenders()[0];
ASSERT_EQ(sender->media_type(), cricket::MEDIA_TYPE_VIDEO);
ASSERT_GT(sender->GetParameters().encodings.size(), 0u);
EXPECT_TRUE(
sender->GetParameters().encodings[0].codec_payload_type.has_value());
}
// Test that if a track is removed and added again with a different stream ID,
// the new stream ID is successfully communicated in SDP and media continues to
// flow end-to-end.

10
pc/peerconnection_rtp_unittest.cc
View File

@ -1424,7 +1424,7 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan,
auto default_send_encodings = init.send_encodings;
// Unimplemented RtpParameters: ssrc, fec, rtx, dtx,
// Unimplemented RtpParameters: ssrc, codec_payload_type, fec, rtx, dtx,
// ptime, scale_resolution_down_by, scale_framerate_down_by, rid,
// dependency_rids.
init.send_encodings[0].ssrc = 1;
@ -1435,6 +1435,14 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan,
.type());
init.send_encodings = default_send_encodings;
init.send_encodings[0].codec_payload_type = 1;
EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
caller->pc()
->AddTransceiver(cricket::MEDIA_TYPE_AUDIO, init)
.error()
.type());
init.send_encodings = default_send_encodings;
init.send_encodings[0].fec = RtpFecParameters();
EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
caller->pc()

3
pc/rtpsender.cc
View File

@ -38,7 +38,8 @@ int GenerateUniqueId() {
// contains a value.
bool UnimplementedRtpEncodingParameterHasValue(
const RtpEncodingParameters& encoding_params) {
if (encoding_params.fec.has_value() || encoding_params.rtx.has_value() ||
if (encoding_params.codec_payload_type.has_value() ||
encoding_params.fec.has_value() || encoding_params.rtx.has_value() ||
encoding_params.dtx.has_value() || encoding_params.ptime.has_value() ||
!encoding_params.rid.empty() ||
encoding_params.scale_resolution_down_by.has_value() ||

45
pc/rtpsenderreceiver_unittest.cc
View File

@ -798,33 +798,19 @@ TEST_F(RtpSenderReceiverTest, AudioSenderCantSetUnimplementedRtpParameters) {
DestroyAudioRtpSender();
}
TEST_F(RtpSenderReceiverTest, AudioSenderCantSetReadOnlyEncodingParameters) {
CreateAudioRtpSender();
RtpParameters params = audio_rtp_sender_->GetParameters();
for (size_t i = 0; i < params.encodings.size(); i++) {
params.encodings[i].ssrc = 1337;
EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION,
audio_rtp_sender_->SetParameters(params).type());
params = audio_rtp_sender_->GetParameters();
params.encodings[i].codec_payload_type = 42;
EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION,
audio_rtp_sender_->SetParameters(params).type());
params = audio_rtp_sender_->GetParameters();
}
DestroyAudioRtpSender();
}
TEST_F(RtpSenderReceiverTest,
AudioSenderCantSetUnimplementedRtpEncodingParameters) {
CreateAudioRtpSender();
RtpParameters params = audio_rtp_sender_->GetParameters();
EXPECT_EQ(1u, params.encodings.size());
// Unimplemented RtpParameters: fec, rtx, dtx, ptime,
// Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime,
// scale_resolution_down_by, scale_framerate_down_by, rid, dependency_rids.
params.encodings[0].codec_payload_type = 1;
EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
audio_rtp_sender_->SetParameters(params).type());
params = audio_rtp_sender_->GetParameters();
params.encodings[0].fec = RtpFecParameters();
EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
audio_rtp_sender_->SetParameters(params).type());
@ -1093,8 +1079,13 @@ TEST_F(RtpSenderReceiverTest,
RtpParameters params = video_rtp_sender_->GetParameters();
EXPECT_EQ(1u, params.encodings.size());
// Unimplemented RtpParameters: fec, rtx, dtx, ptime,
// Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime,
// scale_resolution_down_by, scale_framerate_down_by, rid, dependency_rids.
params.encodings[0].codec_payload_type = 1;
EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
video_rtp_sender_->SetParameters(params).type());
params = video_rtp_sender_->GetParameters();
params.encodings[0].fec = RtpFecParameters();
EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
video_rtp_sender_->SetParameters(params).type());
@ -1138,9 +1129,14 @@ TEST_F(RtpSenderReceiverTest,
RtpParameters params = video_rtp_sender_->GetParameters();
EXPECT_EQ(kVideoSimulcastLayerCount, params.encodings.size());
// Unimplemented RtpParameters: fec, rtx, dtx, ptime,
// Unimplemented RtpParameters: codec_payload_type, fec, rtx, dtx, ptime,
// scale_resolution_down_by, scale_framerate_down_by, rid, dependency_rids.
for (size_t i = 0; i < params.encodings.size(); i++) {
params.encodings[i].codec_payload_type = 1;
EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
video_rtp_sender_->SetParameters(params).type());
params = video_rtp_sender_->GetParameters();
params.encodings[i].fec = RtpFecParameters();
EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER,
video_rtp_sender_->SetParameters(params).type());
@ -1209,11 +1205,6 @@ TEST_F(RtpSenderReceiverTest, VideoSenderCantSetReadOnlyEncodingParameters) {
EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION,
video_rtp_sender_->SetParameters(params).type());
params = video_rtp_sender_->GetParameters();
params.encodings[i].codec_payload_type = 1337;
EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION,
video_rtp_sender_->SetParameters(params).type());
params = video_rtp_sender_->GetParameters();
}
DestroyVideoRtpSender();

14
sdk/android/api/org/webrtc/RtpParameters.java
View File

@ -30,9 +30,6 @@ public class RtpParameters {
// Set to true to cause this encoding to be sent, and false for it not to
// be sent.
public boolean active = true;
// The payloadType of the codec used by the sender.
// Can't be changed between getParameters/setParameters.
@Nullable public Integer codecPayloadType;
// If non-null, this represents the Transport Independent Application
// Specific maximum bandwidth defined in RFC3890. If null, there is no
// maximum bitrate.
@ -48,10 +45,9 @@ public class RtpParameters {
public Long ssrc;
@CalledByNative("Encoding")
Encoding(boolean active, Integer codecPayloadType, Integer maxBitrateBps, Integer minBitrateBps,
Integer maxFramerate, Integer numTemporalLayers, Long ssrc) {
Encoding(boolean active, Integer maxBitrateBps, Integer minBitrateBps, Integer maxFramerate,
Integer numTemporalLayers, Long ssrc) {
this.active = active;
this.codecPayloadType = codecPayloadType;
this.maxBitrateBps = maxBitrateBps;
this.minBitrateBps = minBitrateBps;
this.maxFramerate = maxFramerate;
@ -59,12 +55,6 @@ public class RtpParameters {
this.ssrc = ssrc;
}
@Nullable
@CalledByNative("Encoding")
Integer getCodecPayloadType() {
return codecPayloadType;
}
@CalledByNative("Encoding")
boolean getActive() {
return active;

8
sdk/android/src/jni/pc/rtpparameters.cc
View File

@ -24,9 +24,7 @@ ScopedJavaLocalRef<jobject> NativeToJavaRtpEncodingParameter(
JNIEnv* env,
const RtpEncodingParameters& encoding) {
return Java_Encoding_Constructor(
env, encoding.active,
NativeToJavaInteger(env, encoding.codec_payload_type),
NativeToJavaInteger(env, encoding.max_bitrate_bps),
env, encoding.active, NativeToJavaInteger(env, encoding.max_bitrate_bps),
NativeToJavaInteger(env, encoding.min_bitrate_bps),
NativeToJavaInteger(env, encoding.max_framerate),
NativeToJavaInteger(env, encoding.num_temporal_layers),
@ -68,10 +66,6 @@ RtpEncodingParameters JavaToNativeRtpEncodingParameters(
encoding.active = Java_Encoding_getActive(jni, j_encoding_parameters);
ScopedJavaLocalRef<jobject> j_max_bitrate =
Java_Encoding_getMaxBitrateBps(jni, j_encoding_parameters);
ScopedJavaLocalRef<jobject> j_codec_payload_type =
Java_Encoding_getCodecPayloadType(jni, j_encoding_parameters);
encoding.codec_payload_type =
JavaToNativeOptionalInt(jni, j_codec_payload_type);
encoding.max_bitrate_bps = JavaToNativeOptionalInt(jni, j_max_bitrate);
ScopedJavaLocalRef<jobject> j_min_bitrate =
Java_Encoding_getMinBitrateBps(jni, j_encoding_parameters);

5
sdk/objc/api/peerconnection/RTCRtpEncodingParameters.h
View File

@ -17,11 +17,6 @@ NS_ASSUME_NONNULL_BEGIN
RTC_OBJC_EXPORT
@interface RTCRtpEncodingParameters : NSObject
/** The codec payloadType used by the encoder, or nil if it is not currently
* available.
*/
@property(nonatomic, readonly, nullable) NSNumber *codecPayloadType;
/** Controls whether the encoding is currently transmitted. */
@property(nonatomic, assign) BOOL isActive;

7
sdk/objc/api/peerconnection/RTCRtpEncodingParameters.mm
View File

@ -12,7 +12,6 @@
@implementation RTCRtpEncodingParameters
@synthesize codecPayloadType = _codecPayloadType;
@synthesize isActive = _isActive;
@synthesize maxBitrateBps = _maxBitrateBps;
@synthesize minBitrateBps = _minBitrateBps;
@ -27,9 +26,6 @@
- (instancetype)initWithNativeParameters:
(const webrtc::RtpEncodingParameters &)nativeParameters {
if (self = [self init]) {
if (nativeParameters.codec_payload_type) {
_codecPayloadType = [NSNumber numberWithInt:*nativeParameters.codec_payload_type];
}
_isActive = nativeParameters.active;
if (nativeParameters.max_bitrate_bps) {
_maxBitrateBps =
@ -54,9 +50,6 @@
- (webrtc::RtpEncodingParameters)nativeParameters {
webrtc::RtpEncodingParameters parameters;
if (_codecPayloadType != nil) {
parameters.codec_payload_type = absl::optional<int>(_codecPayloadType.intValue);
}
parameters.active = _isActive;
if (_maxBitrateBps != nil) {
parameters.max_bitrate_bps = absl::optional<int>(_maxBitrateBps.intValue);