Repository: pion/rtp
Pull request #60 (2020-05-15 16:44:58)

RTP Packet del helper + fix
Some minor fixes/updates:
* DelExtension helper function
* Return nil in GetExtension if h.Extension is false
* Fix extension size check in SetExtension

codecov[bot]: # [Codecov](https://codecov.io/gh/pion/rtp/pull/60?src=pr&el=h1) Report
> Merging [#60](https://codecov.io/gh/pion/rtp/pull/60?src=pr&el=desc) into [master](https://codecov.io/gh/pion/rtp/commit/4b1b58251dc834458b8650376b674fe1613e84d7&el=desc) will **increase** coverage by `0.34%`.
> The diff coverage is `76.92%`.

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

```diff
@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
+ Coverage   88.51%   88.85%   +0.34%     
==========================================
  Files          13       13              
  Lines         618      628      +10     
==========================================
+ Hits          547      558      +11     
+ Misses         55       54       -1     
  Partials       16       16              
```

| Flag | Coverage Δ | |
|---|---|---|
| #go | `88.85% <76.92%> (+0.34%)` | :arrow_up: |
| #wasm | `88.85% <76.92%> (+0.34%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pion/rtp/pull/60?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [packet.go](https://codecov.io/gh/pion/rtp/pull/60/diff?src=pr&el=tree#diff-cGFja2V0Lmdv) | `82.69% <76.92%> (+1.37%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/rtp/pull/60?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/rtp/pull/60?src=pr&el=footer). Last update [4b1b582...e2334ff](https://codecov.io/gh/pion/rtp/pull/60?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/packet.go b/packet.go
index 076638c..6986623 100644
--- a/packet.go
+++ b/packet.go
@@ -354,10 +354,10 @@ func (h *Header) SetExtension(id uint8, payload []byte) error {
 		// RFC 8285 RTP Two Byte Header Extension
 		case extensionProfileTwoByte:
 			if id < 1 || id > 255 {
-				return fmt.Errorf("header extension id must be between 1 and 14 for RFC 5285 extensions")
+				return fmt.Errorf("header extension id must be between 1 and 255 for RFC 5285 extensions")
 			}
-			if len(payload) > 256 {
-				return fmt.Errorf("header extension payload must be 256bytes or less for RFC 5285 two byte extensions")
+			if len(payload) > 255 {
+				return fmt.Errorf("header extension payload must be 255bytes or less for RFC 5285 two byte extensions")
 			}
 		default: // RFC3550 Extension
 			if id != 0 {
@@ -392,6 +392,9 @@ func (h *Header) SetExtension(id uint8, payload []byte) error {
 
 // GetExtension returns an RTP header extension
 func (h *Header) GetExtension(id uint8) []byte {
+	if !h.Extension {
+		return nil
+	}
 	for _, extension := range h.Extensions {
 		if extension.id == id {
 			return extension.payload
@@ -400,6 +403,20 @@ func (h *Header) GetExtension(id uint8) []byte {
 	return nil
 }
 
+// DelExtension Removes an RTP Header extension
+func (h *Header) DelExtension(id uint8) error {
+	if !h.Extension {
+		return fmt.Errorf("h.Extension not enabled")
+	}
+	for i, extension := range h.Extensions {
+		if extension.id == id {
+			h.Extensions = append(h.Extensions[:i], h.Extensions[i+1:]...)
+			return nil
+		}
+	}
+	return fmt.Errorf("extension not found")
+}
+
 // Marshal serializes the packet into bytes.
 func (p *Packet) Marshal() (buf []byte, err error) {
 	buf = make([]byte, p.MarshalSize())


Test patch:
diff --git a/packet_test.go b/packet_test.go
index 9405f0d..89eba2b 100644
--- a/packet_test.go
+++ b/packet_test.go
@@ -447,6 +447,102 @@ func TestRFC8285TwoByteMultipleExtensionsWithLargeExtension(t *testing.T) {
 	}
 }
 
+func TestRFC8285GetExtensionReturnsNilWhenExtensionsDisabled(t *testing.T) {
+	payload := []byte{
+		// Payload
+		0x98, 0x36, 0xbe, 0x88, 0x9e,
+	}
+	p := &Packet{Header: Header{
+		Marker:         true,
+		Extension:      false,
+		Version:        2,
+		PayloadOffset:  26,
+		PayloadType:    96,
+		SequenceNumber: 27023,
+		Timestamp:      3653407706,
+		SSRC:           476325762,
+		CSRC:           []uint32{},
+	},
+		Payload: payload,
+	}
+
+	err := p.GetExtension(1)
+	if err != nil {
+		t.Error("Should return nil on GetExtension when h.Extension: false")
+	}
+}
+
+func TestRFC8285DelExtension(t *testing.T) {
+	payload := []byte{
+		// Payload
+		0x98, 0x36, 0xbe, 0x88, 0x9e,
+	}
+	p := &Packet{Header: Header{
+		Marker:           true,
+		Extension:        true,
+		ExtensionProfile: 0xBEDE,
+		Extensions: []Extension{
+			{1, []byte{
+				0xAA,
+			}},
+		},
+		Version:        2,
+		PayloadOffset:  26,
+		PayloadType:    96,
+		SequenceNumber: 27023,
+		Timestamp:      3653407706,
+		SSRC:           476325762,
+		CSRC:           []uint32{},
+	},
+		Payload: payload,
+	}
+
+	ext := p.GetExtension(1)
+	if ext == nil {
+		t.Error("Extension should exist")
+	}
+
+	err := p.DelExtension(1)
+	if err != nil {
+		t.Error("Should successfully delete extension")
+	}
+
+	ext = p.GetExtension(1)
+	if ext != nil {
+		t.Error("Extension should not exist")
+	}
+
+	err = p.DelExtension(1)
+	if err == nil {
+		t.Error("Should return error when deleting extension that doesnt exist")
+	}
+}
+
+func TestRFC8285DelExtensionReturnsErrorWhenExtensionsDisabled(t *testing.T) {
+	payload := []byte{
+		// Payload
+		0x98, 0x36, 0xbe, 0x88, 0x9e,
+	}
+	p := &Packet{Header: Header{
+		Marker:         true,
+		Extension:      false,
+		Version:        2,
+		PayloadOffset:  26,
+		PayloadType:    96,
+		SequenceNumber: 27023,
+		Timestamp:      3653407706,
+		SSRC:           476325762,
+		CSRC:           []uint32{},
+	},
+		Payload: payload,
+	}
+
+	err := p.DelExtension(1)
+	if err == nil {
+		t.Error("Should return error on DelExtension when h.Extension: false")
+	}
+}
+
 func TestRFC8285OneByteSetExtensionShouldEnableExensionsWhenAdding(t *testing.T) {
 	payload := []byte{
 		// Payload