[Java] Make default sdpSemantics NOT_SPECIFIED.
The default value of sdpSemantics is about to change from PLAN_B to
UNIFIED_PLAN. In order not to cause subtle bugs by applications that
depend on the default value being PLAN_B, we are temporarily making the
default NOT_SPECIFIED. Constructing with NOT_SPECIFIED causes the C++
layer to crash (https://webrtc-review.googlesource.com/c/src/+/242968).
This is in accordance to the publically announced plans:
https://groups.google.com/u/1/g/discuss-webrtc/c/SdoVP02eUIk
While we're at it, we're upgrading almost all unit tests to use Unified
Plan. However there are still two tests using Plan B for which I added
TODO comments to be dealt with later; not having an Android setup makes
it impossible to debug these efficiently.
Bug: webrtc:11121
Change-Id: Ib086186bee947d18d31b413e3aeba0cb247b377d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/246000
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Xavier Lepaul <xalep@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35700}
diff --git a/sdk/android/api/org/webrtc/PeerConnection.java b/sdk/android/api/org/webrtc/PeerConnection.java
index 468413b..56d584e 100644
--- a/sdk/android/api/org/webrtc/PeerConnection.java
+++ b/sdk/android/api/org/webrtc/PeerConnection.java
@@ -432,9 +432,10 @@
/**
* Java version of webrtc::SdpSemantics.
*
- * Configure the SDP semantics used by this PeerConnection. Note that the
- * WebRTC 1.0 specification requires UNIFIED_PLAN semantics. The
- * RtpTransceiver API is only available with UNIFIED_PLAN semantics.
+ * Configure the SDP semantics used by this PeerConnection. That the WebRTC
+ * 1.0 specification requires UNIFIED_PLAN semantics and the RtpTransceiver
+ * API is only available with UNIFIED_PLAN semantics. PLAN_B is being
+ * deprecated and will be removed at a future date.
*
* <p>PLAN_B will cause PeerConnection to create offers and answers with at
* most one audio and one video m= section with multiple RtpSenders and
@@ -448,13 +449,27 @@
* will also cause PeerConnection to ignore all but the first a=ssrc lines
* that form a Plan B stream.
*
- * <p>For users who wish to send multiple audio/video streams and need to stay
- * interoperable with legacy WebRTC implementations, specify PLAN_B.
+ * <p>For users who have to interwork with legacy WebRTC implementations, it
+ * is possible to specify PLAN_B until the code is finally removed
+ * (https://crbug.com/webrtc/13528).
*
- * <p>For users who wish to send multiple audio/video streams and/or wish to
- * use the new RtpTransceiver API, specify UNIFIED_PLAN.
+ * <p>The default SdpSemantics value is about to change to UNIFIED_PLAN.
+ * During a short transition period, NOT_SPECIFIED is used to ensure clients
+ * that don't set SdpSemantics are aware of the change by CHECK-crashing.
+ * TODO(https://crbug.com/webrtc/11121): When the default has changed to
+ * UNIFIED_PLAN, delete NOT_SPECIFIED.
*/
- public enum SdpSemantics { PLAN_B, UNIFIED_PLAN }
+ public enum SdpSemantics {
+ // TODO(https://crbug.com/webrtc/13528): Remove support for PLAN_B.
+ @Deprecated PLAN_B,
+ UNIFIED_PLAN,
+ // The default SdpSemantics value is about to change to UNIFIED_PLAN. During
+ // a short transition period, NOT_SPECIFIED is used to ensure clients that
+ // don't set SdpSemantics are aware of the change by CHECK-crashing.
+ // TODO(https://crbug.com/webrtc/11121): When the default has changed to
+ // kUnifiedPlan, delete kNotSpecified.
+ NOT_SPECIFIED
+ }
/** Java version of PeerConnectionInterface.RTCConfiguration */
// TODO(qingsi): Resolve the naming inconsistency of fields with/without units.
@@ -611,7 +626,7 @@
screencastMinBitrate = null;
combinedAudioVideoBwe = null;
networkPreference = AdapterType.UNKNOWN;
- sdpSemantics = SdpSemantics.PLAN_B;
+ sdpSemantics = SdpSemantics.NOT_SPECIFIED;
activeResetSrtpParams = false;
cryptoOptions = null;
turnLoggingId = null;
diff --git a/sdk/android/api/org/webrtc/PeerConnectionFactory.java b/sdk/android/api/org/webrtc/PeerConnectionFactory.java
index 2b33c6c..e9cf6d3 100644
--- a/sdk/android/api/org/webrtc/PeerConnectionFactory.java
+++ b/sdk/android/api/org/webrtc/PeerConnectionFactory.java
@@ -404,6 +404,7 @@
public PeerConnection createPeerConnection(List<PeerConnection.IceServer> iceServers,
MediaConstraints constraints, PeerConnection.Observer observer) {
PeerConnection.RTCConfiguration rtcConfig = new PeerConnection.RTCConfiguration(iceServers);
+ rtcConfig.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN;
return createPeerConnection(rtcConfig, constraints, observer);
}
@@ -411,6 +412,7 @@
public PeerConnection createPeerConnection(
List<PeerConnection.IceServer> iceServers, PeerConnection.Observer observer) {
PeerConnection.RTCConfiguration rtcConfig = new PeerConnection.RTCConfiguration(iceServers);
+ rtcConfig.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN;
return createPeerConnection(rtcConfig, observer);
}
diff --git a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionEndToEndTest.java b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionEndToEndTest.java
index d9799ab..588e647 100644
--- a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionEndToEndTest.java
+++ b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionEndToEndTest.java
@@ -27,6 +27,7 @@
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.List;
@@ -642,6 +643,12 @@
private static WeakReference<MediaStream> addTracksToPC(PeerConnectionFactory factory,
PeerConnection pc, VideoSource videoSource, String streamLabel, String videoTrackId,
String audioTrackId, VideoSink videoSink) {
+ return addTracksToPC(factory, pc, videoSource, streamLabel, videoTrackId, audioTrackId,
+ videoSink, /*useAddStream=*/false);
+ }
+ private static WeakReference<MediaStream> addTracksToPC(PeerConnectionFactory factory,
+ PeerConnection pc, VideoSource videoSource, String streamLabel, String videoTrackId,
+ String audioTrackId, VideoSink videoSink, boolean useAddStream) {
MediaStream lMS = factory.createLocalMediaStream(streamLabel);
VideoTrack videoTrack = factory.createVideoTrack(videoTrackId, videoSource);
assertNotNull(videoTrack);
@@ -653,7 +660,20 @@
lMS.addTrack(videoTrack);
lMS.addTrack(
factory.createAudioTrack(audioTrackId, factory.createAudioSource(new MediaConstraints())));
- pc.addStream(lMS);
+ if (!useAddStream) {
+ // In Unified Plan, addTrack() is the preferred way of adding tracks.
+ for (AudioTrack track : lMS.audioTracks) {
+ pc.addTrack(track, Collections.singletonList(lMS.getId()));
+ }
+ for (VideoTrack track : lMS.videoTracks) {
+ pc.addTrack(track, Collections.singletonList(lMS.getId()));
+ }
+ } else {
+ // Only in Plan B is addStream() supported. Used by a legacy test not yet
+ // updated to Unified Plan.
+ // TODO(https://crbug.com/webrtc/13528): Remove use of addStream().
+ pc.addStream(lMS);
+ }
return new WeakReference<MediaStream>(lMS);
}
@@ -680,6 +700,7 @@
.createIceServer());
PeerConnection.RTCConfiguration rtcConfig = new PeerConnection.RTCConfiguration(iceServers);
+ rtcConfig.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN;
ObserverExpectations offeringExpectations = new ObserverExpectations("PCTest:offerer");
PeerConnection offeringPC = factory.createPeerConnection(rtcConfig, offeringExpectations);
@@ -946,6 +967,7 @@
.createIceServer());
PeerConnection.RTCConfiguration rtcConfig = new PeerConnection.RTCConfiguration(iceServers);
+ rtcConfig.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN;
ObserverExpectations offeringExpectations = new ObserverExpectations("PCTest:offerer");
PeerConnection offeringPC = factory.createPeerConnection(rtcConfig, offeringExpectations);
@@ -1107,7 +1129,8 @@
PeerConnectionFactory.builder().setOptions(options).createPeerConnectionFactory();
PeerConnection.RTCConfiguration rtcConfig =
- new PeerConnection.RTCConfiguration(Arrays.asList());
+ new PeerConnection.RTCConfiguration(Collections.emptyList());
+ rtcConfig.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN;
// NONE would prevent any candidate being signaled to the PC.
rtcConfig.iceTransportsType = PeerConnection.IceTransportsType.NONE;
// We must have the continual gathering enabled to allow the surfacing of candidates on the ICE
@@ -1173,6 +1196,8 @@
PeerConnection.IceServer.builder("stun:stun.l.google.com:19302").createIceServer());
PeerConnection.RTCConfiguration rtcConfig = new PeerConnection.RTCConfiguration(iceServers);
+ // TODO(https://crbug.com/webrtc/13528): Update test not to use Plan B.
+ rtcConfig.sdpSemantics = PeerConnection.SdpSemantics.PLAN_B;
ObserverExpectations offeringExpectations = new ObserverExpectations("PCTest:offerer");
PeerConnection offeringPC = factory.createPeerConnection(rtcConfig, offeringExpectations);
@@ -1198,7 +1223,8 @@
offeringExpectations.expectRenegotiationNeeded();
WeakReference<MediaStream> oLMS =
addTracksToPC(factory, offeringPC, videoSource, "offeredMediaStream", "offeredVideoTrack",
- "offeredAudioTrack", new ExpectedResolutionSetter(answeringExpectations));
+ "offeredAudioTrack", new ExpectedResolutionSetter(answeringExpectations),
+ /*useAddStream=*/true);
offeringExpectations.expectAddTrack(2);
answeringExpectations.expectAddTrack(2);
@@ -1232,7 +1258,8 @@
answeringExpectations.expectRenegotiationNeeded();
WeakReference<MediaStream> aLMS = addTracksToPC(factory, answeringPC, videoSource,
"answeredMediaStream", "answeredVideoTrack", "answeredAudioTrack",
- new ExpectedResolutionSetter(offeringExpectations));
+ new ExpectedResolutionSetter(offeringExpectations),
+ /*useAddStream=*/true);
// Create answer.
sdpLatch = new SdpObserverLatch();
@@ -1376,8 +1403,10 @@
.setVideoDecoderFactory(new SoftwareVideoDecoderFactory())
.createPeerConnectionFactory();
- // This test is fine with no ICE servers.
- List<PeerConnection.IceServer> iceServers = new ArrayList<>();
+ // TODO(https://crbug.com/webrtc/13528): Update test not to use Plan B.
+ PeerConnection.RTCConfiguration planBConfig =
+ new PeerConnection.RTCConfiguration(Collections.emptyList());
+ planBConfig.sdpSemantics = PeerConnection.SdpSemantics.PLAN_B;
// Use OfferToReceiveAudio/Video to ensure every offer has an audio and
// video m= section. Simplifies the test because it means we don't have to
@@ -1391,11 +1420,11 @@
// This PeerConnection will only be used to generate offers.
ObserverExpectations offeringExpectations = new ObserverExpectations("offerer");
- PeerConnection offeringPC = factory.createPeerConnection(iceServers, offeringExpectations);
+ PeerConnection offeringPC = factory.createPeerConnection(planBConfig, offeringExpectations);
assertNotNull(offeringPC);
ObserverExpectations expectations = new ObserverExpectations("PC under test");
- PeerConnection pcUnderTest = factory.createPeerConnection(iceServers, expectations);
+ PeerConnection pcUnderTest = factory.createPeerConnection(planBConfig, expectations);
assertNotNull(pcUnderTest);
// Add offerer media stream with just an audio track.
@@ -1403,9 +1432,9 @@
AudioTrack localAudioTrack =
factory.createAudioTrack("audio", factory.createAudioSource(new MediaConstraints()));
localStream.addTrack(localAudioTrack);
- // TODO(deadbeef): Use addTrack once that's available.
offeringExpectations.expectRenegotiationNeeded();
- offeringPC.addStream(localStream);
+ RtpSender audioSender =
+ offeringPC.addTrack(localAudioTrack, Collections.singletonList(localStream.getId()));
// Create offer.
SdpObserverLatch sdpLatch = new SdpObserverLatch();
offeringPC.createOffer(sdpLatch, offerConstraints);
@@ -1435,6 +1464,7 @@
VideoTrack videoTrack = factory.createVideoTrack("video", videoSource);
offeringExpectations.expectRenegotiationNeeded();
localStream.addTrack(videoTrack);
+ offeringPC.addTrack(videoTrack, Collections.singletonList(localStream.getId()));
// ... and create an updated offer.
sdpLatch = new SdpObserverLatch();
offeringPC.createOffer(sdpLatch, offerConstraints);
@@ -1453,6 +1483,7 @@
offeringExpectations.expectRenegotiationNeeded();
localStream.removeTrack(localAudioTrack);
localAudioTrack.dispose();
+ offeringPC.removeTrack(audioSender);
sdpLatch = new SdpObserverLatch();
offeringPC.createOffer(sdpLatch, offerConstraints);
assertTrue(sdpLatch.await());
@@ -1492,7 +1523,8 @@
@SmallTest
public void testRollback() throws Exception {
PeerConnectionFactory factory = PeerConnectionFactory.builder().createPeerConnectionFactory();
- PeerConnection.RTCConfiguration config = new PeerConnection.RTCConfiguration(Arrays.asList());
+ PeerConnection.RTCConfiguration config =
+ new PeerConnection.RTCConfiguration(Collections.emptyList());
config.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN;
ObserverExpectations offeringExpectations = new ObserverExpectations("PCTest:offerer");
diff --git a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
index 8478fe5..7d19143 100644
--- a/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
+++ b/sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java
@@ -165,6 +165,7 @@
.setPassword("fakePassword")
.createIceServer());
PeerConnection.RTCConfiguration config = new PeerConnection.RTCConfiguration(iceServers);
+ config.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN;
// Test configuration options.
config.continualGatheringPolicy = PeerConnection.ContinualGatheringPolicy.GATHER_CONTINUALLY;
@@ -179,6 +180,7 @@
public void testCreationWithCertificate() throws Exception {
PeerConnectionFactory factory = PeerConnectionFactory.builder().createPeerConnectionFactory();
PeerConnection.RTCConfiguration config = new PeerConnection.RTCConfiguration(Arrays.asList());
+ config.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN;
// Test certificate.
RtcCertificatePem originalCert = RtcCertificatePem.generateCertificate();
@@ -197,6 +199,7 @@
public void testCreationWithCryptoOptions() throws Exception {
PeerConnectionFactory factory = PeerConnectionFactory.builder().createPeerConnectionFactory();
PeerConnection.RTCConfiguration config = new PeerConnection.RTCConfiguration(Arrays.asList());
+ config.sdpSemantics = PeerConnection.SdpSemantics.UNIFIED_PLAN;
assertNull(config.cryptoOptions);
diff --git a/sdk/android/src/jni/pc/peer_connection.cc b/sdk/android/src/jni/pc/peer_connection.cc
index ee3c35c..6bcaef4 100644
--- a/sdk/android/src/jni/pc/peer_connection.cc
+++ b/sdk/android/src/jni/pc/peer_connection.cc
@@ -118,6 +118,9 @@
if (enum_name == "UNIFIED_PLAN")
return SdpSemantics::kUnifiedPlan;
+ if (enum_name == "NOT_SPECIFIED")
+ return SdpSemantics::kNotSpecified;
+
RTC_DCHECK_NOTREACHED();
return SdpSemantics::kPlanB_DEPRECATED;
}
