PR #24070 opened by haochenc URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24070 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24070.patch
In av_frame_apply_cropping, the function returned AVERROR_BUG if the calculated data offsets had a higher alignment than the crop_left value (log2_crop_align < min_log2_align). This check was too strict and caused crashes in decoders like agm when valid crop values and linesizes happened to align to higher powers of 2 (e.g., crop_left=256, but offsets=512). The original check and comment assumed a constant power-of-2 relation between crop alignment (pixels) and data alignment (bytes). This assumption is incorrect because the final offset is the sum of vertical (crop_top * linesize) and horizontal (crop_left * step) shifts. Adding two aligned numbers can result in a sum with a higher alignment (e.g., 1280 + 256 = 1536, which increases alignment from 256-byte to 512-byte). Rejecting this higher alignment is incorrect as it is safe and preferred for CPU performance. Relax the check to only return AVERROR_BUG if the minimum data alignment is also low (min_log2_align < 5). If the data is already sufficiently aligned (>= 32 bytes), the check is bypassed, as no adjustment is needed. Update the comment to reflect this new logic. Signed-off-by: Hao Chen <[email protected]> # Summary of changes Briefly describe what this PR does and why. <!-- If this PR requires new FATE test samples, attach them to the PR and list their target paths below (relative to the fate-suite root). Attached filenames must match the sample's filename: ```fate-samples # e.g. vorbis/new-sample.ogg ``` --> >From 3102e48ee12d3a0486ae5b24c83e065315e8a4d8 Mon Sep 17 00:00:00 2001 From: Hao Chen <[email protected]> Date: Mon, 10 Aug 2026 21:18:56 +0000 Subject: [PATCH] avutil/frame: relax alignment check in av_frame_apply_cropping In av_frame_apply_cropping, the function returned AVERROR_BUG if the calculated data offsets had a higher alignment than the crop_left value (log2_crop_align < min_log2_align). This check was too strict and caused crashes in decoders like agm when valid crop values and linesizes happened to align to higher powers of 2 (e.g., crop_left=256, but offsets=512). The original check and comment assumed a constant power-of-2 relation between crop alignment (pixels) and data alignment (bytes). This assumption is incorrect because the final offset is the sum of vertical (crop_top * linesize) and horizontal (crop_left * step) shifts. Adding two aligned numbers can result in a sum with a higher alignment (e.g., 1280 + 256 = 1536, which increases alignment from 256-byte to 512-byte). Rejecting this higher alignment is incorrect as it is safe and preferred for CPU performance. Relax the check to only return AVERROR_BUG if the minimum data alignment is also low (min_log2_align < 5). If the data is already sufficiently aligned (>= 32 bytes), the check is bypassed, as no adjustment is needed. Update the comment to reflect this new logic. Signed-off-by: Hao Chen <[email protected]> --- libavutil/frame.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libavutil/frame.c b/libavutil/frame.c index be30eb09d2..e91a7d58d7 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -803,9 +803,10 @@ int av_frame_apply_cropping(AVFrame *frame, int flags) min_log2_align = FFMIN(log2_align, min_log2_align); } - /* we assume, and it should always be true, that the data alignment is - * related to the cropping alignment by a constant power-of-2 factor */ - if (log2_crop_align < min_log2_align) + /* We only enforce the alignment relation check if we need to adjust + * alignment (min_log2_align < 5) to avoid negative shifts. + * Otherwise, higher alignment is safe to allow. */ + if (log2_crop_align < min_log2_align && min_log2_align < 5) return AVERROR_BUG; if (min_log2_align < 5 && log2_crop_align != INT_MAX) { -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
