Fixes runtime error: member access within misaligned address
<addr> for type 'av_alias64', which requires 8 byte alignment.
VP9mv is aligned to 4 bytes, so instead doing 8 bytes clear, let's do
2 times 4 bytes.
Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
Reviewed-by: "Ronald S. Bultje" <rsbultje@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: Use-of-uninitialized value
Fixes: clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_METADATA_fuzzer-5458626041413632
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Fixes use of uninitialized value, reported by MSAN.
Found by OSS-Fuzz.
Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
Fixes: 70852/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-5179190066872320
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
There is no known (real) hardware with V and without the complete B
extension. B was indeed required in the RISC-V application profile from
2022, earlier than V. There should not be any relevant hardware in the
future either.
In practice, different R-V Vector optimisations in FFmpeg already depend on
every constituent of the B extension anyhow, so it would not work well.
These are really just wrappers for idct4_add16intra functions, which are in
turn mostly wrappers for idct4_add and idct4_dc_add functions.
For benchmarks refer to the later two sets.
Before the patch, the method to generate DTS only works with
timebase equal to 1/fps. With timebase like 1/1000
./ffmpeg -i foo.mp4 -an -c:v h264_nvenc -enc_time_base 1/1000 bar.mp4
pts 0 dts -3
pts 160 dts 37
pts 80 dts 77
pts 40 dts 117 <-- invalid
pts 120 dts 157
pts 320 dts 197
pts 240 dts 237
pts 200 dts 277 <-- invalid
pts 280 dts 317 <-- invalid
The generated DTS can be larger than PTS, since it only reorder the
input PTS and minus the number of frame delay, which doesn't take
timebase into account. It should minus the "time" of frame delay.
9a245bd trying to fix the issue, but the implementation is incomplete,
which only use time_base.num. Then it got reverted by ac7c265b33.
After this patch:
pts 0 dts -120
pts 160 dts -80
pts 80 dts -40
pts 40 dts 0
pts 120 dts 40
pts 320 dts 80
pts 240 dts 120
pts 200 dts 160
pts 280 dts 200
Signed-off-by: Timo Rothenpieler <timo@rothenpieler.org>
This was added along side the original SSE(one) DSP function in
0a68cd876e without rationale. This was
presumably faster on x87, which is no longer relevant since we pretty
much assume SSE2 or later on x86.
Meanwhile this function is ~2.5x slower than the normal floating point
one on SiFive-U74.
Given that source and destination can alias, the compiler was forced to
perform each read-modify-write sequentially. We cannot use the `restrict`
qualifier to avoid this here because the AC-3 encoder uses the function
in-place. Instead this commit provides an explicit guarantee to the
compiler that batches of 8 elements will not overlap, so that it can
interleave calculations.
In practice contemporary optimising compilers are able to unroll and keep
the temporary array in FPU registers (without spilling).
On SiFive-U74, this speeds the same signs branch by 4x, and the
opposite signs branch 1.5x.
The current code assumes that we have unaligned rows, which hurts on
platforms with slower unaligned accesses. (Also, this lets the compiler
unroll manually, which it seems to do in practice.)
Frame threading in the FFV1 decoder works in a very unusual way - the
state that needs to be propagated from the previous frame is not decoded
pixels(¹), but each slice's entropy coder state after decoding the slice.
For that purpose, the decoder's update_thread_context() callback stores
a pointer to the previous frame thread's private data. Then, when
decoding each slice, the frame thread uses the standard progress
mechanism to wait for the corresponding slice in the previous frame to
be completed, then copies the entropy coder state from the
previously-stored pointer.
This approach is highly dubious, as update_thread_context() should be
the only point where frame-thread contexts come into direct contact.
There are no guarantees that the stored pointer will be valid at all, or
will contain any particular data after update_thread_context() finishes.
More specifically, this code can break due to the fact that keyframes
reset entropy coder state and thus do not need to wait for the previous
frame. As an example, consider a decoder process with 2 frame threads -
thread 0 with its context 0, and thread 1 with context 1 - decoding a
previous frame P, current frame F, followed by a keyframe K. Then
consider concurrent execution consistent with the following sequence of
events:
* thread 0 starts decoding P
* thread 0 reads P's slice header, then calls
ff_thread_finish_setup() allowing next frame thread to start
* main thread calls update_thread_context() to transfer state from
context 0 to context 1; context 1 stores a pointer to context 0's private
data
* thread 1 starts decoding F
* thread 1 reads F's slice header, then calls
ff_thread_finish_setup() allowing the next frame thread to start
decoding
* thread 0 finishes decoding P
* thread 0 starts decoding K; since K is a keyframe, it does not
wait for F and reallocates the arrays holding entropy coder state
* thread 0 finishes decoding K
* thread 1 reads entropy coder state from its stored pointer to context
0, however it finds state from K rather than from P
This execution is currently prevented by special-casing FFV1 in the
generic frame threading code, however that is supremely ugly. It also
involves unnecessary copies of the state arrays, when in fact they can
only be used by one thread at a time.
This commit addresses these deficiencies by changing the array of
PlaneContext (each of which contains the allocated state arrays)
embedded in FFV1SliceContext into a RefStruct object. This object can
then be propagated across frame threads in standard manner. Since the
code structure guarantees only one thread accesses it at a time, no
copies are necessary. It is also re-created for keyframes, solving the
above issue cleanly.
Special-casing of FFV1 in the generic frame threading code will be
removed in a later commit.
(¹) except in the case of a damaged slice, when previous frame's pixels
are used directly
In all cases except decoding version 1 it's either not used, or contains
a copy of a table from quant_tables, which we can just as well use
directly.
When decoding version 1, we can just as well decode into
quant_tables[0], which would otherwise be unused.
FFV1 decoder and encoder currently use the same struct - FFV1Context -
both as codec private data and per-slice context. For this purpose
FFV1Context contains an array of pointers to per-slice FFV1Context
instances.
This pattern is highly confusing, as it is not clear which fields are
per-slice and which per-codec.
Address this by adding a new struct storing only per-slice data. Start
by moving slice_{x,y,width,height} to it.
Fixes: out of array access
Fixes: 70741/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-5703668010647552
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
The snow encoder uses block based motion estimation which can read out of array if
insufficient alignment is used
It may be better to only apply this for the encoder, as it would safe a few bytes of memory
for the decoder. Until then, this fixes the issue in a simple way.
Fixes: out of array access
Fixes: 68963/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-4979988435632128
Fixes: 68969/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-6239933667803136.fuzz
Fixed: 70497/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SNOW_fuzzer-5751882631413760
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Kacper Michajłow
Andreas Rheinhardt
Rémi Denis-Courmont
Timo Rothenpieler
James Almer
Zhao Zhili
Roman Arzumanyan
Anton Khirnov
Michael Niedermayer