Fix nalu parsing for 3-byte & 4-byte start seqs by rileym-arcules · Pull Request #301 · pion/rtp

rileym-arcules · 2025-03-10T23:09:00Z

Description

There is an issue in the current emitNalu function where the implementation can skip over 3-byte start sequences.

The emitNalu function was changed in commit bfe92b9 which introduced this regression: bfe92b9#diff-5eca440e933baa4c5491dcd9ccc7c1f4d807994e6c5b54c304228c34fcc3c963L40

From analysis of the current emitNalu function,

rtp/codecs/h264_packet.go

Lines 45 to 67 in ee5524b

    
           func emitNalus(nals []byte, emit func([]byte)) { 
        
           	start := 0 
        
           	length := len(nals) 
        
           	for start < length { 
        
           		end := bytes.Index(nals[start:], annexbNALUStartCode) 
        
           		offset := 4 
        
           		if end == -1 { 
        
           			end = bytes.Index(nals[start:], naluStartCode) 
        
           			offset = 3 
        
           		} 
        
           		if end == -1 { 
        
           			emit(nals[start:]) 
        
           			break 
        
           		} 
        
           		emit(nals[start : start+end]) 
        
           		// next NAL start position 
        
           		start += end + offset 
        
           	} 
        
           }

one can observe that NALUs with 3-byte start sequences may be skipped over if it is the first NALU, or if it is between two NALUs with 4-byte start sequences (in this case: at L52 end != -1)

This PR aims to resolve this issue by searching for 3-byte start sequences and accounting for 4-byte sequences afterward.

codecs/h264_packet_test.go

codecov · 2025-03-10T23:26:50Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.76%. Comparing base (ee5524b) to head (5662773).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #301      +/-   ##
==========================================
+ Coverage   86.69%   86.76%   +0.07%     
==========================================
  Files          26       26              
  Lines        3043     3061      +18     
==========================================
+ Hits         2638     2656      +18     
  Misses        348      348              
  Partials       57       57

Flag	Coverage Δ
go	`86.76% <100.00%> (+0.07%)`	⬆️
wasm	`86.21% <100.00%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

rileym-arcules commented Mar 10, 2025

View reviewed changes

codecs/h264_packet_test.go Show resolved Hide resolved

Fix nalu parsing for 3-byte & 4-byte start seqs

5662773

Sean-Der merged commit 9bbc0f6 into pion:master Mar 11, 2025
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix nalu parsing for 3-byte & 4-byte start seqs#301

Fix nalu parsing for 3-byte & 4-byte start seqs#301
Sean-Der merged 1 commit intopion:masterfrom
rileym-arcules:rileym/nalu-parsing-fix

rileym-arcules commented Mar 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

codecov bot commented Mar 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	func emitNalus(nals []byte, emit func([]byte)) {
	start := 0
	length := len(nals)

	for start < length {
	end := bytes.Index(nals[start:], annexbNALUStartCode)
	offset := 4
	if end == -1 {
	end = bytes.Index(nals[start:], naluStartCode)
	offset = 3
	}
	if end == -1 {
	emit(nals[start:])

	break
	}

	emit(nals[start : start+end])

	// next NAL start position
	start += end + offset
	}
	}

Conversation

rileym-arcules commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Uh oh!

codecov bot commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rileym-arcules commented Mar 10, 2025 •

edited

Loading

codecov bot commented Mar 10, 2025 •

edited

Loading