Skip to content

Fix stale slice header bytes in the single SEI NAL unit - #988

Open
danielcamposramos wants to merge 1 commit into
Multicorewareinc:masterfrom
danielcamposramos:single-sei-stale-bytes
Open

danielcamposramos wants to merge 1 commit into
Multicorewareinc:masterfrom
danielcamposramos:single-sei-stale-bytes

Conversation

@danielcamposramos

Copy link
Copy Markdown

With --single-sei, the prefix SEI messages of an access unit are written into m_bs and serialized as one NAL unit.
m_bs is reset before them only by the access unit delimiter or by the repeated stream headers.
With --no-aud, on a picture without repeated headers, it still holds the previous picture's slice header, and those bytes are written in front of the first SEI message.

@vunguyen1989 found it while reviewing #986 and asked for the fix as its own pull request, since it affects master on its own.

Reproducing it on master

x265 --y4m --preset ultrafast --frames 48 --frame-threads 1 --pools none --keyint 12 --min-keyint 12 --no-scenecut --bframes 0 --no-info --no-aud --no-repeat-headers --hrd --vbv-bufsize 2000 --vbv-maxrate 1000 --single-sei --input input.y4m --output hrd.h265
ffmpeg -hide_banner -nostdin -i hrd.h265 -map 0:v:0 -c:v copy -bsf:v trace_headers -f null -

FFmpeg reports Invalid SEI message: payload_size too large for 47 of the 48 SEI NAL units.
Before POC 12, for example, the NAL unit is 4E 01 D0 59 FE 20 EC 00 07 80 ...: the five bytes after the NAL unit header are the start of the previous picture's slice header, and the buffering period SEI follows them.

The fix

FrameEncoder::compressFrame() resets m_bs before the SEI messages when bSingleSeiNal is set, unless the picture is a keyframe with repeated headers.
That exception is the first term of isSei: there the stream headers have already reset m_bs and collected their own SEI messages in it, and those are kept.

How it was tested

Built from master f2cf32d, 8 bit, --frame-threads 1 --pools none, 48 pictures, keyint 12.

Options trace_headers errors, master With the fix Output
--single-sei --no-aud --no-repeat-headers --hrd 47 0 fixed
--single-sei --no-aud --repeat-headers --hrd --max-cll 1000,400 44 0 fixed, keyframes byte-identical
--single-sei --no-aud --no-repeat-headers --eos --hrd 47 0 fixed
--single-sei --aud --hrd 0 0 byte-identical
defaults 0 0 byte-identical

Every prefix SEI NAL unit in the fixed streams parses exactly, with the messages ending at the rbsp trailing bits.
With the frame packing option of #986 on top, the same holds for types 3, 4 and 5, with and without AUD and repeated headers, in 8 and 10 bit, and FFmpeg reads the frame packing message on every picture.

Notes

No primitives are touched, so I did not run TestBench.

With --single-sei the prefix SEI messages of an access unit are written
into m_bs and serialized as one NAL unit. m_bs is reset before them only
by the access unit delimiter or by the repeated stream headers, so with
--no-aud, on a picture without repeated headers, it still holds the
previous picture's slice header and those bytes are written in front of
the first SEI message. FFmpeg's trace_headers then reports "Invalid SEI
message: payload_size too large".

On master this happens with --hrd --single-sei --no-aud --no-repeat-headers
(47 of 48 SEI NAL units in a 48 picture encode), and with --frame-packing
as reported on Multicorewareinc#986.

Reset m_bs in that case. Streams that were already correct do not change:
with --aud, without --single-sei, and on keyframes with --repeat-headers
the output is byte-identical.

Assisted-by: Claude Opus via Claude Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant