Bug 1438289: Implemented RsdparsaSdpMediaSection::AddCodec and ::ClearCodecs. r=dminor draft
authorJohannes Willbold <j.willbold@mozilla.com>
Mon, 04 Jun 2018 17:23:42 -0700
changeset 805954 f075c80b1723c794873fb53a77471820ed34569f
parent 805786 e0595117ff5bda3a63a72ad7b3b8754fec4fb4f0
push id112821
push userbmo:johannes.willbold@rub.de
push dateFri, 08 Jun 2018 19:17:46 +0000
reviewersdminor
bugs1438289
milestone62.0a1
Bug 1438289: Implemented RsdparsaSdpMediaSection::AddCodec and ::ClearCodecs. r=dminor Implemented RsdparsaSdpMediaSection::ClearCodecs and RsdparsaSdpMediaSection::AddCodec Added sdp_media_clear_codecs Added corresponding C++/Rust glue code Added rust trait to convert StringView to rust std::String Added testcase CheckAddCodec and CheckClearCodec MozReview-Commit-ID: 5pxl8hR9iz2
media/webrtc/signaling/gtest/sdp_unittests.cpp
media/webrtc/signaling/src/sdp/RsdparsaSdpInc.h
media/webrtc/signaling/src/sdp/RsdparsaSdpMediaSection.cpp
media/webrtc/signaling/src/sdp/rsdparsa/src/media_type.rs
media/webrtc/signaling/src/sdp/rsdparsa_capi/src/media_section.rs
media/webrtc/signaling/src/sdp/rsdparsa_capi/src/types.rs
--- a/media/webrtc/signaling/gtest/sdp_unittests.cpp
+++ b/media/webrtc/signaling/gtest/sdp_unittests.cpp
@@ -3920,16 +3920,77 @@ TEST_P(NewSdpTest, CheckSetPort) {
    constexpr unsigned int expectedParesPort = 56436;
    unsigned int currentPort = mSdp->GetMediaSection(0).GetPort();
    ASSERT_EQ(expectedParesPort, currentPort);
 
    mSdp->GetMediaSection(0).SetPort(currentPort+1);
    ASSERT_EQ(currentPort+1,mSdp->GetMediaSection(0).GetPort());
 }
 
