Repository: pion/rtp
Pull request #68 (2020-05-19 13:52:18)

Fix AudioLevel Extension
ID/len are handled by rtp package header extension api now


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

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

```diff
@@            Coverage Diff             @@
##           master      #68      +/-   ##
==========================================
- Coverage   90.85%   90.67%   -0.18%     
==========================================
  Files          13       13              
  Lines         634      622      -12     
==========================================
- Hits          576      564      -12     
  Misses         41       41              
  Partials       17       17              
```

| Flag | Coverage Δ | |
|---|---|---|
| #go | `90.67% <100.00%> (-0.18%)` | :arrow_down: |
| #wasm | `?` | |

| [Impacted Files](https://codecov.io/gh/pion/rtp/pull/68?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [audiolevelextension.go](https://codecov.io/gh/pion/rtp/pull/68/diff?src=pr&el=tree#diff-YXVkaW9sZXZlbGV4dGVuc2lvbi5nbw==) | `100.00% <100.00%> (ø)` | |

------

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


Patch:
diff --git a/audiolevelextension.go b/audiolevelextension.go
index f85f6b0..d0ba8b4 100644
--- a/audiolevelextension.go
+++ b/audiolevelextension.go
@@ -5,16 +5,12 @@ import (
 )
 
 const (
-	// AudioLevelOneByteExtensionSize One byte header size
-	AudioLevelOneByteExtensionSize = 2
-	// AudioLevelTwoByteExtensionSize Two byte header size
-	AudioLevelTwoByteExtensionSize = 4
+	// audioLevelExtensionSize One byte header size
+	audioLevelExtensionSize = 1
 )
 
 var (
-	errInvalidSize           = errors.New("invalid buffer size")
-	errInvalidExtensonLength = errors.New("invalid extension length")
-	errAudioLevelOverflow    = errors.New("audio level overflow")
+	errAudioLevelOverflow = errors.New("audio level overflow")
 )
 
 // AudioLevelExtension is a extension payload format described in
@@ -37,7 +33,6 @@ var (
 // |      ID       |     len=1     |V|    level    |    0 (pad)    |
 // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 type AudioLevelExtension struct {
-	ID    uint8
 	Level uint8
 	Voice bool
 }
@@ -51,32 +46,17 @@ func (a *AudioLevelExtension) Marshal() ([]byte, error) {
 	if a.Voice {
 		voice = 0x80
 	}
-	buf := make([]byte, AudioLevelOneByteExtensionSize)
-	buf[0] = a.ID << 4 & 0xf0
-	buf[1] = voice | a.Level
+	buf := make([]byte, audioLevelExtensionSize)
+	buf[0] = voice | a.Level
 	return buf, nil
 }
 
 // Unmarshal parses the passed byte slice and stores the result in the members
 func (a *AudioLevelExtension) Unmarshal(rawData []byte) error {
-	// one byte format
-	switch len(rawData) {
-	case AudioLevelOneByteExtensionSize:
-		if rawData[0]&^0xF0 != 0 {
-			return errInvalidExtensonLength
-		}
-		a.ID = rawData[0] >> 4
-		a.Level = rawData[1] & 0x7F
-		a.Voice = rawData[1]&0x80 != 0
-		return nil
-	case AudioLevelTwoByteExtensionSize:
-		if rawData[1] != 1 {
-			return errInvalidExtensonLength
-		}
-		a.ID = rawData[0]
-		a.Level = rawData[2] & 0x7F
-		a.Voice = rawData[2]&0x80 != 0
-		return nil
+	if len(rawData) < audioLevelExtensionSize {
+		return errTooSmall
 	}
-	return errInvalidSize
+	a.Level = rawData[0] & 0x7F
+	a.Voice = rawData[0]&0x80 != 0
+	return nil
 }


Test patch:
diff --git a/audiolevelextension_test.go b/audiolevelextension_test.go
index 0314eb2..13d553f 100644
--- a/audiolevelextension_test.go
+++ b/audiolevelextension_test.go
@@ -10,52 +10,16 @@ func TestAudioLevelExtensionTooSmall(t *testing.T) {
 
 	rawData := []byte{}
 
-	if err := a.Unmarshal(rawData); err != errInvalidSize {
-		t.Fatal("err != errInvalidSize")
+	if err := a.Unmarshal(rawData); err != errTooSmall {
+		t.Fatal("err != errTooSmall")
 	}
 }
 
-func TestAudioLevelExtensionTooBig(t *testing.T) {
-	a := AudioLevelExtension{}
-
-	rawData := []byte{
-		0x00, 0x00, 0x00, 0x00, 0x00,
-	}
-
-	if err := a.Unmarshal(rawData); err != errInvalidSize {
-		t.Fatal("err != errInvalidSize")
-	}
-}
-
-func TestAudioLevelOneByteExtensionInvalidLength(t *testing.T) {
-	a := AudioLevelExtension{}
-
-	rawData := []byte{
-		0x31, 0x88,
-	}
-
-	if err := a.Unmarshal(rawData); err != errInvalidExtensonLength {
-		t.Fatal("err != errInvalidExtensonLength")
-	}
-}
-
-func TestAudioLevelTwoByteExtensionInvalidLength(t *testing.T) {
-	a := AudioLevelExtension{}
-
-	rawData := []byte{
-		0x30, 0x00, 0x00, 0x00,
-	}
-
-	if err := a.Unmarshal(rawData); err != errInvalidExtensonLength {
-		t.Fatal("err != errInvalidExtensonLength")
-	}
-}
-
-func TestAudioLevelOneByteExtensionVoiceTrue(t *testing.T) {
+func TestAudioLevelExtensionVoiceTrue(t *testing.T) {
 	a1 := AudioLevelExtension{}
 
 	rawData := []byte{
-		0x30, 0x88,
+		0x88,
 	}
 
 	if err := a1.Unmarshal(rawData); err != nil {
@@ -63,7 +27,6 @@ func TestAudioLevelOneByteExtensionVoiceTrue(t *testing.T) {
 	}
 
 	a2 := AudioLevelExtension{
-		ID:    3,
 		Level: 8,
 		Voice: true,
 	}
@@ -78,11 +41,11 @@ func TestAudioLevelOneByteExtensionVoiceTrue(t *testing.T) {
 	}
 }
 
-func TestAudioLevelOneByteExtensionVoiceFalse(t *testing.T) {
+func TestAudioLevelExtensionVoiceFalse(t *testing.T) {
 	a1 := AudioLevelExtension{}
 
 	rawData := []byte{
-		0x30, 0x8,
+		0x8,
 	}
 
 	if err := a1.Unmarshal(rawData); err != nil {
@@ -90,7 +53,6 @@ func TestAudioLevelOneByteExtensionVoiceFalse(t *testing.T) {
 	}
 
 	a2 := AudioLevelExtension{
-		ID:    3,
 		Level: 8,
 		Voice: false,
 	}
@@ -105,9 +67,8 @@ func TestAudioLevelOneByteExtensionVoiceFalse(t *testing.T) {
 	}
 }
 
-func TestAudioLevelOneByteExtensionLevelOverflow(t *testing.T) {
+func TestAudioLevelExtensionLevelOverflow(t *testing.T) {
 	a := AudioLevelExtension{
-		ID:    3,
 		Level: 128,
 		Voice: false,
 	}
@@ -116,34 +77,3 @@ func TestAudioLevelOneByteExtensionLevelOverflow(t *testing.T) {
 		t.Fatal("err != errAudioLevelOverflow")
 	}
 }
-
-func TestAudioLevelTwoByteExtensionVoiceFalse(t *testing.T) {
-	a1 := AudioLevelExtension{}
-
-	oneByteRawData := []byte{
-		0x30, 0x8,
-	}
-
-	twoByteRawData := []byte{
-		0x3, 0x1, 0x8, 0x0,
-	}
-
-	if err := a1.Unmarshal(twoByteRawData); err != nil {
-		t.Fatal("Unmarshal error on extension data")
-	}
-
-	a2 := AudioLevelExtension{
-		ID:    3,
-		Level: 8,
-		Voice: false,
-	}
-
-	if a1 != a2 {
-		t.Error("Unmarshal failed")
-	}
-
-	dstData, _ := a2.Marshal()
-	if !bytes.Equal(dstData, oneByteRawData) {
-		t.Error("Marshal failed")
-	}
-}