Repository: pion/rtp
Pull request #213 (2023-02-02 09:03:53)

fix the bug that caused `packet is not large enough: 1 <= 2` in the End of sequence NALU
#### Description
When I Unmarshal(`codecs.H264Packet.Unmarshal`)  packets sent by `codecs.H264Payloader` , I get `packet is not large enough: 1 <= 2`.


- version: [v1.7.13](https://github.com/pion/rtp/releases/tag/v1.7.13)

#### Cause
I checked the packet and found that the H.264 NALU type was the End of sequence (NALU type:10).
I looked into the End of sequence and found that the size of that NALU is only the NALU header and the rbsp is empty.
Please see [hear 7.3.2.5. End of sequence RBSP syntax](https://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=&ved=2ahUKEwj3_rPGu_b8AhWVHHAKHe_MCZgQFnoECAkQAQ&url=https%3A%2F%2Fwww.itu.int%2Frec%2Fdologin_pub.asp%3Flang%3De%26id%3DT-REC-H.264-201304-S!!PDF-E%26type%3Ditems&usg=AOvVaw35JIsiAjAhfgPFgHaI5DOk) for more information on the End of sequence.
<img width="860" alt="スクリーンショット 2023-02-04 23 15 40" src="https://user-images.githubusercontent.com/1884515/216804426-758a922d-5f42-4883-a292-3bdc8be11e64.png">

The Unmarshall makes an error if the size is less than 1, but the End of sequence has a size of 1.

#### Proposed Correction
Remove checks such as payload size less than 2.

Only FU-A explicitly accesses the second byte, which is not affected because a separate boundary value check is performed.



codecov[bot]: # [Codecov](https://codecov.io/gh/pion/rtp/pull/213?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
Base: **87.43**% // Head: **87.58**% // Increases project coverage by **`+0.14%`** :tada:
> Coverage data is based on head [(`1eec7e3`)](https://codecov.io/gh/pion/rtp/pull/213?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) compared to base [(`cf0f407`)](https://codecov.io/gh/pion/rtp/commit/cf0f4079f8a0ed6599d33298112356020f5f1e01?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).
> Patch has no changes to coverable lines.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #213      +/-   ##
==========================================
+ Coverage   87.43%   87.58%   +0.14%     
==========================================
  Files          20       20              
  Lines        1854     1852       -2     
==========================================
+ Hits         1621     1622       +1     
+ Misses        201      199       -2     
+ Partials       32       31       -1     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `87.58% <ø> (+0.14%)` | :arrow_up: |
| wasm | `86.60% <ø> (+0.14%)` | :arrow_up: |

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/213?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [codecs/h264\_packet.go](https://codecov.io/gh/pion/rtp/pull/213?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y29kZWNzL2gyNjRfcGFja2V0Lmdv) | `98.93% <ø> (+1.56%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/pion/rtp/pull/213?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).

aler9: Hello, i can confirm that i have also dealed with NALUS that are 1 byte in size. 
aler9: now you've got to fix the lint error, otherwise i can't merge the patch.

Patch:
diff --git a/codecs/h264_packet.go b/codecs/h264_packet.go
index 11a82fe..3eda4de 100644
--- a/codecs/h264_packet.go
+++ b/codecs/h264_packet.go
@@ -211,10 +211,8 @@ func (p *H264Packet) IsDetectedFinalPacketInSequence(rtpPacketMarketBit bool) bo
 
 // Unmarshal parses the passed byte slice and stores the result in the H264Packet this method is called upon
 func (p *H264Packet) Unmarshal(payload []byte) ([]byte, error) {
-	if payload == nil {
-		return nil, errNilPacket
-	} else if len(payload) <= 2 {
-		return nil, fmt.Errorf("%w: %d <= 2", errShortPacket, len(payload))
+	if len(payload) == 0 {
+		return nil, fmt.Errorf("%w: %d <=0", errShortPacket, len(payload))
 	}
 
 	// NALU Types


Test patch:
diff --git a/codecs/h264_packet_test.go b/codecs/h264_packet_test.go
index 47826d7..fd77a3e 100644
--- a/codecs/h264_packet_test.go
+++ b/codecs/h264_packet_test.go
@@ -115,8 +115,16 @@ func TestH264Packet_Unmarshal(t *testing.T) {
 		t.Fatal("Unmarshal did not fail on nil payload")
 	}
 
-	if _, err := pkt.Unmarshal([]byte{0x00, 0x00}); err == nil {
-		t.Fatal("Unmarshal accepted a packet that is too small for a payload and header")
+	if _, err := pkt.Unmarshal([]byte{}); err == nil {
+		t.Fatal("Unmarshal did not fail on []byte{}")
+	}
+
+	if _, err := pkt.Unmarshal([]byte{0xFC}); err == nil {
+		t.Fatal("Unmarshal accepted a FU-A packet that is too small for a payload and header")
+	}
+
+	if _, err := pkt.Unmarshal([]byte{0x0A}); err != nil {
+		t.Fatal("Unmarshaling end of sequence(NALU Type : 10) should succeed")
 	}
 
 	if _, err := pkt.Unmarshal([]byte{0xFF, 0x00, 0x00}); err == nil {