Repository: pion/rtp
Pull request #74 (2020-07-12 16:01:09)

Support VP9 Scalability Structure (SS)
#### Description

Add support for parsing VP9 scalability structure to avoid error
when a packet contains scalability structure (V=1).

#### Reference issue
None

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

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

```diff
@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
+ Coverage   90.99%   91.17%   +0.17%     
==========================================
  Files          13       13              
  Lines         622      680      +58     
==========================================
+ Hits          566      620      +54     
- Misses         40       42       +2     
- Partials       16       18       +2     
```

| Flag | Coverage Δ | |
|---|---|---|
| #go | `91.17% <93.47%> (+0.17%)` | :arrow_up: |
| #wasm | `91.17% <93.47%> (+0.17%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pion/rtp/pull/74?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [codecs/vp9\_packet.go](https://codecov.io/gh/pion/rtp/pull/74/diff?src=pr&el=tree#diff-Y29kZWNzL3ZwOV9wYWNrZXQuZ28=) | `94.52% <93.47%> (-0.94%)` | :arrow_down: |

------

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

Sean-Der: The PR looks great to me! Thank you so much for the contribution.

Great to see you here @hakobera if you are interested I would love to have you more involved in pion. We need all the help we can get :)



Patch:
diff --git a/README.md b/README.md
index dfdded4..cabd3c3 100644
--- a/README.md
+++ b/README.md
@@ -45,6 +45,7 @@ Check out the **[contributing wiki](https://github.com/pion/webrtc/wiki/Contribu
 * [Simone Gotti](https://github.com/sgotti)
 * [Guilherme Souza](https://github.com/gqgs)
 * [Rob Lofthouse](https://github.com/roblofthouse)
+* [Kazuyuki Honda](https://github.com/hakobera)
 
 ### License
 MIT License - see [LICENSE](LICENSE) for full text
diff --git a/codecs/vp9_packet.go b/codecs/vp9_packet.go
index b7e07a3..642d7bf 100644
--- a/codecs/vp9_packet.go
+++ b/codecs/vp9_packet.go
@@ -14,13 +14,15 @@ type VP9Payloader struct {
 }
 
 const (
-	vp9HeaderSize = 3 // Flexible mode 15 bit picture ID
+	vp9HeaderSize    = 3 // Flexible mode 15 bit picture ID
+	maxSpatialLayers = 5
+	maxVP9RefPics    = 3
 )
 
 // Payload fragments an VP9 packet across one or more byte arrays
 func (p *VP9Payloader) Payload(mtu int, payload []byte) [][]byte {
 	/*
-	 * https://www.ietf.org/id/draft-ietf-payload-vp9-09.txt
+	 * https://www.ietf.org/id/draft-ietf-payload-vp9-10.txt
 	 *
 	 * Flexible mode (F=1)
 	 *        0 1 2 3 4 5 6 7
@@ -128,6 +130,17 @@ type VP9Packet struct {
 	PDiff     []uint8 // Reference index (F=1)
 	TL0PICIDX uint8   // Temporal layer zero index (F=0)
 
+	// Scalability structure headers
+	NS      uint8 // N_S + 1 indicates the number of spatial layers present in the VP9 stream
+	Y       bool  // Each spatial layer's frame resolution present
+	G       bool  // PG description present flag.
+	NG      uint8 // N_G indicates the number of pictures in a Picture Group (PG)
+	Width   []uint16
+	Height  []uint16
+	PGTID   []uint8   // Temporal layer ID of pictures in a Picture Group
+	PGU     []bool    // Switching up point of pictures in a Picture Group
+	PGPDiff [][]uint8 // Reference indecies of pictures in a Picture Group
+
 	Payload []byte
 }
 
@@ -148,68 +161,210 @@ func (p *VP9Packet) Unmarshal(packet []byte) ([]byte, error) {
 	p.E = packet[0]&0x04 != 0
 	p.V = packet[0]&0x02 != 0
 
+	pos := 1
+	var err error
+
+	if p.I {
+		pos, err = p.parsePictureID(packet, pos)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	if p.L {
+		pos, err = p.parseLayerInfo(packet, pos)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	if p.F && p.P {
+		pos, err = p.parseRefIndices(packet, pos)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	if p.V {
-		return nil, errors.New("scalability structure is not yet implemented")
+		pos, err = p.parseSSData(packet, pos)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	pos := 1
+	p.Payload = packet[pos:]
+	return p.Payload, nil
+}
 
-	// if p.F && !p.I { // It's out of the standard but still possible to unmarshal
-	// 	return nil, errors.New("picture ID is required but not present")
-	// }
+// Picture ID:
+//
+//      +-+-+-+-+-+-+-+-+
+// I:   |M| PICTURE ID  |   M:0 => picture id is 7 bits.
+//      +-+-+-+-+-+-+-+-+   M:1 => picture id is 15 bits.
+// M:   | EXTENDED PID  |
+//      +-+-+-+-+-+-+-+-+
+//
+func (p *VP9Packet) parsePictureID(packet []byte, pos int) (int, error) {
+	if len(packet) <= pos {
+		return pos, errShortPacket
+	}
 
-	if p.I {
+	p.PictureID = uint16(packet[pos] & 0x7F)
+	if packet[pos]&0x80 != 0 {
+		pos++
 		if len(packet) <= pos {
-			return nil, errShortPacket
-		}
-		p.PictureID = uint16(packet[pos] & 0x7F)
-		if packet[pos]&0x80 != 0 {
-			pos++
-			if len(packet) <= pos {
-				return nil, errShortPacket
-			}
-			p.PictureID = p.PictureID<<8 | uint16(packet[pos])
+			return pos, errShortPacket
 		}
-		pos++
+		p.PictureID = p.PictureID<<8 | uint16(packet[pos])
 	}
+	pos++
+	return pos, nil
+}
 
-	if p.L {
+func (p *VP9Packet) parseLayerInfo(packet []byte, pos int) (int, error) {
+	pos, err := p.parseLayerInfoCommon(packet, pos)
+	if err != nil {
+		return pos, err
+	}
+
+	if p.F {
+		return pos, nil
+	}
+
+	return p.parseLayerInfoNonFlexibleMode(packet, pos)
+}
+
+// Layer indices (flexible mode):
+//
+//      +-+-+-+-+-+-+-+-+
+// L:   |  T  |U|  S  |D|
+//      +-+-+-+-+-+-+-+-+
+//
+func (p *VP9Packet) parseLayerInfoCommon(packet []byte, pos int) (int, error) {
+	if len(packet) <= pos {
+		return pos, errShortPacket
+	}
+
+	p.TID = packet[pos] >> 5
+	p.U = packet[pos]&0x10 != 0
+	p.SID = (packet[pos] >> 1) & 0x7
+	p.D = packet[pos]&0x01 != 0
+
+	if p.SID >= maxSpatialLayers {
+		return pos, errors.New("too many spatial layers")
+	}
+
+	pos++
+	return pos, nil
+}
+
+// Layer indices (non-flexible mode):
+//
+//      +-+-+-+-+-+-+-+-+
+// L:   |  T  |U|  S  |D|
+//      +-+-+-+-+-+-+-+-+
+//      |   TL0PICIDX   |
+//      +-+-+-+-+-+-+-+-+
+//
+func (p *VP9Packet) parseLayerInfoNonFlexibleMode(packet []byte, pos int) (int, error) {
+	if len(packet) <= pos {
+		return pos, errShortPacket
+	}
+
+	p.TL0PICIDX = packet[pos]
+	pos++
+	return pos, nil
+}
+
+// Reference indices:
+//
+//      +-+-+-+-+-+-+-+-+                P=1,F=1: At least one reference index
+// P,F: | P_DIFF      |N|  up to 3 times          has to be specified.
+//      +-+-+-+-+-+-+-+-+                    N=1: An additional P_DIFF follows
+//                                                current P_DIFF.
+//
+func (p *VP9Packet) parseRefIndices(packet []byte, pos int) (int, error) {
+	for {
 		if len(packet) <= pos {
-			return nil, errShortPacket
+			return pos, errShortPacket
+		}
+		p.PDiff = append(p.PDiff, packet[pos]>>1)
+		if packet[pos]&0x01 == 0 {
+			break
+		}
+		if len(p.PDiff) >= maxVP9RefPics {
+			return pos, errTooManyPDiff
 		}
-		p.TID = packet[pos] >> 5
-		p.U = packet[pos]&0x10 != 0
-		p.SID = (packet[pos] >> 1) & 0x7
-		p.D = packet[pos]&0x01 != 0
 		pos++
-		if !p.F {
-			if len(packet) <= pos {
-				return nil, errShortPacket
-			}
-			p.TL0PICIDX = packet[pos]
-			pos++
+	}
+	pos++
+
+	return pos, nil
+}
+
+// Scalability structure (SS):
+//
+//      +-+-+-+-+-+-+-+-+
+// V:   | N_S |Y|G|-|-|-|
+//      +-+-+-+-+-+-+-+-+              -|
+// Y:   |     WIDTH     | (OPTIONAL)    .
+//      +               +               .
+//      |               | (OPTIONAL)    .
+//      +-+-+-+-+-+-+-+-+               . N_S + 1 times
+//      |     HEIGHT    | (OPTIONAL)    .
+//      +               +               .
+//      |               | (OPTIONAL)    .
+//      +-+-+-+-+-+-+-+-+              -|
+// G:   |      N_G      | (OPTIONAL)
+//      +-+-+-+-+-+-+-+-+                           -|
+// N_G: |  T  |U| R |-|-| (OPTIONAL)                 .
+//      +-+-+-+-+-+-+-+-+              -|            . N_G times
+//      |    P_DIFF     | (OPTIONAL)    . R times    .
+//      +-+-+-+-+-+-+-+-+              -|           -|
+//
+func (p *VP9Packet) parseSSData(packet []byte, pos int) (int, error) {
+	if len(packet) <= pos {
+		return pos, errShortPacket
+	}
+
+	p.NS = packet[pos] >> 5
+	p.Y = packet[pos]&0x10 != 0
+	p.G = (packet[pos]>>1)&0x7 != 0
+	pos++
+
+	NS := p.NS + 1
+	p.NG = 0
+
+	if p.Y {
+		p.Width = make([]uint16, NS)
+		p.Height = make([]uint16, NS)
+		for i := 0; i < int(NS); i++ {
+			p.Width[i] = uint16(packet[pos])<<8 | uint16(packet[pos+1])
+			pos += 2
+			p.Height[i] = uint16(packet[pos])<<8 | uint16(packet[pos+1])
+			pos += 2
 		}
 	}
 
-	if p.F && p.P {
-		for {
-			if len(packet) <= pos {
-				return nil, errShortPacket
-			}
-			p.PDiff = append(p.PDiff, packet[pos]>>1)
-			if packet[pos]&0x01 == 0 {
-				break
-			}
-			if len(p.PDiff) >= 3 {
-				return nil, errTooManyPDiff
-			}
+	if p.G {
+		p.NG = packet[pos]
+		pos++
+	}
+
+	for i := 0; i < int(p.NG); i++ {
+		p.PGTID = append(p.PGTID, packet[pos]>>5)
+		p.PGU = append(p.PGU, packet[pos]&0x10 != 0)
+		R := (packet[pos] >> 2) & 0x3
+		pos++
+
+		p.PGPDiff = append(p.PGPDiff, []uint8{})
+		for j := 0; j < int(R); j++ {
+			p.PGPDiff[i] = append(p.PGPDiff[i], packet[pos])
 			pos++
 		}
-		pos++
 	}
 
-	p.Payload = packet[pos:]
-	return p.Payload, nil
+	return pos, nil
 }
 
 // VP9PartitionHeadChecker checks VP9 partition head


Test patch:
diff --git a/codecs/vp9_packet_test.go b/codecs/vp9_packet_test.go
index 9d303f7..41f66cc 100644
--- a/codecs/vp9_packet_test.go
+++ b/codecs/vp9_packet_test.go
@@ -123,6 +123,49 @@ func TestVP9Packet_Unmarshal(t *testing.T) {
 			b:   []byte{0xD0},
 			err: errShortPacket,
 		},
+		"ScalabilityStructureResolutionsNoPayload": {
+			b: []byte{
+				0x0A,
+				(1 << 5) | (1 << 4) | 0, // NS:1 Y:1 G:0
+				640 >> 8, 640 & 0xff,
+				360 >> 8, 360 & 0xff,
+				1280 >> 8, 1280 & 0xff,
+				720 >> 8, 720 & 0xff,
+			},
+			pkt: VP9Packet{
+				B:       true,
+				V:       true,
+				NS:      1,
+				Y:       true,
+				G:       false,
+				NG:      0,
+				Width:   []uint16{640, 1280},
+				Height:  []uint16{360, 720},
+				Payload: []byte{},
+			},
+		},
+		"ScalabilityStructureNoPayload": {
+			b: []byte{
+				0x0A,
+				(1 << 5) | (0 << 4) | (1 << 3), // NS:1 Y:0 G:1
+				2,
+				(0 << 5) | (1 << 4) | (0 << 2) | 0, // T:0 U:1 R:0 -
+				(2 << 5) | (0 << 4) | (1 << 2) | 0, // T:2 U:0 R:1 -
+				33,
+			},
+			pkt: VP9Packet{
+				B:       true,
+				V:       true,
+				NS:      1,
+				Y:       false,
+				G:       true,
+				NG:      2,
+				PGTID:   []uint8{0, 2},
+				PGU:     []bool{true, false},
+				PGPDiff: [][]uint8{{}, {33}},
+				Payload: []byte{},
+			},
+		},
 	}
 	for name, c := range cases {
 		c := c