Repository: pion/rtp
Pull request #230 (2023-07-16 21:29:42)

Readd non-breaking version of #119
#### Description

As i mentioned in my rollback patch (https://github.com/pion/rtp/pull/227), i removed two backward-incompatible PRs from the main branch and i wrote that i would have contributed them again in a non-breaking version.

This is the non-breaking version of #119.

Field `Header.PayloadOffset` has been marked as deprecated in order to allow maintainers to quickly find and remove it whenever this packet gets a major version bump. It is not populated anymore.

Field `Packet.Raw` has been marked as deprecated in order to allow maintainers to quickly find and remove it whenever this packet gets a major version bump. It is not populated anymore.




codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pion/rtp/pull/230?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
Patch coverage: **`100.00`**% and project coverage change: **`-0.01`** :warning:
> Comparison is base [(`5d59489`)](https://app.codecov.io/gh/pion/rtp/commit/5d59489081b941427ab4362c846175942728d328?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) 87.53% compared to head [(`b148165`)](https://app.codecov.io/gh/pion/rtp/pull/230?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) 87.52%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #230      +/-   ##
==========================================
- Coverage   87.53%   87.52%   -0.01%     
==========================================
  Files          20       20              
  Lines        1877     1876       -1     
==========================================
- Hits         1643     1642       -1     
  Misses        204      204              
  Partials       30       30              
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `87.52% <100.00%> (-0.01%)` | :arrow_down: |
| wasm | `86.56% <100.00%> (-0.01%)` | :arrow_down: |

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://app.codecov.io/gh/pion/rtp/pull/230?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [packet.go](https://app.codecov.io/gh/pion/rtp/pull/230?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGFja2V0Lmdv) | `88.37% <100.00%> (-0.04%)` | :arrow_down: |


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/pion/rtp/pull/230?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?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 a47e5fc..500c60f 100644
--- a/packet.go
+++ b/packet.go
@@ -16,13 +16,11 @@ type Extension struct {
 }
 
 // Header represents an RTP packet header
-// NOTE: PayloadOffset is populated by Marshal/Unmarshal and should not be modified
 type Header struct {
 	Version          uint8
 	Padding          bool
 	Extension        bool
 	Marker           bool
-	PayloadOffset    int
 	PayloadType      uint8
 	SequenceNumber   uint16
 	Timestamp        uint32
@@ -30,15 +28,19 @@ type Header struct {
 	CSRC             []uint32
 	ExtensionProfile uint16
 	Extensions       []Extension
+
+	// Deprecated: will be removed in a future version.
+	PayloadOffset int
 }
 
 // Packet represents an RTP Packet
-// NOTE: Raw is populated by Marshal/Unmarshal and should not be modified
 type Packet struct {
 	Header
-	Raw         []byte
 	Payload     []byte
 	PaddingSize byte
+
+	// Deprecated: will be removed in a future version.
+	Raw []byte
 }
 
 const (
@@ -210,14 +212,12 @@ func (h *Header) Unmarshal(buf []byte) (n int, err error) { //nolint:gocognit
 		}
 	}
 
-	h.PayloadOffset = n
-
 	return n, nil
 }
 
 // Unmarshal parses the passed byte slice and stores the result in the Packet.
 func (p *Packet) Unmarshal(buf []byte) error {
-	_, err := p.Header.Unmarshal(buf)
+	n, err := p.Header.Unmarshal(buf)
 	if err != nil {
 		return err
 	}
@@ -227,12 +227,11 @@ func (p *Packet) Unmarshal(buf []byte) error {
 		p.PaddingSize = buf[end-1]
 		end -= int(p.PaddingSize)
 	}
-	if end < p.PayloadOffset {
+	if end < n {
 		return errTooSmall
 	}
 
-	p.Payload = buf[p.PayloadOffset:end]
-	p.Raw = buf
+	p.Payload = buf[n:end]
 
 	return nil
 }
@@ -343,8 +342,6 @@ func (h Header) MarshalTo(buf []byte) (n int, err error) {
 		}
 	}
 
-	h.PayloadOffset = n
-
 	return n, nil
 }
 
@@ -499,7 +496,7 @@ func (p *Packet) MarshalTo(buf []byte) (n int, err error) {
 	}
 
 	m := copy(buf[n:], p.Payload)
-	p.Raw = buf[:n+m]
+
 	if p.Header.Padding {
 		buf[n+m+int(p.PaddingSize-1)] = p.PaddingSize
 	}


Test patch:
diff --git a/packet_test.go b/packet_test.go
index be13efb..bc8f97a 100644
--- a/packet_test.go
+++ b/packet_test.go
@@ -35,7 +35,6 @@ func TestBasic(t *testing.T) {
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  20,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -43,7 +42,6 @@ func TestBasic(t *testing.T) {
 			CSRC:           []uint32{},
 		},
 		Payload:     rawPkt[20:],
-		Raw:         rawPkt,
 		PaddingSize: 0,
 	}
 
@@ -62,20 +60,12 @@ func TestBasic(t *testing.T) {
 				t.Errorf("wrong computed marshal size")
 			}
 
-			if p.PayloadOffset != 20 {
-				t.Errorf("wrong payload offset: %d != %d", p.PayloadOffset, 20)
-			}
-
 			raw, err := p.Marshal()
 			if err != nil {
 				t.Error(err)
 			} else if !reflect.DeepEqual(raw, rawPkt) {
 				t.Errorf("TestBasic marshal: got %#v, want %#v", raw, rawPkt)
 			}
-
-			if p.PayloadOffset != 20 {
-				t.Errorf("wrong payload offset: %d != %d", p.PayloadOffset, 20)
-			}
 		})
 	}
 
