From e2040a58935d4463a53fdaf4c952109cefe37cc2 Mon Sep 17 00:00:00 2001 From: claincly Date: Mon, 21 Jun 2021 12:17:45 +0100 Subject: [PATCH] Use a HashMap like behaviour in parsing SDP. Some server will wrongly insert duplicated attributes. We used to treat this as a unrecoverable error, but it is better to treat the duplicated attributes in an "over-writable" fashion like HashMaps. Issue: #9080, Issue: #9014 PiperOrigin-RevId: 380547079 --- .../source/rtsp/MediaDescription.java | 10 +++---- .../source/rtsp/SessionDescription.java | 9 ++++--- .../source/rtsp/SessionDescriptionTest.java | 26 ++++++++++++++----- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/library/rtsp/src/main/java/com/google/android/exoplayer2/source/rtsp/MediaDescription.java b/library/rtsp/src/main/java/com/google/android/exoplayer2/source/rtsp/MediaDescription.java index e4467d395e..6bda38e8d4 100644 --- a/library/rtsp/src/main/java/com/google/android/exoplayer2/source/rtsp/MediaDescription.java +++ b/library/rtsp/src/main/java/com/google/android/exoplayer2/source/rtsp/MediaDescription.java @@ -33,6 +33,7 @@ import com.google.common.collect.ImmutableMap; import java.lang.annotation.Documented; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.HashMap; /** Represents one media description section in a SDP message. */ /* package */ final class MediaDescription { @@ -106,7 +107,7 @@ import java.lang.annotation.RetentionPolicy; private final int port; private final String transportProtocol; private final int payloadType; - private final ImmutableMap.Builder attributesBuilder; + private final HashMap attributes; private int bitrate; @Nullable private String mediaTitle; @@ -126,7 +127,7 @@ import java.lang.annotation.RetentionPolicy; this.port = port; this.transportProtocol = transportProtocol; this.payloadType = payloadType; - attributesBuilder = new ImmutableMap.Builder<>(); + attributes = new HashMap<>(); bitrate = Format.NO_VALUE; } @@ -184,7 +185,7 @@ import java.lang.annotation.RetentionPolicy; * @return This builder. */ public Builder addAttribute(String attributeName, String attributeValue) { - attributesBuilder.put(attributeName, attributeValue); + attributes.put(attributeName, attributeValue); return this; } @@ -195,13 +196,12 @@ import java.lang.annotation.RetentionPolicy; * cannot be parsed. */ public MediaDescription build() { - ImmutableMap attributes = attributesBuilder.build(); try { // rtpmap attribute is mandatory in RTSP (RFC2326 Section C.1.3). checkState(attributes.containsKey(ATTR_RTPMAP)); RtpMapAttribute rtpMapAttribute = RtpMapAttribute.parse(castNonNull(attributes.get(ATTR_RTPMAP))); - return new MediaDescription(this, attributes, rtpMapAttribute); + return new MediaDescription(this, ImmutableMap.copyOf(attributes), rtpMapAttribute); } catch (ParserException e) { throw new IllegalStateException(e); } diff --git a/library/rtsp/src/main/java/com/google/android/exoplayer2/source/rtsp/SessionDescription.java b/library/rtsp/src/main/java/com/google/android/exoplayer2/source/rtsp/SessionDescription.java index 4ac138f554..770709b1b1 100644 --- a/library/rtsp/src/main/java/com/google/android/exoplayer2/source/rtsp/SessionDescription.java +++ b/library/rtsp/src/main/java/com/google/android/exoplayer2/source/rtsp/SessionDescription.java @@ -24,6 +24,7 @@ import com.google.android.exoplayer2.Format; import com.google.android.exoplayer2.util.Util; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import java.util.HashMap; /** * Records all the information in a SDP message. @@ -35,7 +36,7 @@ import com.google.common.collect.ImmutableMap; /** Builder class for {@link SessionDescription}. */ public static final class Builder { - private final ImmutableMap.Builder attributesBuilder; + private final HashMap attributes; private final ImmutableList.Builder mediaDescriptionListBuilder; private int bitrate; @Nullable private String sessionName; @@ -50,7 +51,7 @@ import com.google.common.collect.ImmutableMap; /** Creates a new instance. */ public Builder() { - attributesBuilder = new ImmutableMap.Builder<>(); + attributes = new HashMap<>(); mediaDescriptionListBuilder = new ImmutableList.Builder<>(); bitrate = Format.NO_VALUE; } @@ -179,7 +180,7 @@ import com.google.common.collect.ImmutableMap; * @return This builder. */ public Builder addAttribute(String attributeName, String attributeValue) { - attributesBuilder.put(attributeName, attributeValue); + attributes.put(attributeName, attributeValue); return this; } @@ -259,7 +260,7 @@ import com.google.common.collect.ImmutableMap; /** Creates a new instance. */ private SessionDescription(Builder builder) { - this.attributes = builder.attributesBuilder.build(); + this.attributes = ImmutableMap.copyOf(builder.attributes); this.mediaDescriptionList = builder.mediaDescriptionListBuilder.build(); this.sessionName = castNonNull(builder.sessionName); this.origin = castNonNull(builder.origin); diff --git a/library/rtsp/src/test/java/com/google/android/exoplayer2/source/rtsp/SessionDescriptionTest.java b/library/rtsp/src/test/java/com/google/android/exoplayer2/source/rtsp/SessionDescriptionTest.java index 47cd40e714..d71976148d 100644 --- a/library/rtsp/src/test/java/com/google/android/exoplayer2/source/rtsp/SessionDescriptionTest.java +++ b/library/rtsp/src/test/java/com/google/android/exoplayer2/source/rtsp/SessionDescriptionTest.java @@ -30,7 +30,6 @@ import static org.junit.Assert.assertThrows; import android.net.Uri; import androidx.test.ext.junit.runners.AndroidJUnit4; -import com.google.android.exoplayer2.ParserException; import org.junit.Test; import org.junit.runner.RunWith; @@ -151,32 +150,45 @@ public class SessionDescriptionTest { } @Test - public void parse_sdpStringWithDuplicatedMediaAttribute_throwsParserException() { + public void parse_sdpStringWithDuplicatedMediaAttribute_recordsTheMostRecentValue() + throws Exception { String testMediaSdpInfo = "v=0\r\n" + "o=MNobody 2890844526 2890842807 IN IP4 192.0.2.46\r\n" + "s=SDP Seminar\r\n" + + "t=0 0\r\n" + "i=A Seminar on the session description protocol\r\n" + "m=audio 3456 RTP/AVP 0\r\n" - + "a=control:audio\r\n" + + "a=control:video\r\n" + + "a=rtpmap:97 AC3/44100\r\n" + // Duplicate attribute. + "a=control:audio\r\n"; - assertThrows(ParserException.class, () -> SessionDescriptionParser.parse(testMediaSdpInfo)); + SessionDescription sessionDescription = SessionDescriptionParser.parse(testMediaSdpInfo); + + assertThat(sessionDescription.mediaDescriptionList.get(0).attributes) + .containsEntry(ATTR_CONTROL, "audio"); } @Test - public void parse_sdpStringWithDuplicatedSessionAttribute_throwsParserException() { + public void parse_sdpStringWithDuplicatedSessionAttribute_recordsTheMostRecentValue() + throws Exception { String testMediaSdpInfo = "v=0\r\n" + "o=MNobody 2890844526 2890842807 IN IP4 192.0.2.46\r\n" + "s=SDP Seminar\r\n" + + "t=0 0\r\n" + "a=control:*\r\n" - + "a=control:*\r\n" + // Duplicate attribute. + + "a=control:session1\r\n" + "i=A Seminar on the session description protocol\r\n" + "m=audio 3456 RTP/AVP 0\r\n" + + "a=rtpmap:97 AC3/44100\r\n" + "a=control:audio\r\n"; - assertThrows(ParserException.class, () -> SessionDescriptionParser.parse(testMediaSdpInfo)); + SessionDescription sessionDescription = SessionDescriptionParser.parse(testMediaSdpInfo); + + assertThat(sessionDescription.attributes).containsEntry(ATTR_CONTROL, "session1"); } @Test