The SEI message read/write functions are called
via function pointers where the SEI message-specific
context is passed as void*. But the actual function
definitions use a pointer to their proper context
in place of void*, making the calls undefined behaviour.
Clang UBSan 17 warns about this.
This commit fixes this by adding wrapper functions
(created via macros) that have the right type that
call the actual functions. This reduced the number of failing
FATE tests with UBSan from 164 to 85 here.
Reviewed-by: Mark Thompson <sw@jkqxz.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
nal->skipped_bytes_pos contains the positions of errors relative to the
start of the slice header, whereas the position they were tested against
is relative to the start of the slice data, i.e. one byte after the end
of the slice header.
Patch fixes this by storing the size of the slice header in H266RawSlice
and adding it to the position given by the GetBitContext before
comparing to skipped_bytes_pos. This fixes AVERROR_INVALIDDATAs for
various valid bitstreams, such as the LMCS_B_Dolby_2 conformance
bitstream.
Signed-off-by: Frank Plowman <post@frankplowman.com>
The SEI message code uses the AVBuffer API for its SEI messages
and contained buffers (like the extension buffer for HEVC
or the user data (un)registered payload buffers).
Contrary to the ordinary CBS code (where some of these
contained buffer references are actually references
to the provided AVPacket's data so that one can't replace
them with the RefStruct API), the CBS SEI code never uses
outside buffers at all and can therefore be switched entirely
to the RefStruct API. This avoids the overhead inherent
in the AVBuffer API (namely the separate allocations etc.).
Notice that the refcounting here is actually currently unused;
the refcounts are always one (or zero in case of no refcounting);
its only advantage is the flexibility provided by custom
free functions.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This avoids allocations and error checks etc. as well
as duplicate pointer lists in the CodedBitstreamFooContexts.
It also avoids casting const away for use as opaque,
as the RefStruct API supports const opaques.
The fact that some of the units are not refcounted
(i.e. they are sometimes part of an encoding context
like VAAPIEncodeH264Context) meant that CodedBitstreamUnit
still contains two pointers, one to the content
and another ownership pointer, replacing the AVBufferRef* pointer.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Turn tracing into callbacks for each syntax element, with default
callbacks to match current trace_headers behaviour for debug. Move
the construction of bit strings into the trace callback, which
simplifies all of the read and write functions.
Signed-off-by: Fei Wang <fei.w.wang@intel.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
These functions allow not only to read and write unsigned values,
but also to check ranges and to emit trace output which can be
beautified when processing arrays (indices like "[i]" are replaced
by their actual numbers).
Yet lots of callers actually only need something simpler:
Their range is only implicitly restricted by the amount
of bits used and they are not part of arrays, hence don't
need this beautification.
This commit adds specializations for these callers;
this is very beneficial size-wise (it reduced the size
of .text by 23312 bytes here), as a call is now cheaper.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The unchecked read caused the 2nd subsequent tell call to move backward resulting
in a negative length
Fixes: assertion failure
Fixes: 60276/clusterfuzz-testcase-minimized-ffmpeg_BSF_TRACE_HEADERS_fuzzer-5434126636023808
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Reviewed-by: James Almer <jamrial@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Add CodedBitstreamContext to parse VPS,SPS,PPS in VVC nal units.
Implement parsing and writing of SPS,PPS,VPS,PH,AUD,SEI and slices.
Add ff_cbs_type_h266 to cbs types tables and AV_CODEC_ID_H266
to cbs codec ids.
Co-authored-by: Thomas Siedel <thomas.ff@spin-digital.com>
Signed-off-by: James Almer <jamrial@gmail.com>
The functions to replace parameter sets are only called
after the respective parameter set has just been read or
has just been written; all of these functions check
that the id field is within the appropriate range.
So the checks in the replace-functions can be removed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
It is no longer used.
Also rename ff_cbs_alloc_unit_content2 to ff_cbs_alloc_unit_content.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Several cbs-functions had an unused CodedBitstreamContext parameter.
This commit removes these.
Reviewed-by: Mark Thompson <sw@jkqxz.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If this happens, it's a sign of parsing issues earlier in the process, or
misuse by the calling module.
Prevents writing invalid bitstreams.
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc>
Signed-off-by: James Almer <jamrial@gmail.com>
Fixes ticket #8622
Reviewed-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Mark Thompson <sw@jkqxz.net>
Signed-off-by: James Almer <jamrial@gmail.com>
According to C99, there has to be at least one argument for every ...
in a variadic function-like macro. In practice most (all?) compilers also
allow to leave it completely out, but it is nevertheless required: In a
variadic macro "there shall be more arguments in the invocation than there
are parameters in the macro definition (excluding the ...)." (C99,
6.10.3.4).
CBS (not the framework itself, but the macros used in the
cbs_*_syntax_template.c files) relies on the compiler allowing to leave
a variadic macro argument out. This leads to warnings when compiling in
-pedantic mode, e.g. "warning: must specify at least one argument for
'...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments]"
from Clang.
Most of these warnings can be easily avoided: The syntax_templates
mostly contain helper macros that expand to more complex variadic macros
and these helper macros often omit an argument for the .... Modifying
them to always expand to complex macros with an empty argument for the
... at the end fixes most of these warnings: The number of warnings went
down from 400 to 0 for cbs_av1, from 1114 to 32 for cbs_h2645, from 38 to
0 for cbs_jpeg, from 166 to 0 for cbs_mpeg2 and from 110 to 8 for cbs_vp9.
These eight remaining warnings for cbs_vp9 have been fixed by switching
to another macro in cbs_vp9_syntax_template: The fixed values for the
sync bytes as well as the trailing bits for byte-alignment are now read
via the fixed() macro (this also adds a check to ensure that trailing
bits are indeed zero as they have to be).
Reviewed-by: Mark Thompson <sw@jkqxz.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Slices that end after their header (meaning slices after the header
without any data before the rbsp_stop_one_bit or possibly without any
rbsp_stop_one_bit at all) are invalid and are now dropped. This ensures
that one doesn't run into two asserts in cbs_h2645_write_slice_data().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Fixes: 19629/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_METADATA_fuzzer-5676822528524288
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Trailing zeroes are already discarded when splitting a fragment, which
makes the code to remove them when decomposing slices dead code.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
HEVC standard supports multi-layer streams (ITU-T H.265 02/2018 Annex
F). Each NAL unit belongs to a particular layer defined by nuh_layer_id
in the header.
Currently, all NAL units that do not belong to a base layer are
automatically removed in ff_h2645_packet_split(). Some data may
therefore be lost when future filters/decoders are designed to support
multi-layer streams.
A better approach is to forward nuh_layer_id > 0 packets and let blocks
down the chain decide how to process them. The condition to remove
packets has been moved to hevcdec and cbs.
Found-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
In the worst case the startcode prefix has 4 bytes.
This fixes a trigerred assertion:
Assertion dp <= max_size failed at libavcodec/cbs_h2645.c:1451
Found-by:libFuzzer
Reviewed-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
The number of bits in a PutBitContext must fit into an int, yet nothing
guaranteed the size argument cbs_write_unit_data() uses in init_put_bits()
to be in the range 0..INT_MAX / 8. This has been changed.
Furthermore, the check 8 * data_size > data_bit_start that there is
data beyond the initial padding when writing mpeg2 or H.264/5 slices
could also overflow, so divide it by 8 to get an equivalent check
without this problem.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
All cbs-functions to write units share a common pattern:
1. They check whether they have a write buffer (that is used to store
the unit's data until the needed size becomes known after writing the
unit when a dedicated buffer will be allocated).
2. They use this buffer for a PutBitContext.
3. The (codec-specific) writing takes place through the PutBitContext.
4. The return value is checked. AVERROR(ENOSPC) here always indicates
that the buffer was too small and leads to a reallocation of said
buffer.
5. The final buffer will be allocated and the data copied.
This commit factors this common code out in a single function in cbs.c.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The maximum allowed index for an array access is FF_ARRAY_ELEMS - 1; yet
the current code allowed FF_ARRAY_ELEMS. This wasn't dangerous in practice,
as parameter sets with invalid ids were already filtered out during
reading.
Found via PVS-Studio (see ticket #8156).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If adding an SEI message to an access unit fails, said SEI message was
not touched, so that the caller had to free any data associated with it
that might need to be freed. But given that ff_cbs_h264_add_sei_message
can simply call cbs_h264_free_sei_payload, one can easily free
the content of the SEI payload.
This fixes a memleak when inserting a user data unregistered string for
h264_metadata fails.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
