Disregard SPS and PPS NALUs when there is no slice [chromium/src : main]
0 views
Skip to first unread message
Ted (Chromium) Meyer (Gerrit)
Jul 7, 2025, 9:26:15 PM7/7/25
to Dale Curtis, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Dale Curtis
Ted (Chromium) Meyer added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Ted (Chromium) Meyer . resolved
I'm having trouble generating a bitstream which can trigger this for a test. Do we have any good tools for manually creating valid H264 NALU streams? VQ doesn't seem to let us edit files or anything like that. I only have the one I sourced from the site linked by the reporter in the bug, but it's NSFW...
Related details
Attention is currently required from:
- Dale Curtis
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Jul 8, 2025, 12:50:59 PM7/8/25
to Ted (Chromium) Meyer, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Ted (Chromium) Meyer
Dale Curtis added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Dale Curtis . unresolved
Seems okay if VQ is happy with it, but can you add a unit test?
Related details
Attention is currently required from:
- Ted (Chromium) Meyer
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ted (Chromium) Meyer (Gerrit)
Jul 8, 2025, 2:17:29 PM7/8/25
to Chromium LUCI CQ, Dale Curtis, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Dale Curtis
Ted (Chromium) Meyer added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Dale Curtis . unresolved
Seems okay if VQ is happy with it, but can you add a unit test?
Ted (Chromium) Meyer
Yeah, like I mentioned in the comment above, I'm still trying to craft an example media file using the bear clip (instead of the NSFW-ish clip from the bug)
Related details
Attention is currently required from:
- Dale Curtis
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ted (Chromium) Meyer (Gerrit)
Jul 8, 2025, 8:48:35 PM7/8/25
to Chromium LUCI CQ, Dale Curtis, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Dale Curtis
Ted (Chromium) Meyer added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Dale Curtis . resolved
Ted (Chromium) MeyerSeems okay if VQ is happy with it, but can you add a unit test?
Yeah, like I mentioned in the comment above, I'm still trying to craft an example media file using the bear clip (instead of the NSFW-ish clip from the bug)
Ted (Chromium) Meyer
well, it was an absolute nightmare to craft that test file, the NAL Units regularly cross TS packet boundaries, so it was a lot of fiddling about in a hex editor and ViCue.
Related details
Attention is currently required from:
- Dale Curtis
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Jul 9, 2025, 12:59:41 PM7/9/25
to Ted (Chromium) Meyer, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Ted (Chromium) Meyer
Dale Curtis voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dale Curtis
| Code-Review | +1 |
1 comment
File media/test/data/extraNALU.ts
File-level comment, Patchset 3 (Latest):
Dale Curtis . unresolved
rename to `extra-nalu.ts` so style is like all other files in dir.
Related details
Attention is currently required from:
- Ted (Chromium) Meyer
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ted (Chromium) Meyer (Gerrit)
Jul 9, 2025, 1:45:11 PM7/9/25
to Dale Curtis, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org
Ted (Chromium) Meyer added 1 comment![]( "Open in Gerrit")
File media/test/data/extraNALU.ts
Dale Curtis . resolved
rename to `extra-nalu.ts` so style is like all other files in dir.
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Jul 9, 2025, 2:19:33 PM7/9/25
to Ted (Chromium) Meyer, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org
Attention needed from Ted (Chromium) Meyer
Dale Curtis voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Ted (Chromium) Meyer
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ted (Chromium) Meyer (Gerrit)
Jul 9, 2025, 2:43:47 PM7/9/25
to Dale Curtis, Chromium LUCI CQ, chromium...@chromium.org, feature-me...@chromium.org
Ted (Chromium) Meyer voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Jul 9, 2025, 2:53:59 PM7/9/25
to Ted (Chromium) Meyer, Dale Curtis, chromium...@chromium.org, feature-me...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Disregard SPS and PPS NALUs when there is no slice
Some H264 media content seems to contain the following order of NAL
Units:
AUD - SPS - PPS - AUD - SPS - PPS - SEI - NonIDR
ViCue & FFmpeg both seem to accept this ordering of NAL units and
consider the first slice to be a valid picture, without even providing a
warning. Our current es parser wants to attempt to emit a frame every
time we would have hit an AUD NALU, but sometimes there just isn't a
picture and the pps id is zero. In this case, we can just skip emitting
a frame.
Bug: 427774381
Change-Id: Ie471707e41e117b742cf9b3d054dd443ebd868fd
Reviewed-by: Dale Curtis <dalec...@chromium.org>
Commit-Queue: Ted (Chromium) Meyer <tmath...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1484508}
Files:
- M media/formats/mp2t/es_parser_h264.cc
- M media/formats/mp2t/mp2t_stream_parser_unittest.cc
- M media/test/data/README.md
- A media/test/data/extra-nalu.ts
- M media/test/media_bundle_data.filelist
- M media/unit_tests_bundle_data.filelist
Change size: S
Delta: 6 files changed, 26 insertions(+), 3 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Dale Curtis
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages