Repository: pion/rtp
Pull request #110 (2020-12-19 20:08:36)

Support AVC depacketizer
#### Description
- Add `IsAVC` flag to choose between `Annex B` or `AVC` packaging,
- Add `fuaBuffer` to keep non-final `FU-A` packets until we know the complete `NALU` length, and
- Add `fuaEndBitmask` to determine if we have to flush `FU-A` buffer.

#### Reference issue
https://github.com/pion/rtp/issues/107

codecov[bot]: # [Codecov](https://codecov.io/gh/pion/rtp/pull/110?src=pr&el=h1) Report
> Merging [#110](https://codecov.io/gh/pion/rtp/pull/110?src=pr&el=desc) (efbb92e) into [master](https://codecov.io/gh/pion/rtp/commit/319e780b743768216fcd1facedd8b26ca208ff01?el=desc) (319e780) will **decrease** coverage by `0.67%`.
> The diff coverage is `68.75%`.

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

```diff
@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
- Coverage   91.26%   90.59%   -0.68%     
==========================================
  Files          13       13              
  Lines         618      627       +9     
==========================================
+ Hits          564      568       +4     
- Misses         37       40       +3     
- Partials       17       19       +2     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `90.59% <68.75%> (-0.68%)` | :arrow_down: |
| wasm | `89.95% <68.75%> (-0.67%)` | :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/rtp/pull/110?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [codecs/h264\_packet.go](https://codecov.io/gh/pion/rtp/pull/110/diff?src=pr&el=tree#diff-Y29kZWNzL2gyNjRfcGFja2V0Lmdv) | `92.63% <68.75%> (-5.05%)` | :arrow_down: |

------

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

debiandebiandebian: @Sean-Der Can you review this PR?

Patch:
diff --git a/README.md b/README.md
index b17709e..9679764 100644
--- a/README.md
+++ b/README.md
@@ -48,6 +48,7 @@ Check out the **[contributing wiki](https://github.com/pion/webrtc/wiki/Contribu
 * [Kazuyuki Honda](https://github.com/hakobera)
 * [Haiyang Wang](https://github.com/ocean2811)
 * [lxb](https://github.com/lxb531)
+* [debiandebiandebian](https://github.com/debiandebiandebian)
 
 ### License
 MIT License - see [LICENSE](LICENSE) for full text
diff --git a/codecs/h264_packet.go b/codecs/h264_packet.go
index 3ee5926..910aac0 100644
--- a/codecs/h264_packet.go
+++ b/codecs/h264_packet.go
@@ -18,7 +18,7 @@ const (
 
 	naluTypeBitmask   = 0x1F
 	naluRefIdcBitmask = 0x60
-	fuaStartBitmask   = 0x80
+	fuaEndBitmask     = 0x40
 )
 
 func annexbNALUStartCode() []byte { return []byte{0x00, 0x00, 0x00, 0x01} }
@@ -149,6 +149,18 @@ func (p *H264Payloader) Payload(mtu int, payload []byte) [][]byte {
 
 // H264Packet represents the H264 header that is stored in the payload of an RTP Packet
 type H264Packet struct {
+	IsAVC     bool
+	fuaBuffer []byte
+}
+
+func (p *H264Packet) doPackaging(nalu []byte) []byte {
+	if p.IsAVC {
+		naluLength := make([]byte, 4)
+		binary.BigEndian.PutUint32(naluLength, uint32(len(nalu)))
+		return append(naluLength, nalu...)
+	}
+
+	return append(annexbNALUStartCode(), nalu...)
 }
 
 // Unmarshal parses the passed byte slice and stores the result in the H264Packet this method is called upon
@@ -164,7 +176,7 @@ func (p *H264Packet) Unmarshal(payload []byte) ([]byte, error) {
 	naluType := payload[0] & naluTypeBitmask
 	switch {
 	case naluType > 0 && naluType < 24:
-		return append(annexbNALUStartCode(), payload...), nil
+		return p.doPackaging(payload), nil
 
 	case naluType == stapaNALUType:
 		currOffset := int(stapaHeaderSize)
@@ -177,8 +189,7 @@ func (p *H264Packet) Unmarshal(payload []byte) ([]byte, error) {
 				return nil, fmt.Errorf("%w STAP-A declared size(%d) is larger than buffer(%d)", errShortPacket, naluSize, len(payload)-currOffset)
 			}
 
-			result = append(result, annexbNALUStartCode()...)
-			result = append(result, payload[currOffset:currOffset+naluSize]...)
+			result = append(result, p.doPackaging(payload[currOffset:currOffset+naluSize])...)
 			currOffset += naluSize
 		}
 		return result, nil
@@ -188,17 +199,23 @@ func (p *H264Packet) Unmarshal(payload []byte) ([]byte, error) {
 			return nil, errShortPacket
 		}
 
-		if payload[1]&fuaStartBitmask != 0 {
+		if p.fuaBuffer == nil {
+			p.fuaBuffer = []byte{}
+		}
+
+		p.fuaBuffer = append(p.fuaBuffer, payload[fuaHeaderSize:]...)
+
+		if payload[1]&fuaEndBitmask != 0 {
 			naluRefIdc := payload[0] & naluRefIdcBitmask
 			fragmentedNaluType := payload[1] & naluTypeBitmask
 
-			// Take a copy of payload since we are mutating it.
-			payloadCopy := append([]byte{}, payload...)
-			payloadCopy[fuaHeaderSize-1] = naluRefIdc | fragmentedNaluType
-			return append(annexbNALUStartCode(), payloadCopy[fuaHeaderSize-1:]...), nil
+			nalu := append([]byte{}, naluRefIdc|fragmentedNaluType)
+			nalu = append(nalu, p.fuaBuffer...)
+			p.fuaBuffer = nil
+			return p.doPackaging(nalu), nil
 		}
 
-		return payload[fuaHeaderSize:], nil
+		return []byte{}, nil
 	}
 
 	return nil, fmt.Errorf("%w: %d", errUnhandledNALUType, naluType)


Test patch:
diff --git a/codecs/h264_packet_test.go b/codecs/h264_packet_test.go
index 83c170c..d7366a9 100644
--- a/codecs/h264_packet_test.go
+++ b/codecs/h264_packet_test.go
@@ -91,8 +91,10 @@ func TestH264Payloader_Payload(t *testing.T) {
 func TestH264Packet_Unmarshal(t *testing.T) {
 	singlePayload := []byte{0x90, 0x90, 0x90}
 	singlePayloadUnmarshaled := []byte{0x00, 0x00, 0x00, 0x01, 0x90, 0x90, 0x90}
+	singlePayloadUnmarshaledAVC := []byte{0x00, 0x00, 0x00, 0x03, 0x90, 0x90, 0x90}
 
 	largepayload := []byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15}
+	largepayloadAVC := []byte{0x00, 0x00, 0x00, 0x10, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15}
 	largePayloadPacketized := [][]byte{
 		{0x1c, 0x80, 0x01, 0x02, 0x03},
 		{0x1c, 0x00, 0x04, 0x05, 0x06},
@@ -103,10 +105,12 @@ func TestH264Packet_Unmarshal(t *testing.T) {
 
 	singlePayloadMultiNALU := []byte{0x78, 0x00, 0x0f, 0x67, 0x42, 0xc0, 0x1f, 0x1a, 0x32, 0x35, 0x01, 0x40, 0x7a, 0x40, 0x3c, 0x22, 0x11, 0xa8, 0x00, 0x05, 0x68, 0x1a, 0x34, 0xe3, 0xc8}
 	singlePayloadMultiNALUUnmarshaled := []byte{0x00, 0x00, 0x00, 0x01, 0x67, 0x42, 0xc0, 0x1f, 0x1a, 0x32, 0x35, 0x01, 0x40, 0x7a, 0x40, 0x3c, 0x22, 0x11, 0xa8, 0x00, 0x00, 0x00, 0x01, 0x68, 0x1a, 0x34, 0xe3, 0xc8}
+	singlePayloadMultiNALUUnmarshaledAVC := []byte{0x00, 0x00, 0x00, 0x0f, 0x67, 0x42, 0xc0, 0x1f, 0x1a, 0x32, 0x35, 0x01, 0x40, 0x7a, 0x40, 0x3c, 0x22, 0x11, 0xa8, 0x00, 0x00, 0x00, 0x05, 0x68, 0x1a, 0x34, 0xe3, 0xc8}
 
 	incompleteSinglePayloadMultiNALU := []byte{0x78, 0x00, 0x0f, 0x67, 0x42, 0xc0, 0x1f, 0x1a, 0x32, 0x35, 0x01, 0x40, 0x7a, 0x40, 0x3c, 0x22, 0x11}
 
 	pkt := H264Packet{}
+	avcPkt := H264Packet{IsAVC: true}
 	if _, err := pkt.Unmarshal(nil); err == nil {
 		t.Fatal("Unmarshal did not fail on nil payload")
 	}
@@ -130,6 +134,13 @@ func TestH264Packet_Unmarshal(t *testing.T) {
 		t.Fatal("Unmarshaling a single payload shouldn't modify the payload")
 	}
 
+	res, err = avcPkt.Unmarshal(singlePayload)
+	if err != nil {
+		t.Fatal(err)
+	} else if !reflect.DeepEqual(res, singlePayloadUnmarshaledAVC) {
+		t.Fatal("Unmarshaling a single payload into avc stream shouldn't modify the payload")
+	}
+
 	largePayloadResult := []byte{}
 	for i := range largePayloadPacketized {
 		res, err = pkt.Unmarshal(largePayloadPacketized[i])
@@ -142,10 +153,29 @@ func TestH264Packet_Unmarshal(t *testing.T) {
 		t.Fatal("Failed to unmarshal a large payload")
 	}
 
+	largePayloadResultAVC := []byte{}
+	for i := range largePayloadPacketized {
+		res, err = avcPkt.Unmarshal(largePayloadPacketized[i])
+		if err != nil {
+			t.Fatal(err)
+		}
+		largePayloadResultAVC = append(largePayloadResultAVC, res...)
+	}
+	if !reflect.DeepEqual(largePayloadResultAVC, largepayloadAVC) {
+		t.Fatal("Failed to unmarshal a large payload into avc stream")
+	}
+
 	res, err = pkt.Unmarshal(singlePayloadMultiNALU)
 	if err != nil {
 		t.Fatal(err)
 	} else if !reflect.DeepEqual(res, singlePayloadMultiNALUUnmarshaled) {
 		t.Fatal("Failed to unmarshal a single packet with multiple NALUs")
 	}
+
+	res, err = avcPkt.Unmarshal(singlePayloadMultiNALU)
+	if err != nil {
+		t.Fatal(err)
+	} else if !reflect.DeepEqual(res, singlePayloadMultiNALUUnmarshaledAVC) {
+		t.Fatal("Failed to unmarshal a single packet with multiple NALUs into avc stream")
+	}
 }