Repository: pion/webrtc
Pull request #1493 (2020-10-25 01:38:38)

Add SessionDescription Unmarshal helper
#### Description

This will allow to re-use the internally parsed sdp, and prevent parsing multiple times and save some cpu cycles.

codecov[bot]: # [Codecov](https://codecov.io/gh/pion/webrtc/pull/1493?src=pr&el=h1) Report
> Merging [#1493](https://codecov.io/gh/pion/webrtc/pull/1493?src=pr&el=desc) into [master](https://codecov.io/gh/pion/webrtc/commit/a38b7a48e1e6ccefe6a899259c36cc9b5f84d00e?el=desc) will **decrease** coverage by `2.61%`.
> The diff coverage is `0.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/webrtc/pull/1493/graphs/tree.svg?width=650&height=150&src=pr&token=XOOuQo07I2)](https://codecov.io/gh/pion/webrtc/pull/1493?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1493      +/-   ##
==========================================
- Coverage   74.29%   71.68%   -2.62%     
==========================================
  Files          73       54      -19     
  Lines        5291     1946    -3345     
==========================================
- Hits         3931     1395    -2536     
+ Misses        999      452     -547     
+ Partials      361       99     -262     
```

| Flag | Coverage Δ | |
|---|---|---|
| #go | `?` | |
| #wasm | `71.68% <0.00%> (-0.19%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/webrtc/pull/1493?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sessiondescription.go](https://codecov.io/gh/pion/webrtc/pull/1493/diff?src=pr&el=tree#diff-c2Vzc2lvbmRlc2NyaXB0aW9uLmdv) | `0.00% <0.00%> (ø)` | |
| [icetcp.go](https://codecov.io/gh/pion/webrtc/pull/1493/diff?src=pr&el=tree#diff-aWNldGNwLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [sdpsemantics.go](https://codecov.io/gh/pion/webrtc/pull/1493/diff?src=pr&el=tree#diff-c2Rwc2VtYW50aWNzLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [icecandidatepair.go](https://codecov.io/gh/pion/webrtc/pull/1493/diff?src=pr&el=tree#diff-aWNlY2FuZGlkYXRlcGFpci5nbw==) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [stats.go](https://codecov.io/gh/pion/webrtc/pull/1493/diff?src=pr&el=tree#diff-c3RhdHMuZ28=) | `0.00% <0.00%> (-67.86%)` | :arrow_down: |
| [gathering\_complete\_promise.go](https://codecov.io/gh/pion/webrtc/pull/1493/diff?src=pr&el=tree#diff-Z2F0aGVyaW5nX2NvbXBsZXRlX3Byb21pc2UuZ28=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [dtlsrole.go](https://codecov.io/gh/pion/webrtc/pull/1493/diff?src=pr&el=tree#diff-ZHRsc3JvbGUuZ28=) | `68.96% <0.00%> (-31.04%)` | :arrow_down: |
| [networktype.go](https://codecov.io/gh/pion/webrtc/pull/1493/diff?src=pr&el=tree#diff-bmV0d29ya3R5cGUuZ28=) | `46.80% <0.00%> (-19.15%)` | :arrow_down: |
| [icegathererstate.go](https://codecov.io/gh/pion/webrtc/pull/1493/diff?src=pr&el=tree#diff-aWNlZ2F0aGVyZXJzdGF0ZS5nbw==) | `84.61% <0.00%> (-15.39%)` | :arrow_down: |
| [icecandidatetype.go](https://codecov.io/gh/pion/webrtc/pull/1493/diff?src=pr&el=tree#diff-aWNlY2FuZGlkYXRldHlwZS5nbw==) | `64.70% <0.00%> (-14.71%)` | :arrow_down: |
| ... and [24 more](https://codecov.io/gh/pion/webrtc/pull/1493/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/webrtc/pull/1493?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/webrtc/pull/1493?src=pr&el=footer). Last update [a38b7a4...76bc992](https://codecov.io/gh/pion/webrtc/pull/1493?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/peerconnection.go b/peerconnection.go
index d49d6609..8e4c9da3 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -964,8 +964,7 @@ func (pc *PeerConnection) SetRemoteDescription(desc SessionDescription) error {
 
 	isRenegotation := pc.currentRemoteDescription != nil
 
-	desc.parsed = &sdp.SessionDescription{}
-	if err := desc.parsed.Unmarshal([]byte(desc.SDP)); err != nil {
+	if _, err := desc.Unmarshal(); err != nil {
 		return err
 	}
 	if err := pc.setDescription(&desc, stateChangeOpSetRemote); err != nil {
diff --git a/sessiondescription.go b/sessiondescription.go
index 4087ed71..ea8927e0 100644
--- a/sessiondescription.go
+++ b/sessiondescription.go
@@ -12,3 +12,14 @@ type SessionDescription struct {
 	// This will never be initialized by callers, internal use only
 	parsed *sdp.SessionDescription
 }
+
+// Unmarshal is a helper to deserialize the sdp, and re-use it internally
+// if required
+func (sd *SessionDescription) Unmarshal() (*sdp.SessionDescription, error) {
+	if sd.parsed != nil {
+		return sd.parsed, nil
+	}
+	sd.parsed = &sdp.SessionDescription{}
+	err := sd.parsed.Unmarshal([]byte(sd.SDP))
+	return sd.parsed, err
+}


Test patch:
diff --git a/sessiondescription_test.go b/sessiondescription_test.go
index e5706fd9..dfe3f7a7 100644
--- a/sessiondescription_test.go
+++ b/sessiondescription_test.go
@@ -57,3 +57,23 @@ func TestSessionDescription_JSON(t *testing.T) {
 		)
 	}
 }
+
+func TestSessionDescription_Unmarshal(t *testing.T) {
+	pc, err := NewPeerConnection(Configuration{})
+	assert.NoError(t, err)
+	offer, err := pc.CreateOffer(nil)
+	assert.NoError(t, err)
+	desc := SessionDescription{
+		Type: offer.Type,
+		SDP:  offer.SDP,
+	}
+	assert.Nil(t, desc.parsed)
+	parsed, err := desc.Unmarshal()
+	assert.NotNil(t, parsed)
+	assert.NotNil(t, desc.parsed)
+	assert.NoError(t, err)
+	parsed, err = desc.Unmarshal()
+	assert.NotNil(t, parsed)
+	assert.NoError(t, err)
+	pc.Close()
+}