Skip to content

Commit

Permalink
Use a HashMap like behaviour in parsing SDP.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
claincly authored and icbaker committed Jul 16, 2021
1 parent 60bbe64 commit e2040a5
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
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 {
Expand Down Expand Up @@ -106,7 +107,7 @@ public static final class Builder {
private final int port;
private final String transportProtocol;
private final int payloadType;
private final ImmutableMap.Builder<String, String> attributesBuilder;
private final HashMap<String, String> attributes;

private int bitrate;
@Nullable private String mediaTitle;
Expand All @@ -126,7 +127,7 @@ public Builder(String mediaType, int port, String transportProtocol, int payload
this.port = port;
this.transportProtocol = transportProtocol;
this.payloadType = payloadType;
attributesBuilder = new ImmutableMap.Builder<>();
attributes = new HashMap<>();
bitrate = Format.NO_VALUE;
}

Expand Down Expand Up @@ -184,7 +185,7 @@ public Builder setKey(String key) {
* @return This builder.
*/
public Builder addAttribute(String attributeName, String attributeValue) {
attributesBuilder.put(attributeName, attributeValue);
attributes.put(attributeName, attributeValue);
return this;
}

Expand All @@ -195,13 +196,12 @@ public Builder addAttribute(String attributeName, String attributeValue) {
* cannot be parsed.
*/
public MediaDescription build() {
ImmutableMap<String, String> 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
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.
Expand All @@ -35,7 +36,7 @@

/** Builder class for {@link SessionDescription}. */
public static final class Builder {
private final ImmutableMap.Builder<String, String> attributesBuilder;
private final HashMap<String, String> attributes;
private final ImmutableList.Builder<MediaDescription> mediaDescriptionListBuilder;
private int bitrate;
@Nullable private String sessionName;
Expand All @@ -50,7 +51,7 @@ public static final class Builder {

/** Creates a new instance. */
public Builder() {
attributesBuilder = new ImmutableMap.Builder<>();
attributes = new HashMap<>();
mediaDescriptionListBuilder = new ImmutableList.Builder<>();
bitrate = Format.NO_VALUE;
}
Expand Down Expand Up @@ -179,7 +180,7 @@ public Builder setPhoneNumber(String phoneNumber) {
* @return This builder.
*/
public Builder addAttribute(String attributeName, String attributeValue) {
attributesBuilder.put(attributeName, attributeValue);
attributes.put(attributeName, attributeValue);
return this;
}

Expand Down Expand Up @@ -259,7 +260,7 @@ public SessionDescription build() {

/** 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@

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;

Expand Down Expand Up @@ -151,32 +150,45 @@ public void parse_sdpString2_succeeds() throws Exception {
}

@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
Expand Down

0 comments on commit e2040a5

Please sign in to comment.