From abf10eac16eb62de76b1ae8363916e80b4deec6b Mon Sep 17 00:00:00 2001
From: Scott Lamb <slamb@slamb.org>
Date: Mon, 6 Sep 2021 12:13:26 -0700
Subject: [PATCH] use sdp-types 0.1.3 with more permissive parsing

Fixes #26
Fixes #33
---
 CHANGELOG.md                          |  3 +
 Cargo.lock                            | 19 +++---
 Cargo.toml                            |  2 +-
 src/client/parse.rs                   | 89 +++++++++++++++------------
 src/client/testdata/anpviz_sdp.txt    | 21 +++++++
 src/client/testdata/geovision_sdp.txt | 15 +++++
 6 files changed, 101 insertions(+), 48 deletions(-)
 create mode 100644 src/client/testdata/anpviz_sdp.txt
 create mode 100644 src/client/testdata/geovision_sdp.txt

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5ea129a..5d209ac 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,9 @@
 *   warn when connecting via TCP to a known-broken live555 server version.
 *   improve compatibility with Geovision cameras (work in progress).
 *   UDP fixes.
+*   improve compatibility with cameras with non-compliant SDP, including
+    Anpviz ([#26](https://github.com/scottlamb/retina/issues/26) and
+    Geovision ([#33])(https://github.com/scottlamb/retina/issues/33)).
 
 ## `v0.3.0` (2021-08-31)
 
diff --git a/Cargo.lock b/Cargo.lock
index f1648da..09659ee 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -296,6 +296,12 @@ version = "1.6.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "e78d4f1cc4ae33bbfc157ed5d5a5ef3bc29227303d595861deb238fcec4e9457"
 
+[[package]]
+name = "fallible-iterator"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4443176a9f2c162692bd3d352d745ef9413eec5782a80d8fd6f8a1ac692a07f7"
+
 [[package]]
 name = "form_urlencoded"
 version = "1.0.1"
@@ -991,7 +997,7 @@ dependencies = [
  "rand",
  "rtp-rs",
  "rtsp-types",
- "sdp",
+ "sdp-types",
  "smallvec",
  "structopt",
  "thiserror",
@@ -1061,14 +1067,13 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd"
 
 [[package]]
-name = "sdp"
-version = "0.1.5"
+name = "sdp-types"
+version = "0.1.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "db73ce8329b973830407fb1ba0e51bc32716392281f7757a92f372a1420bb8ec"
+checksum = "ae499f6886cff026ebd8355c8f67a1881cd15f23ce89de4aab13588cf52142dd"
 dependencies = [
- "rand",
- "thiserror",
- "url",
+ "bstr",
+ "fallible-iterator",
 ]
 
 [[package]]
diff --git a/Cargo.toml b/Cargo.toml
index 1d67bde..6c2111e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -25,7 +25,7 @@ pretty-hex = "0.2.1"
 rand = "0.8.3"
 rtp-rs = "0.6.0"
 rtsp-types = "0.0.2"
-sdp = "0.1.4"
+sdp-types = "0.1.3"
 smallvec = { version = "1.6.1", features = ["union"] }
 thiserror = "1.0.25"
 time = "0.1.43"
diff --git a/src/client/parse.rs b/src/client/parse.rs
index 4082de2..cba837c 100644
--- a/src/client/parse.rs
+++ b/src/client/parse.rs
@@ -1,11 +1,11 @@
 // Copyright (C) 2021 Scott Lamb <slamb@slamb.org>
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
-use bytes::{Buf, Bytes};
+use bytes::Bytes;
 use log::debug;
 use pretty_hex::PrettyHex;
-use sdp::media_description::MediaDescription;
-use std::{convert::TryFrom, net::IpAddr, num::NonZeroU16};
+use sdp_types::Media;
+use std::{net::IpAddr, num::NonZeroU16};
 use url::Url;
 
 use super::{Presentation, Stream};
@@ -224,21 +224,16 @@ pub(crate) fn get_cseq(response: &rtsp_types::Response<Bytes>) -> Option<u32> {
 /// Parses a [MediaDescription] to a [Stream].
 /// On failure, returns an error which is expected to be supplemented with
 /// the [MediaDescription] debug string and packed into a `RtspResponseError`.
-fn parse_media(base_url: &Url, media_description: &MediaDescription) -> Result<Stream, String> {
-    let media = media_description.media_name.media.clone();
+fn parse_media(base_url: &Url, media_description: &Media) -> Result<Stream, String> {
+    let media = media_description.media.clone();
 
     // https://tools.ietf.org/html/rfc8866#section-5.14 says "If the <proto>
     // sub-field is "RTP/AVP" or "RTP/SAVP" the <fmt> sub-fields contain RTP
     // payload type numbers."
     // https://www.iana.org/assignments/sdp-parameters/sdp-parameters.xhtml#sdp-parameters-2
-    // shows several other variants, such as "TCP/RTP/AVP". Looking a "RTP" component
+    // shows several other variants, such as "TCP/RTP/AVP". Looking for a "RTP" component
     // seems appropriate.
-    if !media_description
-        .media_name
-        .protos
-        .iter()
-        .any(|p| p == "RTP")
-    {
+    if !media_description.proto.starts_with("RTP/") && !media_description.proto.contains("/RTP/") {
         return Err("Expected RTP-based proto".into());
     }
 
@@ -248,10 +243,10 @@ fn parse_media(base_url: &Url, media_description: &MediaDescription) -> Result<S
     // format for the session." Just use the first until we find a stream
     // where this isn't the right thing to do.
     let rtp_payload_type_str = media_description
-        .media_name
-        .formats
-        .first()
-        .ok_or_else(|| "missing RTP payload type".to_string())?;
+        .fmt
+        .split_ascii_whitespace()
+        .next()
+        .unwrap();
     let rtp_payload_type = u8::from_str_radix(rtp_payload_type_str, 10)
         .map_err(|_| format!("invalid RTP payload type {:?}", rtp_payload_type_str))?;
     if (rtp_payload_type & 0x80) != 0 {
@@ -268,7 +263,7 @@ fn parse_media(base_url: &Url, media_description: &MediaDescription) -> Result<S
     let mut fmtp = None;
     let mut control = None;
     for a in &media_description.attributes {
-        if a.key == "rtpmap" {
+        if a.attribute == "rtpmap" {
             let v = a
                 .value
                 .as_ref()
@@ -287,7 +282,7 @@ fn parse_media(base_url: &Url, media_description: &MediaDescription) -> Result<S
             if rtpmap_payload_type == rtp_payload_type_str {
                 rtpmap = Some(v);
             }
-        } else if a.key == "fmtp" {
+        } else if a.attribute == "fmtp" {
             // Similarly starts with payload-type SP.
             let v = a
                 .value
@@ -299,7 +294,7 @@ fn parse_media(base_url: &Url, media_description: &MediaDescription) -> Result<S
             if fmtp_payload_type == rtp_payload_type_str {
                 fmtp = Some(v);
             }
-        } else if a.key == "control" {
+        } else if a.attribute == "control" {
             control = a
                 .value
                 .as_deref()
@@ -385,24 +380,13 @@ pub(crate) fn parse_describe(
         ));
     }
 
-    let sdp;
-    {
-        let mut cursor = std::io::Cursor::new(&response.body()[..]);
-        sdp =
-            sdp::session_description::SessionDescription::unmarshal(&mut cursor).map_err(|e| {
-                format!(
-                    "Unable to parse SDP: {}\n\n{:#?}",
-                    e,
-                    response.body().hex_dump()
-                )
-            })?;
-        if cursor.has_remaining() {
-            return Err(format!(
-                "garbage after sdp: {:?}",
-                &response.body()[usize::try_from(cursor.position()).unwrap()..]
-            ));
-        }
-    }
+    let sdp = sdp_types::Session::parse(&response.body()[..]).map_err(|e| {
+        format!(
+            "Unable to parse SDP: {}\n\n{:#?}",
+            e,
+            response.body().hex_dump()
+        )
+    })?;
 
     // https://tools.ietf.org/html/rfc2326#appendix-C.1.1
     let base_url = response
@@ -419,21 +403,21 @@ pub(crate) fn parse_describe(
     let mut control = None;
     let mut tool = None;
     for a in &sdp.attributes {
-        if a.key == "control" {
+        if a.attribute == "control" {
             control = a
                 .value
                 .as_deref()
                 .map(|c| join_control(&base_url, c))
                 .transpose()?;
             break;
-        } else if a.key == "tool" {
+        } else if a.attribute == "tool" {
             tool = a.value.as_deref().map(Into::into);
         }
     }
     let control = control.unwrap_or(request_url);
 
     let streams = sdp
-        .media_descriptions
+        .medias
         .iter()
         .enumerate()
         .map(|(i, m)| {
@@ -616,6 +600,7 @@ pub(crate) fn parse_play(
 mod tests {
     use std::num::NonZeroU16;
 
+    use bytes::Bytes;
     use url::Url;
 
     use crate::{client::StreamStateInit, codec::Parameters};
@@ -631,6 +616,30 @@ mod tests {
         super::parse_describe(url, &response(raw_response))
     }
 
+    #[test]
+    fn anvpiz_sdp() {
+        let url = Url::parse("rtsp://127.0.0.1/").unwrap();
+        let response =
+            rtsp_types::Response::builder(rtsp_types::Version::V1_0, rtsp_types::StatusCode::Ok)
+                .header(rtsp_types::headers::CONTENT_TYPE, "application/sdp")
+                .build(Bytes::from_static(include_bytes!(
+                    "testdata/anpviz_sdp.txt"
+                )));
+        super::parse_describe(url, &response).unwrap();
+    }
+
+    #[test]
+    fn geovision_sdp() {
+        let url = Url::parse("rtsp://127.0.0.1/").unwrap();
+        let response =
+            rtsp_types::Response::builder(rtsp_types::Version::V1_0, rtsp_types::StatusCode::Ok)
+                .header(rtsp_types::headers::CONTENT_TYPE, "application/sdp")
+                .build(Bytes::from_static(include_bytes!(
+                    "testdata/geovision_sdp.txt"
+                )));
+        super::parse_describe(url, &response).unwrap();
+    }
+
     #[test]
     fn dahua_h264_aac_onvif() {
         // DESCRIBE.
diff --git a/src/client/testdata/anpviz_sdp.txt b/src/client/testdata/anpviz_sdp.txt
new file mode 100644
index 0000000..95a0d12
--- /dev/null
+++ b/src/client/testdata/anpviz_sdp.txt
@@ -0,0 +1,21 @@
+v=0
+o=- 1109162014219182 1109162014219192 IN IP4 x.y.z.w
+s=RTSP/RTP stream from anjvision ipcamera
+e=NONE
+c=IN IP4 0.0.0.0
+a=tool:LIVE555 Streaming Media v2011.05.25 CHAM.LI@ANJVISION.COM
+t=0 0
+a=range:npt=0-
+a=control:*
+m=video 0 RTP/AVP 96
+a=rtpmap:96 H264/90000
+a=control:trackID=1
+a=fmtp:96 profile-level-id=4D401F;packetization-mode=0;sprop-parameter-sets=Z01AH5WgLASabAQ=,aO48gA==;config=00000001674d401f95a02c049a6c040000000168ee3c800000000106f02c0445c6f5000620ebc2f3f7639e48250bfcb561bb2b85dda6fe5f06cc8b887b6a915f5aa3bebfffffffffff7380
+a=x-dimensions: 704, 576
+a=x-framerate: 12
+m=audio 0 RTP/AVP 0
+a=rtpmap:0 MPEG4-GENERIC/16000/2
+a=fmtp:0 config=1408
+a=control:trackID=2
+a=Media_header:MEDIAINFO=494D4B48010100000400010010710110401F000000FA000000000000000000000000000000000000;
+a=appversion:1.0
\ No newline at end of file
diff --git a/src/client/testdata/geovision_sdp.txt b/src/client/testdata/geovision_sdp.txt
new file mode 100644
index 0000000..ef96e4d
--- /dev/null
+++ b/src/client/testdata/geovision_sdp.txt
@@ -0,0 +1,15 @@
+v=0
+o=- 1001 1 IN IP4 192.168.5.237
+s=VCP IPC Realtime stream
+m=video 0 RTP/AVP 105
+c=IN IP4 192.168.5.237
+a=control:rtsp://192.168.5.237/media/video1/video
+a=rtpmap:105 H264/90000
+a=fmtp:105 profile-level-id=4d4032; packetization-mode=1; sprop-parameter-sets=Z01AMpWgCoAwfiZuAgICgAAB9AAAdTBC,aO48gA==
+a=recvonly
+m=application 0 RTP/AVP 107
+c=IN IP4 192.168.5.237
+a=control:rtsp://192.168.5.237/media/video1/metadata
+a=rtpmap:107 vnd.onvif.metadata/90000
+a=fmtp:107 DecoderTag=h3c-v3 RTCP=0
+a=recvonly