Repository: pion/rtp
Pull request #130 (2021-05-17 13:58:41)

Add Z bit to VP9 unmarshaller
Adds the Z flag, which is needed in order to strip the lower spatial layers without lookahead.  I have verified that this is generated correctly by Chromium 90.

The patch depends on a slightly older commit, since I'm unable to update due to #129.

codecov[bot]: # [Codecov](https://codecov.io/gh/pion/rtp/pull/130?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
> Merging [#130](https://codecov.io/gh/pion/rtp/pull/130?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (ba4a8e4) into [master](https://codecov.io/gh/pion/rtp/commit/4e87540a7fe6f31459f720208b36e446a66868d9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (4e87540) will **decrease** coverage by `0.74%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/rtp/pull/130/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/130?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)

```diff
@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
- Coverage   88.97%   88.22%   -0.75%     
==========================================
  Files          13       13              
  Lines         671      671              
==========================================
- Hits          597      592       -5     
- Misses         56       61       +5     
  Partials       18       18              
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `?` | |
| wasm | `88.22% <100.00%> (-0.15%)` | :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://codecov.io/gh/pion/rtp/pull/130?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [codecs/vp9\_packet.go](https://codecov.io/gh/pion/rtp/pull/130/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y29kZWNzL3ZwOV9wYWNrZXQuZ28=) | `92.59% <100.00%> (+0.05%)` | :arrow_up: |
| [packet.go](https://codecov.io/gh/pion/rtp/pull/130/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGFja2V0Lmdv) | `83.17% <0.00%> (-2.48%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/rtp/pull/130?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/130?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Last update [4e87540...ba4a8e4](https://codecov.io/gh/pion/rtp/pull/130?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/.github/assert-contributors.sh b/.github/generate-authors.sh
similarity index 58%
rename from .github/assert-contributors.sh
rename to .github/generate-authors.sh
index 12e6afe..182e4f5 100755
--- a/.github/assert-contributors.sh
+++ b/.github/generate-authors.sh
@@ -12,6 +12,7 @@
 set -e
 
 SCRIPT_PATH=$( cd "$(dirname "${BASH_SOURCE[0]}")" ; pwd -P )
+AUTHORS_PATH="$GITHUB_WORKSPACE/AUTHORS.txt"
 
 if [ -f ${SCRIPT_PATH}/.ci.conf ]
 then
@@ -21,18 +22,18 @@ fi
 #
 # DO NOT EDIT THIS
 #
-EXCLUDED_CONTRIBUTORS+=('John R. Bradley' 'renovate[bot]' 'Renovate Bot' 'Pion Bot')
+EXCLUDED_CONTRIBUTORS+=('John R. Bradley' 'renovate[bot]' 'Renovate Bot' 'Pion Bot' 'pionbot')
 # If you want to exclude a name from all repositories, send a PR to
 # https://github.com/pion/.goassets instead of this repository.
 # If you want to exclude a name only from this repository,
 # add EXCLUDED_CONTRIBUTORS=('name') to .github/.ci.conf
 
-MISSING_CONTRIBUTORS=()
+CONTRIBUTORS=()
 
 shouldBeIncluded () {
 	for i in "${EXCLUDED_CONTRIBUTORS[@]}"
 	do
-		if [ "$i" == "$1" ] ; then
+		if [[ $1 =~ "$i" ]]; then
 			return 1
 		fi
 	done
@@ -41,21 +42,25 @@ shouldBeIncluded () {
 
 
 IFS=$'\n' #Only split on newline
-for contributor in $(git log --format='%aN' | sort -u)
+for contributor in $(git log --format='%aN <%aE>' | LC_ALL=C.UTF-8 sort -uf)
 do
 	if shouldBeIncluded $contributor; then
-		if ! grep -q "$contributor" "$SCRIPT_PATH/../README.md"; then
-			MISSING_CONTRIBUTORS+=("$contributor")
-		fi
+		CONTRIBUTORS+=("$contributor")
 	fi
 done
 unset IFS
 
-if [ ${#MISSING_CONTRIBUTORS[@]} -ne 0 ]; then
-    echo "Please add the following contributors to the README"
-    for i in "${MISSING_CONTRIBUTORS[@]}"
+if [ ${#CONTRIBUTORS[@]} -ne 0 ]; then
+	cat >$AUTHORS_PATH <<-'EOH'
+# Thank you to everyone that made Pion possible. If you are interested in contributing
+# we would love to have you https://github.com/pion/webrtc/wiki/Contributing
+#
+# This file is auto generated, using git to list all individuals contributors.
+# see `.github/generate-authors.sh` for the scripting
+EOH
+    for i in "${CONTRIBUTORS[@]}"
     do
-	    echo "$i"
+	    echo "$i" >> $AUTHORS_PATH
     done
-    exit 1
+    exit 0
 fi
diff --git a/.github/hooks/pre-push.sh b/.github/hooks/pre-push.sh
index 7cb2365..bfe65bc 100755
--- a/.github/hooks/pre-push.sh
+++ b/.github/hooks/pre-push.sh
@@ -8,6 +8,6 @@
 
 set -e
 
-.github/assert-contributors.sh
+.github/generate-authors.sh
 
 exit 0
diff --git a/.github/workflows/generate-authors.yml b/.github/workflows/generate-authors.yml
new file mode 100644
index 0000000..e619025
--- /dev/null
+++ b/.github/workflows/generate-authors.yml
@@ -0,0 +1,62 @@
+name: generate-authors
+
+on:
+  pull_request:
+
+jobs:
+  checksecret:
+    runs-on: ubuntu-latest
+    outputs:
+      is_PIONBOT_PRIVATE_KEY_set: ${{ steps.checksecret_job.outputs.is_PIONBOT_PRIVATE_KEY_set }}
+    steps:
+      - id: checksecret_job
+        env:
+          PIONBOT_PRIVATE_KEY: ${{ secrets.PIONBOT_PRIVATE_KEY }}
+        run: |
+          echo "is_PIONBOT_PRIVATE_KEY_set: ${{ env.PIONBOT_PRIVATE_KEY != '' }}"
+          echo "::set-output name=is_PIONBOT_PRIVATE_KEY_set::${{ env.PIONBOT_PRIVATE_KEY != '' }}"
+
+  generate-authors:
+    needs: [checksecret]
+    if: needs.checksecret.outputs.is_PIONBOT_PRIVATE_KEY_set == 'true'
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+      with:
+        ref: ${{ github.head_ref }}
+        fetch-depth: 0
+        token: ${{ secrets.PIONBOT_PRIVATE_KEY }}
+
+    - name: Generate the authors file
+      run: .github/generate-authors.sh
+
+    - name: Add the authors file to git
+      run: git add AUTHORS.txt
+
+    - name: Get last commit message
+      id: last-commit-message
+      run: |
+        COMMIT_MSG=$(git log -1 --pretty=%B)
+        COMMIT_MSG="${COMMIT_MSG//'%'/'%25'}"
+        COMMIT_MSG="${COMMIT_MSG//$'\n'/'%0A'}"
+        COMMIT_MSG="${COMMIT_MSG//$'\r'/'%0D'}"
+        echo "::set-output name=msg::$COMMIT_MSG"
+
+    - name: Get last commit author
+      id: last-commit-author
+      run: |
+        echo "::set-output name=msg::$(git log -1 --pretty='%aN <%ae>')"
+
+    - name: Check if AUTHORS.txt file has changed
+      id: git-status-output
+      run: |
+        echo "::set-output name=msg::$(git status -s | wc -l)"
+
+    - uses: stefanzweifel/git-auto-commit-action@v4
+      if: ${{ steps.git-status-output.outputs.msg != '0' }}
+      with:
+        commit_message: ${{ steps.last-commit-message.outputs.msg }}
+        commit_author: ${{ steps.last-commit-author.outputs.msg }}
+        commit_options: '--amend --no-edit'
+        push_options: '--force'
+        skip_fetch: true
diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
index 8824c34..bc44c3a 100644
--- a/.github/workflows/lint.yaml
+++ b/.github/workflows/lint.yaml
@@ -22,9 +22,6 @@ jobs:
         - name: File names
           run: .github/lint-filename.sh
 
-        - name: Contributors
-          run: .github/assert-contributors.sh
-
         - name: Functions
           run: .github/lint-disallowed-functions-in-library.sh
 
diff --git a/AUTHORS.txt b/AUTHORS.txt
new file mode 100644
index 0000000..5e5c680
--- /dev/null
+++ b/AUTHORS.txt
@@ -0,0 +1,30 @@
+# Thank you to everyone that made Pion possible. If you are interested in contributing
+# we would love to have you https://github.com/pion/webrtc/wiki/Contributing
+#
+# This file is auto generated, using git to list all individuals contributors.
+# see `.github/generate-authors.sh` for the scripting
+adwpc <adwpc@hotmail.com>
+aler9 <46489434+aler9@users.noreply.github.com>
+Antoine Baché <antoine.bache@epitech.eu>
+Atsushi Watanabe <atsushi.w@ieee.org>
+Bao Nguyen <bao@n4n.dev>
+debiandebiandebian <debiandebiandebiandebian@gmail.com>
+ffmiyo <leffmiyo@gmail.com>
+Guilherme <gqgs@protonmail.com>
+Haiyang Wang <ocean2811@outlook.com>
+Hugo Arregui <hugo@decentraland.org>
+John Bradley <jrb@turrettech.com>
+Juliusz Chroboczek <jch@irif.fr>
+Kazuyuki Honda <hakobera@gmail.com>
+Luke Curley <kixelated@gmail.com>
+lxb <xiangbing.li@uama.com.cn>
+Michael MacDonald <github@macdonald.cx>
+Michael MacDonald <mike.macdonald@savantsystems.com>
+Raphael Derosso Pereira <raphaelpereira@gmail.com>
+Rob Lofthouse <ri.lofthouse@gmail.com>
+Robin Raymond <robin@goheadroom.com>
+Sean DuBois <seaduboi@amazon.com>
+Sean DuBois <sean@siobud.com>
+Simone Gotti <simone.gotti@gmail.com>
+Tarrence van As <tarrence13@gmail.com>
+Woodrow Douglass <wdouglass@carnegierobotics.com>
diff --git a/README.md b/README.md
index 789e9ac..ce04599 100644
--- a/README.md
+++ b/README.md
@@ -30,29 +30,5 @@ If you need commercial support or don't want to use public methods you can conta
 ### Contributing
 Check out the **[contributing wiki](https://github.com/pion/webrtc/wiki/Contributing)** to join the group of amazing people making this project possible:
 
-* [John Bradley](https://github.com/kc5nra) - *Original Author*
-* [Sean DuBois](https://github.com/Sean-Der) - *Original Author*
-* [Woodrow Douglass](https://github.com/wdouglass) *RTCP, RTP improvements, G.722 support, Bugfixes*
-* [Michael MacDonald](https://github.com/mjmac)
-* [Luke Curley](https://github.com/kixelated) *Performance*
-* [Antoine Baché](https://github.com/Antonito) *Fixed crashes*
-* [Hugo Arregui](https://github.com/hugoArregui)
-* [Raphael Derosso Pereira](https://github.com/raphaelpereira)
-* [Atsushi Watanabe](https://github.com/at-wat)
-* [adwpc](https://github.com/adwpc) *add transport-cc extension*
-* [Bao Nguyen](https://github.com/sysbot) *add VP9 noop, bug fixes.
-* [Tarrence van As](https://github.com/tarrencev) *add audio level extension*
-* [Simone Gotti](https://github.com/sgotti)
-* [Guilherme Souza](https://github.com/gqgs)
-* [Rob Lofthouse](https://github.com/roblofthouse)
-* [Kazuyuki Honda](https://github.com/hakobera)
-* [Haiyang Wang](https://github.com/ocean2811)
-* [lxb](https://github.com/lxb531)
-* [Robin Raymond](https://github.com/robin-raymond)
-* [debiandebiandebian](https://github.com/debiandebiandebian)
-* [Juliusz Chroboczek](https://github.com/jech)
-* [ffmiyo](https://github.com/ffmiyo)
-* [Alessandro Ros](https://github.com/aler9)
-
 ### License
 MIT License - see [LICENSE](LICENSE) for full text
diff --git a/codecs/vp8_packet.go b/codecs/vp8_packet.go
index b602ef5..24e7200 100644
--- a/codecs/vp8_packet.go
+++ b/codecs/vp8_packet.go
@@ -2,7 +2,8 @@ package codecs
 
 // VP8Payloader payloads VP8 packets
 type VP8Payloader struct {
-	pictureID uint16
+	EnablePictureID bool
+	pictureID       uint16
 }
 
 const (
@@ -33,12 +34,14 @@ func (p *VP8Payloader) Payload(mtu int, payload []byte) [][]byte {
 	 */
 
 	usingHeaderSize := vp8HeaderSize
-	switch {
-	case p.pictureID == 0:
-	case p.pictureID < 128:
-		usingHeaderSize = vp8HeaderSize + 2
-	default:
-		usingHeaderSize = vp8HeaderSize + 3
+	if p.EnablePictureID {
+		switch {
+		case p.pictureID == 0:
+		case p.pictureID < 128:
+			usingHeaderSize = vp8HeaderSize + 2
+		default:
+			usingHeaderSize = vp8HeaderSize + 3
+		}
 	}
 
 	maxFragmentSize := mtu - usingHeaderSize
@@ -62,17 +65,19 @@ func (p *VP8Payloader) Payload(mtu int, payload []byte) [][]byte {
 			out[0] = 0x10
 			first = false
 		}
-		switch usingHeaderSize {
-		case vp8HeaderSize:
-		case vp8HeaderSize + 2:
-			out[0] |= 0x80
-			out[1] |= 0x80
-			out[2] |= uint8(p.pictureID & 0x7F)
-		case vp8HeaderSize + 3:
-			out[0] |= 0x80
-			out[1] |= 0x80
-			out[2] |= 0x80 | uint8((p.pictureID>>8)&0x7F)
-			out[3] |= uint8(p.pictureID & 0xFF)
+		if p.EnablePictureID {
+			switch usingHeaderSize {
+			case vp8HeaderSize:
+			case vp8HeaderSize + 2:
+				out[0] |= 0x80
+				out[1] |= 0x80
+				out[2] |= uint8(p.pictureID & 0x7F)
+			case vp8HeaderSize + 3:
+				out[0] |= 0x80
+				out[1] |= 0x80
+				out[2] |= 0x80 | uint8((p.pictureID>>8)&0x7F)
+				out[3] |= uint8(p.pictureID & 0xFF)
+			}
 		}
 
 		copy(out[usingHeaderSize:], payloadData[payloadDataIndex:payloadDataIndex+currentFragmentSize])
diff --git a/codecs/vp9_packet.go b/codecs/vp9_packet.go
index 0396554..9d05340 100644
--- a/codecs/vp9_packet.go
+++ b/codecs/vp9_packet.go
@@ -25,12 +25,12 @@ const (
 // 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-10.txt
+	 * https://www.ietf.org/id/draft-ietf-payload-vp9-13.txt
 	 *
 	 * Flexible mode (F=1)
 	 *        0 1 2 3 4 5 6 7
 	 *       +-+-+-+-+-+-+-+-+
-	 *       |I|P|L|F|B|E|V|-| (REQUIRED)
+	 *       |I|P|L|F|B|E|V|Z| (REQUIRED)
 	 *       +-+-+-+-+-+-+-+-+
 	 *  I:   |M| PICTURE ID  | (REQUIRED)
 	 *       +-+-+-+-+-+-+-+-+
@@ -47,7 +47,7 @@ func (p *VP9Payloader) Payload(mtu int, payload []byte) [][]byte {
 	 * Non-flexible mode (F=0)
 	 *        0 1 2 3 4 5 6 7
 	 *       +-+-+-+-+-+-+-+-+
-	 *       |I|P|L|F|B|E|V|-| (REQUIRED)
+	 *       |I|P|L|F|B|E|V|Z| (REQUIRED)
 	 *       +-+-+-+-+-+-+-+-+
 	 *  I:   |M| PICTURE ID  | (RECOMMENDED)
 	 *       +-+-+-+-+-+-+-+-+
@@ -121,6 +121,7 @@ type VP9Packet struct {
 	B bool // Start of a frame
 	E bool // End of a frame
 	V bool // Scalability structure (SS) data present
+	Z bool // Not a reference frame for upper spatial layers
 
 	// Recommended headers
 	PictureID uint16 // 7 or 16 bits, picture ID
@@ -171,6 +172,7 @@ func (p *VP9Packet) Unmarshal(packet []byte) ([]byte, error) {
 	p.B = packet[0]&0x08 != 0
 	p.E = packet[0]&0x04 != 0
 	p.V = packet[0]&0x02 != 0
+	p.Z = packet[0]&0x01 != 0
 
 	pos := 1
 	var err error


Test patch:
diff --git a/codecs/vp8_packet_test.go b/codecs/vp8_packet_test.go
index 1c062c3..37d160d 100644
--- a/codecs/vp8_packet_test.go
+++ b/codecs/vp8_packet_test.go
@@ -2,6 +2,7 @@ package codecs
 
 import (
 	"errors"
+	"reflect"
 	"testing"
 )
 
@@ -109,33 +110,102 @@ func TestVP8Packet_Unmarshal(t *testing.T) {
 }
 
 func TestVP8Payloader_Payload(t *testing.T) {
-	pck := VP8Payloader{}
-	payload := []byte{0x90, 0x90, 0x90}
-
-	// Positive MTU, nil payload
-	res := pck.Payload(1, nil)
-	if len(res) != 0 {
-		t.Fatal("Generated payload should be empty")
-	}
-
-	// Positive MTU, small payload
-	// MTU of 1 results in fragment size of 0
-	res = pck.Payload(1, payload)
-	if len(res) != 0 {
-		t.Fatal("Generated payload should be empty")
-	}
+	testCases := map[string]struct {
+		payloader VP8Payloader
+		mtu       int
+		payload   [][]byte
+		expected  [][][]byte
+	}{
+		"WithoutPictureID": {
+			payloader: VP8Payloader{},
+			mtu:       2,
+			payload: [][]byte{
+				{0x90, 0x90, 0x90},
+				{0x91, 0x91},
+			},
+			expected: [][][]byte{
+				{{0x10, 0x90}, {0x00, 0x90}, {0x00, 0x90}},
+				{{0x10, 0x91}, {0x00, 0x91}},
+			},
+		},
+		"WithPictureID_1byte": {
+			payloader: VP8Payloader{
+				EnablePictureID: true,
+				pictureID:       0x20,
+			},
+			mtu: 5,
+			payload: [][]byte{
+				{0x90, 0x90, 0x90},
+				{0x91, 0x91},
+			},
+			expected: [][][]byte{
+				{
+					{0x90, 0x80, 0x20, 0x90, 0x90},
+					{0x80, 0x80, 0x20, 0x90},
+				},
+				{
+					{0x90, 0x80, 0x21, 0x91, 0x91},
+				},
+			},
+		},
+		"WithPictureID_2bytes": {
+			payloader: VP8Payloader{
+				EnablePictureID: true,
+				pictureID:       0x120,
+			},
+			mtu: 6,
+			payload: [][]byte{
+				{0x90, 0x90, 0x90},
+				{0x91, 0x91},
+			},
+			expected: [][][]byte{
+				{
+					{0x90, 0x80, 0x81, 0x20, 0x90, 0x90},
+					{0x80, 0x80, 0x81, 0x20, 0x90},
+				},
+				{
+					{0x90, 0x80, 0x81, 0x21, 0x91, 0x91},
+				},
+			},
+		},
+	}
+	for name, testCase := range testCases {
+		testCase := testCase
+		t.Run(name, func(t *testing.T) {
+			pck := testCase.payloader
+
+			for i := range testCase.payload {
+				res := pck.Payload(testCase.mtu, testCase.payload[i])
+				if !reflect.DeepEqual(testCase.expected[i], res) {
+					t.Fatalf("Generated packet[%d] differs, expected: %v, got: %v", i, testCase.expected[i], res)
+				}
+			}
+		})
+	}
+
+	t.Run("Error", func(t *testing.T) {
+		pck := VP8Payloader{}
+		payload := []byte{0x90, 0x90, 0x90}
+
+		// Positive MTU, nil payload
+		res := pck.Payload(1, nil)
+		if len(res) != 0 {
+			t.Fatal("Generated payload should be empty")
+		}
 
-	// Negative MTU, small payload
-	res = pck.Payload(-1, payload)
-	if len(res) != 0 {
-		t.Fatal("Generated payload should be empty")
-	}
+		// Positive MTU, small payload
+		// MTU of 1 results in fragment size of 0
+		res = pck.Payload(1, payload)
+		if len(res) != 0 {
+			t.Fatal("Generated payload should be empty")
+		}
 
-	// Positive MTU, small payload
-	res = pck.Payload(2, payload)
-	if len(res) != len(payload) {
-		t.Fatal("Generated payload should be the same size as original payload size")
-	}
+		// Negative MTU, small payload
+		res = pck.Payload(-1, payload)
+		if len(res) != 0 {
+			t.Fatal("Generated payload should be empty")
+		}
+	})
 }
 
 func TestVP8PartitionHeadChecker_IsPartitionHead(t *testing.T) {