Conversation
alan-george-lk
left a comment
alan-george-lk
left a comment
There was a problem hiding this comment.
This was more of a rubber stamp since I don't know much about h26x.rs, but I like the modularity!
|
|
||
| //! H.264/H.265 parsing helpers: NAL-unit splitting, access-unit assembly, | ||
| //! and keyframe detection. | ||
|
|
There was a problem hiding this comment.
This file is pretty large (1K+). Consider splitting up if it makes sense to
| }) | ||
| } | ||
|
|
||
| fn split_start_code_index(bytes: &[u8], nal_start: usize) -> Result<usize, H26xParseError> { |
There was a problem hiding this comment.
q: i see utility functions in rust getting unit tested much less than we do in cpp. Is this due the nature of anything rust specific? or maybe should we beef up testing on this?
af01f4b to
f2ea3fb
Compare
Changeset incompleteThis PR's changeset is missing version bumps for packages that are affected by the change. The following packages still require a bump:
Already covered:
A package must be bumped when its own files change, and whenever a package it depends on is bumped (so downstream consumers get a matching release). Click here to create a changeset for the missing packages The link pre-populates a changeset file with If this change doesn't require a version bump, add the |
f2ea3fb to
387d4a2
Compare
387d4a2 to
6359891
Compare
6359891 to
dea4286
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| // A VCL NAL opens a new picture when first_mb_in_slice == 0: | ||
| // ue(v) == 0 is a lone 1 bit, so the first RBSP bit after the | ||
| // header is set. The header byte is nonzero, so the next byte | ||
| // cannot be an emulation-prevention byte. | ||
| 1..=5 => nal.len() >= 2 && nal[1] & 0x80 != 0, |
There was a problem hiding this comment.
🟡 Partitioned H.264 pictures split apart
starts_new_access_unit reads partition B/C payload bits as first_mb_in_slice. A set first bit splits one partitioned picture into invalid frames.
Prompt for agents
Fix H.264 access-unit boundary detection for data-partitioned streams in livekit-capture/src/encoded/h26x.rs. starts_new_access_unit currently interprets byte 1 as first_mb_in_slice for NAL types 1 through 5, but types 3 and 4 carry data partitions B and C and begin with slice_id instead. Parse boundaries only from NAL types that contain a slice header, and keep the associated B/C partitions with partition A. Add a test containing type 2 followed by type 3 and type 4 NAL units whose slice_id starts with a set bit, followed by the next picture.
Was this helpful? React with 👍 or 👎 to provide feedback.
c169311 to
fb4530d
Compare
fb4530d to
79c6537
Compare
79c6537 to
7e80a34
Compare
7e80a34 to
deb0ea0
Compare
deb0ea0 to
f7ea43c
Compare
f7ea43c to
ebee3a9
Compare
3 participants
Add H.264 and H.265 access unit parsing to support GStreamer source (and potentially others later).
Closes BOT-550