From 36e9a99ebde754ccff7cac57baf39b82d210484d Mon Sep 17 00:00:00 2001 From: Juliusz Chroboczek Date: Mon, 15 Apr 2024 17:43:37 +0200 Subject: [PATCH] Remove OBU parsing from AV1Packet In Pion, the (*XXXPacket).Unmarshal operation is supposed to be fast and perform no allocation, as it is sometimes used to check just a single flag in the packet header. The AV1Packet implementation breaks this property by parsing the whole list of OBUs at Unmarshal time, even though it is likely to be unneeded. We remove OBU parsing from Unmarshal, to make the method consistent with the other implementations in Pion (for example, H264Packet does not parse the list of NALs at Unmarshal time). If OBU parsing is required, it should be provided as a user-callable function in codecs/av1/obu. --- codecs/av1_packet.go | 35 ----------------------------------- codecs/av1_packet_test.go | 8 -------- 2 files changed, 43 deletions(-) diff --git a/codecs/av1_packet.go b/codecs/av1_packet.go index 3a78b90..e7404cc 100644 --- a/codecs/av1_packet.go +++ b/codecs/av1_packet.go @@ -130,10 +130,6 @@ type AV1Packet struct { // N: MUST be set to 1 if the packet is the first packet of a coded video sequence, and MUST be set to 0 otherwise. N bool - - // Each AV1 RTP Packet is a collection of OBU Elements. Each OBU Element may be a full OBU, or just a fragment of one. - // AV1Frame provides the tools to construct a collection of OBUs from a collection of OBU Elements - OBUElements [][]byte } // Unmarshal parses the passed byte slice and stores the result in the AV1Packet this method is called upon @@ -153,36 +149,5 @@ func (p *AV1Packet) Unmarshal(payload []byte) ([]byte, error) { return nil, errIsKeyframeAndFragment } - currentIndex := uint(1) - p.OBUElements = [][]byte{} - - var ( - obuElementLength, bytesRead uint - err error - ) - for i := 1; ; i++ { - if currentIndex == uint(len(payload)) { - break - } - - // If W bit is set the last OBU Element will have no length header - if byte(i) == p.W { - bytesRead = 0 - obuElementLength = uint(len(payload)) - currentIndex - } else { - obuElementLength, bytesRead, err = obu.ReadLeb128(payload[currentIndex:]) - if err != nil { - return nil, err - } - } - - currentIndex += bytesRead - if uint(len(payload)) < currentIndex+obuElementLength { - return nil, errShortPacket - } - p.OBUElements = append(p.OBUElements, payload[currentIndex:currentIndex+obuElementLength]) - currentIndex += obuElementLength - } - return payload[1:], nil } diff --git a/codecs/av1_packet_test.go b/codecs/av1_packet_test.go index 2a65c4f..9a30bbc 100644 --- a/codecs/av1_packet_test.go +++ b/codecs/av1_packet_test.go @@ -8,8 +8,6 @@ import ( "errors" "reflect" "testing" - - "github.com/pion/rtp/codecs/av1/obu" ) func TestAV1_Marshal(t *testing.T) { @@ -89,8 +87,6 @@ func TestAV1_Unmarshal_Error(t *testing.T) { {errNilPacket, nil}, {errShortPacket, []byte{0x00}}, {errIsKeyframeAndFragment, []byte{byte(0b10001000), 0x00}}, - {obu.ErrFailedToReadLEB128, []byte{byte(0b10000000), 0xFF, 0xFF}}, - {errShortPacket, []byte{byte(0b10000000), 0xFF, 0x0F, 0x00, 0x00}}, } { test := test av1Pkt := &AV1Packet{} @@ -284,10 +280,6 @@ func TestAV1_Unmarshal(t *testing.T) { Y: true, W: 2, N: true, - OBUElements: [][]byte{ - av1Payload[2:14], - av1Payload[14:], - }, }) { t.Fatal("AV1 Unmarshal didn't store the expected results in the packet") }