Repository: pion/rtp
Pull request #155 (2021-11-22 05:45:55)

Add PaddingSize to rtp.Packet
When a packet has padding, the padding size information
is lost during unmarshal. Add a PaddingSize field to
Packet structure.

Also support marshalling on an RTP packet with padding.

Testing:
--------
Modified/added unit tests.


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/rtp/pull/155?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
> Merging [#155](https://codecov.io/gh/pion/rtp/pull/155?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (762664c) into [master](https://codecov.io/gh/pion/rtp/commit/45d5015175b2a0229d160bc4903a9ada2563e106?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (45d5015) will **decrease** coverage by `0.17%`.
> The diff coverage is `88.88%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/rtp/pull/155/graphs/tree.svg?width=650&height=150&src=pr&token=4KrbiBsspj&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)](https://codecov.io/gh/pion/rtp/pull/155?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)

```diff
@@            Coverage Diff             @@
##           master     #155      +/-   ##
==========================================
- Coverage   93.06%   92.88%   -0.18%     
==========================================
  Files          14       14              
  Lines         937      942       +5     
==========================================
+ Hits          872      875       +3     
- Misses         46       49       +3     
+ Partials       19       18       -1     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `?` | |
| wasm | `92.88% <88.88%> (+0.25%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/rtp/pull/155?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [packet.go](https://codecov.io/gh/pion/rtp/pull/155/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGFja2V0Lmdv) | `86.01% <88.88%> (-0.57%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/rtp/pull/155?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/rtp/pull/155?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Last update [45d5015...762664c](https://codecov.io/gh/pion/rtp/pull/155?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).


Patch:
diff --git a/packet.go b/packet.go
index c1d20f3..1065e17 100644
--- a/packet.go
+++ b/packet.go
@@ -30,7 +30,8 @@ type Header struct {
 // Packet represents an RTP Packet
 type Packet struct {
 	Header
-	Payload []byte
+	Payload     []byte
+	PaddingSize byte
 }
 
 const (
@@ -212,7 +213,8 @@ func (p *Packet) Unmarshal(buf []byte) error {
 	}
 	end := len(buf)
 	if p.Header.Padding {
-		end -= int(buf[end-1])
+		p.PaddingSize = buf[end-1]
+		end -= int(p.PaddingSize)
 	}
 	if end < n {
 		return errTooSmall
@@ -468,24 +470,28 @@ func (p *Packet) Marshal() (buf []byte, err error) {
 
 // MarshalTo serializes the packet and writes to the buffer.
 func (p *Packet) MarshalTo(buf []byte) (n int, err error) {
+	p.Header.Padding = p.PaddingSize != 0
 	n, err = p.Header.MarshalTo(buf)
 	if err != nil {
 		return 0, err
 	}
 
 	// Make sure the buffer is large enough to hold the packet.
-	if n+len(p.Payload) > len(buf) {
+	if n+len(p.Payload)+int(p.PaddingSize) > len(buf) {
 		return 0, io.ErrShortBuffer
 	}
 
 	m := copy(buf[n:], p.Payload)
+	if p.Header.Padding {
+		buf[n+m+int(p.PaddingSize-1)] = p.PaddingSize
+	}
 
-	return n + m, nil
+	return n + m + int(p.PaddingSize), nil
 }
 
 // MarshalSize returns the size of the packet once marshaled.
 func (p *Packet) MarshalSize() int {
-	return p.Header.MarshalSize() + len(p.Payload)
+	return p.Header.MarshalSize() + len(p.Payload) + int(p.PaddingSize)
 }
 
 // Clone returns a deep copy of p.
@@ -496,6 +502,7 @@ func (p *Packet) Clone() *Packet {
 		clone.Payload = make([]byte, len(p.Payload))
 		copy(clone.Payload, p.Payload)
 	}
+	clone.PaddingSize = p.PaddingSize
 	return clone
 }
 


Test patch:
diff --git a/packet_test.go b/packet_test.go
index abd76f9..5d19c11 100644
--- a/packet_test.go
+++ b/packet_test.go
@@ -38,7 +38,8 @@ func TestBasic(t *testing.T) {
 			SSRC:           476325762,
 			CSRC:           []uint32{},
 		},
-		Payload: rawPkt[20:],
+		Payload:     rawPkt[20:],
+		PaddingSize: 0,
 	}
 
 	// Unmarshal to the used Packet should work as well.
@@ -88,7 +89,8 @@ func TestBasic(t *testing.T) {
 			SSRC:           476325762,
 			CSRC:           []uint32{},
 		},
-		Payload: rawPkt[20:21],
+		Payload:     rawPkt[20:21],
+		PaddingSize: 4,
 	}
 	if err := p.Unmarshal(rawPkt); err != nil {
 		t.Error(err)
@@ -119,7 +121,8 @@ func TestBasic(t *testing.T) {
 			SSRC:           476325762,
 			CSRC:           []uint32{},
 		},
-		Payload: []byte{},
+		Payload:     []byte{},
+		PaddingSize: 5,
 	}
 	if err := p.Unmarshal(rawPkt); err != nil {
 		t.Error(err)
@@ -153,7 +156,8 @@ func TestBasic(t *testing.T) {
 			SSRC:           476325762,
 			CSRC:           []uint32{},
 		},
-		Payload: []byte{},
+		Payload:     []byte{},
+		PaddingSize: 0,
 	}
 	err := p.Unmarshal(rawPkt)
 	if err == nil {
@@ -162,6 +166,107 @@ func TestBasic(t *testing.T) {
 	if !errors.Is(err, errTooSmall) {
 		t.Errorf("Expected error: %v, got: %v", errTooSmall, err)
 	}
+
+	// marshal packet with padding
+	rawPkt = []byte{
+		0xb0, 0xe0, 0x69, 0x8f, 0xd9, 0xc2, 0x93, 0xda, 0x1c, 0x64,
+		0x27, 0x82, 0x00, 0x01, 0x00, 0x01, 0xFF, 0xFF, 0xFF, 0xFF, 0x98, 0x00, 0x00, 0x00, 0x04,
+	}
+	parsedPacket = &Packet{
+		Header: Header{
+			Padding:          true,
+			Marker:           true,
+			Extension:        true,
+			ExtensionProfile: 1,
+			Extensions: []Extension{
+				{0, []byte{
+					0xFF, 0xFF, 0xFF, 0xFF,
+				}},
+			},
+			Version:        2,
+			PayloadType:    96,
+			SequenceNumber: 27023,
+			Timestamp:      3653407706,
+			SSRC:           476325762,
+			CSRC:           []uint32{},
+		},
+		Payload:     rawPkt[20:21],
+		PaddingSize: 4,
+	}
+	buf, err := parsedPacket.Marshal()
+	if err != nil {
+		t.Error(err)
+	}
+	if !reflect.DeepEqual(buf, rawPkt) {
+		t.Errorf("TestBasic padding marshal: got %#v, want %#v", buf, rawPkt)
+	}
+
+	// marshal packet with padding only
+	rawPkt = []byte{
+		0xb0, 0xe0, 0x69, 0x8f, 0xd9, 0xc2, 0x93, 0xda, 0x1c, 0x64,
+		0x27, 0x82, 0x00, 0x01, 0x00, 0x01, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, 0x05,
+	}
+	parsedPacket = &Packet{
+		Header: Header{
+			Padding:          true,
+			Marker:           true,
+			Extension:        true,
+			ExtensionProfile: 1,
+			Extensions: []Extension{
+				{0, []byte{
+					0xFF, 0xFF, 0xFF, 0xFF,
+				}},
+			},
+			Version:        2,
+			PayloadType:    96,
+			SequenceNumber: 27023,
+			Timestamp:      3653407706,
+			SSRC:           476325762,
+			CSRC:           []uint32{},
+		},
+		Payload:     []byte{},
+		PaddingSize: 5,
+	}
+	buf, err = parsedPacket.Marshal()
+	if err != nil {
+		t.Error(err)
+	}
+	if !reflect.DeepEqual(buf, rawPkt) {
+		t.Errorf("TestBasic padding marshal: got %#v, want %#v", buf, rawPkt)
+	}
+
+	// marshal packet with padding only without setting Padding explicitly in Header
+	rawPkt = []byte{
+		0xb0, 0xe0, 0x69, 0x8f, 0xd9, 0xc2, 0x93, 0xda, 0x1c, 0x64,
+		0x27, 0x82, 0x00, 0x01, 0x00, 0x01, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, 0x05,
+	}
+	parsedPacket = &Packet{
+		Header: Header{
+			Marker:           true,
+			Extension:        true,
+			ExtensionProfile: 1,
+			Extensions: []Extension{
+				{0, []byte{
+					0xFF, 0xFF, 0xFF, 0xFF,
+				}},
+			},
+			Version:        2,
+			PayloadType:    96,
+			SequenceNumber: 27023,
+			Timestamp:      3653407706,
+			SSRC:           476325762,
+			CSRC:           []uint32{},
+		},
+		Payload:     []byte{},
+		PaddingSize: 5,
+	}
+	buf, err = parsedPacket.Marshal()
+	if err != nil {
+		t.Error(err)
+	}
+	if !reflect.DeepEqual(buf, rawPkt) {
+		t.Errorf("TestBasic padding marshal: got %#v, want %#v", buf, rawPkt)
+	}
 }
 
 func TestExtension(t *testing.T) {