@@ -101,10 +91,8 @@ func TestBasic(t *testing.T) {
 			Timestamp:      3653407706,
 			SSRC:           476325762,
 			CSRC:           []uint32{},
-			PayloadOffset:  20,
 		},
 		Payload:     rawPkt[20:21],
-		Raw:         rawPkt,
 		PaddingSize: 4,
 	}
 	if err := p.Unmarshal(rawPkt); err != nil {
@@ -135,10 +123,8 @@ func TestBasic(t *testing.T) {
 			Timestamp:      3653407706,
 			SSRC:           476325762,
 			CSRC:           []uint32{},
-			PayloadOffset:  20,
 		},
 		Payload:     []byte{},
-		Raw:         rawPkt,
 		PaddingSize: 5,
 	}
 	if err := p.Unmarshal(rawPkt); err != nil {
@@ -345,7 +331,6 @@ func TestRFC8285OneByteExtension(t *testing.T) {
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  18,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -353,7 +338,6 @@ func TestRFC8285OneByteExtension(t *testing.T) {
 			CSRC:           []uint32{},
 		},
 		Payload: rawPkt[20:],
-		Raw:     rawPkt,
 	}
 
 	dstData, _ := p.Marshal()
@@ -409,7 +393,6 @@ func TestRFC8285OneByteTwoExtensionOfTwoBytes(t *testing.T) {
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -417,7 +400,6 @@ func TestRFC8285OneByteTwoExtensionOfTwoBytes(t *testing.T) {
 			CSRC:           []uint32{},
 		},
 		Payload: rawPkt[20:],
-		Raw:     rawPkt,
 	}
 
 	dstData, _ := p.Marshal()
@@ -534,7 +516,6 @@ func TestRFC8285OneByteMultipleExtensions(t *testing.T) {
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -542,7 +523,6 @@ func TestRFC8285OneByteMultipleExtensions(t *testing.T) {
 			CSRC:           []uint32{},
 		},
 		Payload: rawPkt[28:],
-		Raw:     rawPkt,
 	}
 
 	dstData, _ := p.Marshal()
@@ -578,7 +558,6 @@ func TestRFC8285TwoByteExtension(t *testing.T) {
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  42,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -586,7 +565,6 @@ func TestRFC8285TwoByteExtension(t *testing.T) {
 			CSRC:           []uint32{},
 		},
 		Payload: rawPkt[44:],
-		Raw:     rawPkt,
 	}
 
 	dstData, _ := p.Marshal()
@@ -681,7 +659,6 @@ func TestRFC8285TwoByteMultipleExtensionsWithLargeExtension(t *testing.T) {
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  40,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -689,7 +666,6 @@ func TestRFC8285TwoByteMultipleExtensionsWithLargeExtension(t *testing.T) {
 			CSRC:           []uint32{},
 		},
 		Payload: rawPkt[40:],
-		Raw:     rawPkt,
 	}
 
 	dstData, _ := p.Marshal()
@@ -708,7 +684,6 @@ func TestRFC8285GetExtensionReturnsNilWhenExtensionsDisabled(t *testing.T) {
 			Marker:         true,
 			Extension:      false,
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -740,7 +715,6 @@ func TestRFC8285DelExtension(t *testing.T) {
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -790,7 +764,6 @@ func TestRFC8285GetExtensionIDs(t *testing.T) {
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -826,7 +799,6 @@ func TestRFC8285GetExtensionIDsReturnsErrorWhenExtensionsDisabled(t *testing.T)
 			Marker:         true,
 			Extension:      false,
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -852,7 +824,6 @@ func TestRFC8285DelExtensionReturnsErrorWhenExtensionsDisabled(t *testing.T) {
 			Marker:         true,
 			Extension:      false,
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -878,7 +849,6 @@ func TestRFC8285OneByteSetExtensionShouldEnableExensionsWhenAdding(t *testing.T)
 			Marker:         true,
 			Extension:      false,
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -921,7 +891,6 @@ func TestRFC8285OneByteSetExtensionShouldSetCorrectExtensionProfileFor16ByteExte
 			Marker:         true,
 			Extension:      false,
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -963,7 +932,6 @@ func TestRFC8285OneByteSetExtensionShouldUpdateExistingExension(t *testing.T) {
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -1004,7 +972,6 @@ func TestRFC8285OneByteSetExtensionShouldErrorWhenInvalidIDProvided(t *testing.T
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -1062,7 +1029,6 @@ func TestRFC8285OneByteSetExtensionShouldErrorWhenPayloadTooLarge(t *testing.T)
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -1090,7 +1056,6 @@ func TestRFC8285TwoByteSetExtensionShouldEnableExensionsWhenAdding(t *testing.T)
 			Marker:         true,
 			Extension:      false,
 			Version:        2,
-			PayloadOffset:  31,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -1142,7 +1107,6 @@ func TestRFC8285TwoByteSetExtensionShouldUpdateExistingExension(t *testing.T) {
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -1186,7 +1150,6 @@ func TestRFC8285TwoByteSetExtensionShouldErrorWhenPayloadTooLarge(t *testing.T)
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -1244,7 +1207,6 @@ func TestRFC3550SetExtensionShouldErrorWhenNonZero(t *testing.T) {
 				}},
 			},
 			Version:        2,
-			PayloadOffset:  26,
 			PayloadType:    96,
 			SequenceNumber: 27023,
 			Timestamp:      3653407706,
@@ -1276,7 +1238,6 @@ func TestRFC3550SetExtensionShouldRaiseErrorWhenSettingNonzeroID(t *testing.T) {
 			Extension:        true,
 			ExtensionProfile: 0x1111,
 			Version:          2,
-			PayloadOffset:    26,
 			PayloadType:      96,
 			SequenceNumber:   27023,
 			Timestamp:        3653407706,
@@ -1364,9 +1325,6 @@ func TestRoundtrip(t *testing.T) {
 	if err := p.Unmarshal(rawPkt); err != nil {
 		t.Fatal(err)
 	}
-	if !bytes.Equal(rawPkt, p.Raw) {
-		t.Errorf("p.Raw must be same as rawPkt.\n p.Raw: %+v,\nrawPkt: %+v", p.Raw, rawPkt)
-	}
 	if !bytes.Equal(payload, p.Payload) {
 		t.Errorf("p.Payload must be same as payload.\n  payload: %+v,\np.Payload: %+v",
 			payload, p.Payload,
@@ -1380,9 +1338,6 @@ func TestRoundtrip(t *testing.T) {
 	if !bytes.Equal(rawPkt, buf) {
 		t.Errorf("buf must be same as rawPkt.\n   buf: %+v,\nrawPkt: %+v", buf, rawPkt)
 	}
-	if !bytes.Equal(rawPkt, p.Raw) {
-		t.Errorf("p.Raw must be same as rawPkt.\n p.Raw: %+v,\nrawPkt: %+v", p.Raw, rawPkt)
-	}
 	if !bytes.Equal(payload, p.Payload) {
 		t.Errorf("p.Payload must be same as payload.\n  payload: %+v,\np.Payload: %+v",
 			payload, p.Payload,
diff --git a/packetizer_test.go b/packetizer_test.go
index a4ac3b8..78f8220 100644
--- a/packetizer_test.go
+++ b/packetizer_test.go
@@ -51,7 +51,6 @@ func TestPacketizer_AbsSendTime(t *testing.T) {
 			Padding:          false,
 			Extension:        true,
 			Marker:           true,
-			PayloadOffset:    0, // not set by Packetize() at now
 			PayloadType:      98,
 			SequenceNumber:   1234,
 			Timestamp:        45678,
@@ -144,9 +143,7 @@ func TestPacketizer_Roundtrip(t *testing.T) {
 			t.Errorf("Packet versions don't match, expected %v but got %v", expectedPkt.Payload, pkt.Payload)
 		}
 
-		pkt.PayloadOffset = 0
 		pkt.PaddingSize = 0
-		pkt.Raw = nil
 
 		if !reflect.DeepEqual(expectedPkt, pkt) {
 			t.Errorf("Packets don't match, expected %v but got %v", expectedPkt, pkt)