+TEST_P(NewSdpTest, CheckAddCodec) {
+  // Parse any valid sdp with a media section
+  ParseSdp("v=0" CRLF
+           "o=- 4294967296 2 IN IP4 127.0.0.1" CRLF
+           "s=SIP Call" CRLF
+           "c=IN IP4 198.51.100.7" CRLF
+           "b=CT:5000" CRLF
+           "t=0 0" CRLF
+           "m=video 56436 RTP/SAVPF 120" CRLF
+           "a=rtpmap:120 VP8/90000" CRLF
+           "a=sendonly" CRLF);
+
+   ASSERT_TRUE(!!mSdp) << "Parse failed: " << GetParseErrors();
+   ASSERT_EQ(1U,mSdp->GetMediaSectionCount());
+
+   ASSERT_EQ(1U,mSdp->GetMediaSection(0).GetFormats().size());
+   ASSERT_EQ(1U,mSdp->GetMediaSection(0).GetAttributeList().GetRtpmap().mRtpmaps.size());
+
+   mSdp->GetMediaSection(0).AddCodec("110","opus",48000,2);
+
+   ASSERT_EQ(2U,mSdp->GetMediaSection(0).GetFormats().size());
+   const auto& rtpmaps = mSdp->GetMediaSection(0).GetAttributeList().GetRtpmap();
+   ASSERT_EQ(2U,rtpmaps.mRtpmaps.size());
+
+   ASSERT_TRUE(rtpmaps.HasEntry("120"));
+   ASSERT_TRUE(rtpmaps.HasEntry("110"));
+   const auto aRtpmap = rtpmaps.GetEntry("110");
+   ASSERT_EQ(aRtpmap.pt,"110");
+   ASSERT_EQ(aRtpmap.codec,SdpRtpmapAttributeList::kOpus);
+   ASSERT_EQ(aRtpmap.name,"opus");
+   ASSERT_EQ(aRtpmap.clock,48000U);
+   ASSERT_EQ(aRtpmap.channels,2U);
+}
+
+TEST_P(NewSdpTest, CheckClearCodecs) {
+  // Parse any valid sdp with a media section
+  ParseSdp("v=0" CRLF
+           "o=- 4294967296 2 IN IP4 127.0.0.1" CRLF
+           "s=SIP Call" CRLF
+           "c=IN IP4 198.51.100.7" CRLF
+           "b=CT:5000" CRLF
+           "t=0 0" CRLF
+           "m=video 56436 RTP/SAVPF 120 110" CRLF
+           "a=rtpmap:120 VP8/90000" CRLF
+           "a=sendonly" CRLF
+           "a=rtpmap:110 opus/48000/2" CRLF);
+
+     ASSERT_TRUE(!!mSdp) << "Parse failed: " << GetParseErrors();
+     ASSERT_EQ(1U,mSdp->GetMediaSectionCount());
+
+     ASSERT_EQ(2U,mSdp->GetMediaSection(0).GetFormats().size());
+     ASSERT_EQ(2U,mSdp->GetMediaSection(0).GetAttributeList().
+                  GetRtpmap().mRtpmaps.size());
+
+     mSdp->GetMediaSection(0).ClearCodecs();
+
+     ASSERT_EQ(0U,mSdp->GetMediaSection(0).GetFormats().size());
+     ASSERT_FALSE(mSdp->GetMediaSection(0).GetAttributeList().
+                  HasAttribute(SdpAttribute::kRtpmapAttribute));
+}
+
 TEST(NewSdpTestNoFixture, CheckAttributeTypeSerialize) {
   for (auto a = static_cast<size_t>(SdpAttribute::kFirstAttribute);
        a <= static_cast<size_t>(SdpAttribute::kLastAttribute);
        ++a) {
 
     SdpAttribute::AttributeType type =
       static_cast<SdpAttribute::AttributeType>(a);
 
--- a/media/webrtc/signaling/src/sdp/RsdparsaSdpInc.h
+++ b/media/webrtc/signaling/src/sdp/RsdparsaSdpInc.h
@@ -27,17 +27,17 @@ enum class RustSdpAddrType {
 };
 
 struct RustIpAddr {
   RustSdpAddrType addrType;
   char unicastAddr[50];
 };
 
 struct StringView {
-  char* buf;
+  const char* buf;
   size_t len;
 };
 
 struct RustSdpConnection {
   RustIpAddr addr;
   uint8_t ttl;
   uint64_t amount;
 };
@@ -218,16 +218,21 @@ uint32_t sdp_get_media_bandwidth(const R
                                  const char* aBandwidthType);
 bool sdp_media_has_connection(const RustMediaSection* aMediaSec);
 nsresult sdp_get_media_connection(const RustMediaSection* aMediaSec,
                                   RustSdpConnection* ret);
 
 RustAttributeList*
 sdp_get_media_attribute_list(const RustMediaSection* aMediaSec);
 
+nsresult sdp_media_add_codec(const RustMediaSection* aMediaSec,
+                             uint8_t aPT, StringView aCodecName,
+                             uint32_t aClockrate, uint16_t channels);
+void sdp_media_clear_codecs(const RustMediaSection* aMediaSec);
+
 nsresult sdp_get_iceufrag(const RustAttributeList* aList, StringView* ret);
 nsresult sdp_get_icepwd(const RustAttributeList* aList, StringView* ret);
 nsresult sdp_get_identity(const RustAttributeList* aList, StringView* ret);
 nsresult sdp_get_iceoptions(const RustAttributeList* aList, StringVec** ret);
 
 size_t sdp_get_fingerprint_count(const RustAttributeList* aList);
 void sdp_get_fingerprints(const RustAttributeList* aList, size_t listSize,
                           RustSdpAttributeFingerprint* ret);
--- a/media/webrtc/signaling/src/sdp/RsdparsaSdpMediaSection.cpp
+++ b/media/webrtc/signaling/src/sdp/RsdparsaSdpMediaSection.cpp
@@ -133,24 +133,62 @@ RsdparsaSdpMediaSection::GetDirectionAtt
   return SdpDirectionAttribute(mAttributeList->GetDirection());
 }
 
 void
 RsdparsaSdpMediaSection::AddCodec(const std::string& pt,
                                   const std::string& name,
                                   uint32_t clockrate, uint16_t channels)
 {
-  //TODO: see Bug 1438289
+  StringView rustName{name.c_str(),name.size()};
+
+  // call the rust interface
+  auto nr = sdp_media_add_codec(mSection, std::stoul(pt),rustName,clockrate,channels);
+
+  if (NS_SUCCEEDED(nr)) {
+    // If the rust call was successful, adjust the shadow C++ structures
+    mFormats.push_back(pt);
+
+    // Add a rtpmap in mAttributeList
+    SdpRtpmapAttributeList* rtpmap = new SdpRtpmapAttributeList();
+    if (mAttributeList->HasAttribute(SdpAttribute::kRtpmapAttribute)) {
+      const SdpRtpmapAttributeList& old = mAttributeList->GetRtpmap();
+      for (auto it = old.mRtpmaps.begin(); it != old.mRtpmaps.end(); ++it) {
+        rtpmap->mRtpmaps.push_back(*it);
+      }
+    }
+
+    SdpRtpmapAttributeList::CodecType codec = SdpRtpmapAttributeList::kOtherCodec;
+    if (name == "opus") {
+      codec = SdpRtpmapAttributeList::kOpus;
+    } else if (name == "VP8") {
+      codec = SdpRtpmapAttributeList::kVP8;
+    } else if (name == "VP9") {
+      codec = SdpRtpmapAttributeList::kVP9;
+    } else if (name == "H264") {
+      codec = SdpRtpmapAttributeList::kH264;
+    }
+
+    rtpmap->PushEntry(pt, codec, name, clockrate, channels);
+    mAttributeList->SetAttribute(rtpmap);
+  }
+
 }
 
 void
 RsdparsaSdpMediaSection::ClearCodecs()
 {
+  // Clear the codecs in rust
+  sdp_media_clear_codecs(mSection);
 
-  //TODO: see Bug 1438289
+  mFormats.clear();
+  mAttributeList->RemoveAttribute(SdpAttribute::kRtpmapAttribute);
+  mAttributeList->RemoveAttribute(SdpAttribute::kFmtpAttribute);
+  mAttributeList->RemoveAttribute(SdpAttribute::kSctpmapAttribute);
+  mAttributeList->RemoveAttribute(SdpAttribute::kRtcpFbAttribute);
 }
 
 void
 RsdparsaSdpMediaSection::AddDataChannel(const std::string& name, uint16_t port,
                                         uint16_t streams, uint32_t message_size)
 {
   //TODO: See 1438290
 }
--- a/media/webrtc/signaling/src/sdp/rsdparsa/src/media_type.rs
+++ b/media/webrtc/signaling/src/sdp/rsdparsa/src/media_type.rs
@@ -1,11 +1,11 @@
 use std::fmt;
 use {SdpType, SdpLine, SdpBandwidth, SdpConnection};
-use attribute_type::{SdpAttribute, SdpAttributeType};
+use attribute_type::{SdpAttribute, SdpAttributeType, SdpAttributeRtpmap};
 use error::{SdpParserError, SdpParserInternalError};
 
 #[derive(Clone)]
 pub struct SdpMediaLine {
     pub media: SdpMediaValue,
     pub port: u32,
     pub port_count: u32,
     pub proto: SdpProtocolValue,
@@ -139,16 +139,43 @@ impl SdpMedia {
                                                                attr)));
         }
         Ok(self.attribute.push(attr.clone()))
     }
 
     pub fn get_attribute(&self, t: SdpAttributeType) -> Option<&SdpAttribute> {
         self.attribute.iter().filter(|a| SdpAttributeType::from(*a) == t).next()
     }
+    
+    pub fn remove_codecs(&mut self) {
+        match self.media.formats{
+            SdpFormatList::Integers(_) => self.media.formats = SdpFormatList::Integers(Vec::new()),
+            SdpFormatList::Strings(_) => self.media.formats = SdpFormatList::Strings(Vec::new()),
+        }
+
+        self.attribute.retain({|x|
+            match x {
+                &SdpAttribute::Rtpmap(_) |
+                &SdpAttribute::Fmtp(_) |
+                &SdpAttribute::Rtcpfb(_) |
+                &SdpAttribute::Sctpmap(_) => false,
+                _ => true
+            }
+        });
+    }
+
+    pub fn add_codec(&mut self, rtpmap: SdpAttributeRtpmap) -> Result<(),SdpParserInternalError> {
+          match self.media.formats {
+             SdpFormatList::Integers(ref mut x) => x.push(rtpmap.payload_type as u32),
+             SdpFormatList::Strings(ref mut x) => x.push(rtpmap.payload_type.to_string()),
+         }
+
+        self.add_attribute(&SdpAttribute::Rtpmap(rtpmap))?;
+        Ok(())
+    }    
 
     pub fn has_connection(&self) -> bool {
         self.connection.is_some()
     }
 
     pub fn get_connection(&self) -> &Option<SdpConnection> {
         &self.connection
     }
