If Zbb is enabled at compilation (e.g. Ubuntu), the compiler should
compile the new C mid_pred() function correctly. But if Zbb is *not*
enabled (e.g. Debian), then we can at least fallback at run-time.
On SiFive-U74, before:
sub_median_pred_c: 1331.9 ( 1.00x)
sub_median_pred_rvb_b: 881.8 ( 1.51x)
After:
sub_median_pred_c: 1133.1 ( 1.00x)
sub_median_pred_rvb_b: 875.7 ( 1.29x)
This reduces the minimum instruction emission for mid_pred()
(i.e. median of 3) down to:
- 3 comparisons and 4 conditional moves, or
- 4 min/max.
With that the compiler can eliminate any branch. This optimal
situation is attainable with Clang 21 on Arm64, RVA22 and x86,
with GCC 15 on Arm64 and x86 (RVA22 goes from 2 to 1 branch).
These optimisations also work on Arm32 and LoongArch.
The same algorithm is already implemented via inline assembler for some
architectures such as x86 and Arm32, but notably not Arm64 and RVA22.
Besides, using C code allows the compiler to schedule instruction
properly.
Even on architectures with neither conditional moves nor min/max, this
leads to a visible performance improvement for C code, as seen here for
RVA20 code running on SiFive-U74:
Before:
sub_median_pred_c: 1657.5 ( 1.00x)
sub_median_pred_rvb_b: 875.9 ( 1.89x)
After:
sub_median_pred_c: 1331.9 ( 1.00x)
sub_median_pred_rvb_b: 881.8 ( 1.51x)
Note that this commit leaves the x86 and Arm32 code intact so it has
no effects on those ISA's.
vp8 encoder can be configured to drop frames, when e.g. bitrate
overshoot is detected. At present the code responsible for
managing an internal fifo assumes that we will get an output frame per
each frame fed into encoder. That is not the case if the encoder can
decide to drop frames.
Running:
ffmpeg -stream_loop 100 -i dash_video3.webm -c:v libvpx -b:v 50k
-drop-threshold 20 -screen-content-mode 2 output.webm
results in lots of warnings like:
[libvpx @ 0x563fd8aba100] Mismatching timestamps: libvpx 2187 queued
2185; this is a bug, please report it
[libvpx @ 0x563fd8aba100] Mismatching timestamps: libvpx 2189 queued
2186; this is a bug, please report it
followed by:
[vost#0:0/libvpx @ 0x563fd8ab9b40] [enc:libvpx @ 0x563fd8aba080] Error
submitting video frame to the encoder
[vost#0:0/libvpx @ 0x563fd8ab9b40] [enc:libvpx @ 0x563fd8aba080] Error
encoding a frame: No space left on device
[vost#0:0/libvpx @ 0x563fd8ab9b40] Task finished with error code: -28
(No space left on device)
[vost#0:0/libvpx @ 0x563fd8ab9b40] Terminating thread with return code
-28 (No space left on device)
The reason for the above error is that each dropped frame leaves an
extra item in the fifo, which eventually overflows.
The proposed fix is to keep popping elements from the fifo until the
one with the matching pts is found. A side effect of this change is that
the code no longer considers pts mismatch to be a bug.
This has likely regressed around 5bda4ec6c3
when fifo started to be universally used.
Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com>
JPEG-XS streams can have the bytes corresponding to certain markers as part of
slice data, and no considerations were made for it, so we need to add checks
for false positives.
This fixes assembling several samples.
Signed-off-by: James Almer <jamrial@gmail.com>
The code assumed that the destination buffer was zeroed, a misbehaviour
with which checkasm is bug-compatible as it zeroes the destination
buffer. The fixed code is even faster:
SpacemiT X60:
sub_left_predict_c: 51792.5 ( 1.00x)
sub_left_predict_rvv_i32: 3504.4 (14.78x)
The shader needs ~3 loads per DCT coeff.
This data was not observed to get efficiently stored
in the upper cached levels, loading it explicitely in
shared memory fixes that.
Also reduce code size by moving the bitstream
initialization outside of the switch/case.
(Like the old code, the new code limits the number of threads to 64,
even when the user explicitly set a higher thread count. I don't know
whether it is intentional to apply this limit even when the user
explicitly supplied the number of threads.)
Reviewed-by: James Almer <jamrial@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The pixel format has already been checked generically.
This also fixes the bug that the earlier code ignored
the return value of set_pix_fmt().
Reviewed-by: James Almer <jamrial@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
sub_median_pred_mmxext() calculates a predictor from the left, top
and topleft pixel values. The topleft values need to be initialized
differently for the first loop initialization than for the others
in order to avoid reading ptr[-1]. So it has been initialized before
the loop and then read again at the end of the loop, so that the last
value read was never used. Yet this can lead to reads beyond the end
of the buffer, e.g. with
ffmpeg -cpuflags mmx+mmxext -f lavfi -i "color=size=64x4,format=yuv420p" \
-vf vflip -c:v ffvhuff -pred median -frames 1 -f null -
Fix this by not reading the value at the end of the loop.
Reviewed-by: Lynne <dev@lynne.ee>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
sub_median_pred_mmxext() calculates a predictor from the left, top
and topleft pixel values. The left value is simply read via
ptr[-1], although this is not guaranteed to be inside the buffer
in case of negative strides. This happens e.g. with
ffmpeg -i fate-suite/mpeg2/dvd_single_frame.vob -vf vflip \
-c:v magicyuv -pred median -f null -
Fix this by reading the first value like the topleft value.
Also change the documentation of sub_median_pred to reflect this
change (and the one from 791b5954bc).
Reviewed-by: Lynne <dev@lynne.ee>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The issue is that .flush gets called asynchronously, and modifies the
video session state while its being used for decoding. This did not
result in issues since all known vendors do not keep important state
there, but its not compliant with the specs.
Its not necessary to flush the decoder at all when seeking,
so simply don't.
Fixes#20487
perm gets incremented in the loop in such a manner that
it already has the value it is set to here except for
the first loop iteration.
Reviewed-by: Marton Balint <cus@passwd.hu>
Reviewed-by: Lynne <dev@lynne.ee>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The DPX Vulkan unpack shader computes a word offset as
uint off = (line_off + pix_off >> 5);
Due to GLSL operator precedence this is evaluated as
line_off + (pix_off >> 5) rather than (line_off + pix_off) >> 5.
Since line_off is in bits while off is a 32-bit word index,
scanlines beyond y=0 use an inflated offset and the shader reads
past the end of the DPX slice buffer.
Parenthesize the expression so that the sum is shifted as intended:
uint off = (line_off + pix_off) >> 5;
This corrects the unpacked data and removes the CRC mismatch
observed between the software and Vulkan DPX decoders for
mispacked 12-bit DPX samples. The GPU OOB read itself is only
observable indirectly via this corruption since it occurs inside
the shader.
Repro on x86_64 with Vulkan/llvmpipe (531ce713a0):
./configure --cc=clang --disable-optimizations --disable-stripping \
--enable-debug=3 --disable-doc --disable-ffplay \
--enable-vulkan --enable-libshaderc \
--enable-hwaccel=dpx_vulkan \
--extra-cflags='-fsanitize=address -fno-omit-frame-pointer' \
--extra-ldflags='-fsanitize=address' && make
VK_ICD_FILENAMES=/usr/share/vulkan/icd.d/lvp_icd.json
PoC: packed 12-bit DPX with the packing flag cleared so the unpack
shader runs (4x64 gbrp12le), e.g. poc12_packed0.dpx.
Software decode:
./ffmpeg -v error -i poc12_packed0.dpx -f framecrc -
-> 0, ..., 1536, 0x26cf81c2
Vulkan hwaccel decode:
VK_ICD_FILENAMES=/usr/share/vulkan/icd.d/lvp_icd.json \
./ffmpeg -v error -init_hw_device vulkan \
-hwaccel vulkan -hwaccel_output_format vulkan \
-i poc12_packed0.dpx \
-vf hwdownload,format=gbrp12le -f framecrc -
-> 0, ..., 1536, 0x71e10a51
The only difference between the two runs is the Vulkan unpack
shader, and the stable CRC mismatch indicates that it is reading
past the intended DPX slice region.
Regression since: 531ce713a0
Found-by: Pwno
Required for the following commit, where a parsing function expects the buffer
to include the country code bytes.
Signed-off-by: James Almer <jamrial@gmail.com>
