Repository navigation
Fix HEVC test - #1907
Merged
Merged
Fix HEVC test#1907
Conversation
The x265 plugin calls api->encoder_headers() as soon as a sequence encoder is opened, and encode_sequence_frame() calls get_data() immediately afterwards; before any frame is encoded. That call sees three parameter-set NALs and nothing else, so the optional is still empty when line 260 dereferences it. That is turned into the _M_is_engaged() abort instead of silent UB because of Fedora's default GCC option -Wp,-D_GLIBCXX_ASSERTIONS.
Contributor
|
I can confirm this fixes the test in 1.23.4 for me (Fedora rawhide + RPM Fusion). |
algitbot
pushed a commit
to alpinelinux/aports
that referenced
this pull request
Sep 20, 2026
Member
|
Thanks, confirmed. I reproduced the abort on master with -D_GLIBCXX_ASSERTIONS and the patch fixes it; a two-frame heif-enc -S round trip with the patched build also looks right (hvcC in the sample entry, both frames decode). |
farindk
added a commit
that referenced
this pull request
Sep 21, 2026
…e optional Rename Encoder::encode_sequence_get_data() to encode_sequence_extract_data() and let it hand out the pending CodedImageData with std::exchange(m_current_output_data, std::nullopt). Moving out of a std::optional leaves it engaged with a moved-from value, so after the first frame the member was never disengaged again. "Nothing pending" was represented two ways: a disengaged optional before the first frame, and an engaged one with empty vectors afterwards, which the caller had to detect with a separate bitstream/properties emptiness check. The HEVC/VVC get_data() paths were written against the second form only, which is what let the header-only first drain fixed in #1907 dereference the disengaged optional. With the reset, a disengaged optional is the only "no output pending" state. HEVC and VVC drop the got_some_data flag and use that single check; HEVC also drops the redundant bitstream.empty() test in front of the hvcC attachment. AVIF no longer parks an empty CodedImageData when the encoder is still buffering. AVC keeps its m_output_image_complete gate in front of the exchange, because x264_encoder_headers() emits an SEI together with the SPS/PPS and that SEI has to be held back until the slice data arrives. The JPEG, JPEG 2000 and uncompressed encoders used has_value() as their encode_sequence_started() answer and now track that with a bool. Sequence output for HEVC, AVC, AVIF, VVC and JPEG is byte-identical to the previous code, and the test suite passes with -D_GLIBCXX_ASSERTIONS.
farindk
added a commit
that referenced
this pull request
Sep 21, 2026
Export CXXFLAGS with -D_GLIBCXX_ASSERTIONS for the test job in run-ci.sh. The assertions (container bounds checks, access to a disengaged std::optional, ...) are independent of NDEBUG, so they also fire in the Release build that run-ci.sh forces. Distributions like Fedora build with this flag, which is how the crash fixed in #1907 was found while our own CI stayed green. With the assertions enabled, GCC 13 at -O3 reports bogus -Wstringop-overflow errors ("writing N bytes into a region of size 0" in stl_algobase.h) for plain std::vector range inserts in several places (hvcC/avcC/vvcC header getters, test helpers). The same build is warning-free without the flag and rewriting call sites only moves the diagnostic around, so that one warning is kept visible but non-fatal in this job. Only the test job gets the flag. The other jobs run more compiler versions, which would each need to be checked for further false positives under -Werror before enabling it there.
algitbot
pushed a commit
to alpinelinux/aports
that referenced
this pull request
Sep 21, 2026
bell-sw
pushed a commit
to bell-sw/alpaquita-aports
that referenced
this pull request
Sep 25, 2026
[ commit 1e9f7dadf6a7ebfd718f3d78fa4ad1c63de0af2b ] patch from strukturag/libheif#1907
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The x265 plugin calls
api->encoder_headers()as soon as a sequence encoder is opened, andencode_sequence_frame()callsget_data()immediately afterwards; before any frame is encoded.That call sees three parameter-set NALs and nothing else, so the optional is still empty when line 260 dereferences it. That is turned into the
_M_is_engaged()abort instead of silent UB because of Fedora's default GCC option-Wp,-D_GLIBCXX_ASSERTIONS.Sample error without this patch: