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
This reverts commit 8f48a62, 9af8782, and bd3e71b.
Commit 8f48a62 extends tx to 2M, resulting in the tx_float bss
section reaching a size of 4M.
This isn't a issue on devices with normal memory sizes and OS
supporting virtual memory. But it's a real issue for embedded devices
with realtime OS, which may not support virtual memory, e.g., Nuttx.
This 4M of bss section map to physical memory directly, which is a
scarce resource on embedded devices.
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>
Fix#21056
Refer to RFC 3267 Section 4.4.2:
A ToC entry takes the following format in octet-aligned mode:
0 1 2 3 4 5 6 7
+-+-+-+-+-+-+-+-+
|F| FT |Q|P|P|
+-+-+-+-+-+-+-+-+
P bits: padding bits, MUST be set to zero.
Signed-off-by: Jack Lau <jacklau1222gm@gmail.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
Regression since: 3ba570de8b (port from MMX to SSE2).
The SSE2 inline asm in libavfilter/x86/vf_noise.c (line_noise_sse2 and
line_noise_avg_sse2) uses aligned loads/stores (movdqa, movntdq) but never
checks pointer alignment. When the filter reuses an input frame (common
path when av_frame_is_writable() is true), it may receive misaligned data
from upstream filters that adjust frame->data[i] in place, notably vf_crop:
- vf_crop adjusts plane pointers by arbitrary byte offsets
(frame->data[plane] += ...), so an x offset of 1 on 8-bit formats produces
a 1‑byte misalignment.
- The noise filter then calls the SSE2 path directly on those pointers
without realigning or falling back.
Repro on x86_64/SSE2 (current HEAD at that commit):
./ffmpeg -v error -f lavfi -i testsrc=s=320x240:rate=1 \
-vf "format=yuv420p,crop=w=319:x=1:h=240:exact=1,noise=alls=50" \
-frames:v 1 -f null -
This crashes with SIGSEGV at the aligned load in line_noise_sse2 (movdqa
(%r9,%rax),%xmm0; effective address misaligned by 1 byte).
Impact: denial of service via crafted filtergraphs (e.g., crop + noise).
Applies to planar 8-bit formats where upstream filters can shift data
pointers without reallocating.
Found-by: Pwno OSS Team
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>
Add a public API for producing RFC 4281/6381 codecs trings for
MIME types.
This can be required for providing alternative video files to
a web browser, letting the browser pick the best file it supports.
Such strings also allow querying a browser whether it supports
a certain codec combination.
Finally, if implementing a DASH/HLS segmenter outside of libavformat,
one also has to generate such strings.
Generating such strings for H264/AAC is very simple, but for
more modern codecs, it can require a lot of nontrivial codec
specific parsing of extradata.
As libavformat already implements this, expose it for users as well.
The old, internal function ff_make_codec_str is kept and used by
the HLS and DASH muxers; the old function takes a logging context
which can be used for logging auxillary info about how the string
generation worked out.
ff_vk_video_common_init() calls ff_vk_video_common_uninit() on failure
which leaves dangling object handles. Those get freed again when the
destructor of FFVulkanDecodeShared calls ff_vk_video_common_uninit()
again.
Signed-off-by: Cameron Gutman <aicommander@gmail.com>
Older versions of Clang (Xcode 14, llvm.org Clang 13 and 14)
do support and recognize SME, but when enabled through
".arch_extension sme" it fails to transitively enable support
for Streaming SVE; this was fixed in [1].
This issue results in those versions currently detecting support
for SME, but later failing to build cpu_sme.s with errors like
"error: instruction requires: sve or sme" or "error: instruction
requires: streaming-sve or sve", on the "cntb x0" instruction.
Extend the check for this instruction set extension, to test
with two instructions, both specifically a SME instruction
(smstart) and an instruction that is available in Streaming SVE
mode (cntb).
For the configure check, add an extra parameter to
check_archext_insn for an optional second instruction to check.
It would be tempting to just pass both instructions through
the same parameter, as "smstart; cntb x0". However, Darwin
targets use a different token (%%) for starting a new
instruction on the same line - those targets interpret ";"
as the start of a comment. Due to that, such a check would
entirely ignore the second instruction on Darwin targets.
To avoid dealing with the variability in passing multiple
instructions on one line, just pass the optional second
instruction on a separate line.
[1] ff3f3a54e2
After the full ffmpeg CLI multithreading changes went in, this
test started depending on how far the input side read and decoded
the input compared to how quickly the output encoded things, causing
spurious failures on the CI.
To my knowledge all of the failures have so far been valid correct
results, but unfortunately FATE's built in checks mostly consist of
whether there is a difference against an exact result.
This way we still get the CI and valgrind running of the code,
but stop its comparison. Reference file is left around so that
the previous reference is still available.
The header read check stored the comparison result into ret, so read
failures became ret=1 and were treated as success, leaving the VMD header
uninitialized and letting parsing continue with bogus state.
Regression since: ee623a43e3.
Found-by: Pwno
We already parse the EXIF side data to extract the orientation, so we
should add it to the output file as an EXIF box.
Signed-off-by: Leo Izen <leo.izen@gmail.com>
Before this commit, we ignore the display matrix side data if any EXIF
side data is present, even if that side data contains no orientation
tag. This allows us to calculate the orientation from the display
matrix sidedata first, if present. Ideally the decoder will have
removed the orientation tag upon decoding and attached the data as
display matrix side data instead, so this makes our orientation code
respect this behavior.
Signed-off-by: Leo Izen <leo.izen@gmail.com>
_cflags_noopt was previously set to optimize for size as a workaround
for issues in MSVC builds without any optimizations. Specifically the
lack of DCE and constant merging. This is no longer needed, as those
issues have been fixed in our codebase.
Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