--- a/media/webrtc/signaling/src/sdp/rsdparsa_capi/src/media_section.rs
+++ b/media/webrtc/signaling/src/sdp/rsdparsa_capi/src/media_section.rs
@@ -2,19 +2,20 @@ use std::ptr;
 use std::os::raw::c_char;
 
 use libc::{size_t, uint32_t};
 
 use nserror::{nsresult, NS_OK, NS_ERROR_INVALID_ARG};
 use rsdparsa::{SdpBandwidth, SdpSession};
 use rsdparsa::media_type::{SdpMedia, SdpMediaValue, SdpProtocolValue,
                            SdpFormatList};
-use rsdparsa::attribute_type::SdpAttribute;
+use rsdparsa::attribute_type::{SdpAttribute, SdpAttributeRtpmap};
 
 use network::{get_bandwidth, RustSdpConnection};
+use types::StringView;
 
 #[no_mangle]
 pub unsafe extern "C" fn sdp_get_media_section(session: *const SdpSession,
                                                index: size_t) -> *const SdpMedia {
     return match (*session).media.get(index) {
         Some(m) => m,
         None => ptr::null()
     };
@@ -144,8 +145,36 @@ pub unsafe extern "C" fn sdp_get_media_c
     }
     NS_ERROR_INVALID_ARG
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn sdp_get_media_attribute_list(sdp_media: *const SdpMedia) -> *const Vec<SdpAttribute> {
     (*sdp_media).get_attributes()
 }
+
+#[no_mangle]
+pub unsafe extern "C" fn sdp_media_clear_codecs(sdp_media: *mut SdpMedia) {
+    (*sdp_media).remove_codecs()
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn sdp_media_add_codec(sdp_media: *mut SdpMedia, pt: u8,
+                                             codec_name: StringView, clockrate: u32,
+                                             channels: u16) -> nsresult {
+     let rtpmap = SdpAttributeRtpmap{
+                     payload_type: pt,
+                     codec_name: match codec_name.into() {
+                         Ok(x) => x,
+                         Err(boxed_error) => {
+                             println!("Error while pasing string, description: {:?}", (*boxed_error).description());
+                             return NS_ERROR_INVALID_ARG;
+                         }
+                     },
+                     frequency: clockrate,
+                     channels: Some(channels as u32),
+     };
+
+    match (*sdp_media).add_codec(rtpmap) {
+        Ok(_) => NS_OK,
+        Err(_) => NS_ERROR_INVALID_ARG,
+    }
+}
--- a/media/webrtc/signaling/src/sdp/rsdparsa_capi/src/types.rs
+++ b/media/webrtc/signaling/src/sdp/rsdparsa_capi/src/types.rs
@@ -1,9 +1,13 @@
 use libc::{size_t, uint32_t};
+use std::ffi::CStr;
+use std::{str, slice};
+use std::error::Error;
+use std::boxed::Box;
 
 use nserror::{nsresult, NS_OK, NS_ERROR_INVALID_ARG};
 
 #[repr(C)]
 #[derive(Clone, Copy)]
 pub struct StringView {
     buffer: *const u8,
     len: size_t
@@ -13,16 +17,42 @@ pub const NULL_STRING: StringView = Stri
                                                  len: 0 };
 
 impl<'a> From<&'a str> for StringView {
     fn from(input: &str) -> StringView {
         StringView { buffer: input.as_ptr(), len: input.len()}
     }
 }
 
+impl Into<Result<String,Box<Error>>> for StringView {
+    fn into(self) -> Result<String,Box<Error>> {
+
+        // This block must be unsafe as it converts a StringView, most likly provided from the
+        // C++ code, into a rust String and thus needs to operate with raw pointers.
+        let string_slice: &[u8];
+        unsafe {
+            // Add one to the length as the length passed in the StringView is the length of
+            // the string and is missing the null terminator
+            string_slice = slice::from_raw_parts(self.buffer, self.len+1 as usize);
+        }
+
+         let c_str = match CStr::from_bytes_with_nul(string_slice) {
+                 Ok(string) => string,
+                 Err(x) => { return Err(Box::new(x)); },
+         };
+
+         let str_slice: &str = match str::from_utf8(c_str.to_bytes()) {
+                 Ok(string) => string,
+                 Err(x) => { return Err(Box::new(x)); },
+         };
+
+         Ok(str_slice.to_string())
+    }
+}
+
 impl<'a, T: AsRef<str>> From<&'a Option<T>> for StringView {
     fn from(input: &Option<T>) -> StringView {
         match *input {
             Some(ref x) => StringView { buffer: x.as_ref().as_ptr(),
                                         len: x.as_ref().len()},
             None => NULL_STRING
         }
     }