On 2025/4/16 8:24, Michael Niedermayer wrote:
> On Tue, Apr 08, 2025 at 07:22:32PM +0800, Bin Peng wrote:
>> When decoding a bitstream with weighted-bipred enabled,
>> the results on ARM and x86 platforms may differ.
>>
>> The reason for the inconsistency is that the value of
>> STRIDE_ALIGN differs between platforms. And STRIDE_ALIGN
>> is set to the buffer stride of temporary buffers for U
>> and V components in mc_part_weighted.
>>
>> If the buffer stride is 32 or 64 (as on x86 platforms),
>> the U and V pixels can be interleaved row by row without
>> overlapping, resulting in correct output.
>> However, on ARM platforms where the stride is 16,
>> the V component will overwrite part of the U component's pixels,
>> leading to incorrect predicted pixels.
>>
>> The bug can be reproduced by the following bitstream.
>>
>> https://trac.ffmpeg.org/attachment/ticket/11357/inter_weighted_bipred2.264
>>
>> And the ref/fate file is also added in this patch.
>>
>> Fixes: ticket 11357
>> Signed-off-by: Bin Peng <peng...@visionular.com>
>> ---
>> libavcodec/utils.c | 4 ++-
>
>> .../h264-conformance-weighted_bipred2.264 | 31 +++++++++++++++++++
>
> is this supposed to be a fate test ?
> if so theres something missing also the file extension is a bit misleading
Yes, sorry for the file extension, It should has no extension.
I guess we don't need this fate test after you merged your better fix.
> and tests covering more cases is always welcome
>
>
>> 2 files changed, 34 insertions(+), 1 deletion(-)
>> create mode 100644 tests/ref/fate/h264-conformance-weighted_bipred2.264
>>
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index 90867ed6b1..5d15f5c8fa 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -144,6 +144,7 @@ void avcodec_align_dimensions2(AVCodecContext *s, int
>> *width, int *height,
>> int i;
>> int w_align = 1;
>> int h_align = 1;
>> + int stride_align = STRIDE_ALIGN;
>> AVPixFmtDescriptor const *desc = av_pix_fmt_desc_get(s->pix_fmt);
>>
>> if (desc) {
>> @@ -339,13 +340,14 @@ void avcodec_align_dimensions2(AVCodecContext *s, int
>> *width, int *height,
>> // increasing witdth ensure that the temporary area is large enough,
>> // the next rounded up width is 32
>> *width = FFMAX(*width, 32);
>> + stride_align = FFMAX(stride_align, 32);
>
> does the following fix this too ?
Yes, I checked your change on my local test, it also fix this issue, and your
fix is better.
Would you like to submit a patch with this change? Thanks a lot!
>
> diff --git a/libavcodec/h264_mb.c b/libavcodec/h264_mb.c
> index 6083f7ad84f..0d6562b5830 100644
> --- a/libavcodec/h264_mb.c
> +++ b/libavcodec/h264_mb.c
> @@ -407,7 +407,7 @@ static av_always_inline void mc_part_weighted(const
> H264Context *h, H264SliceCon
> /* don't optimize for luma-only case, since B-frames usually
> * use implicit weights => chroma too. */
> uint8_t *tmp_cb = sl->bipred_scratchpad;
> - uint8_t *tmp_cr = sl->bipred_scratchpad + (16 << pixel_shift);
> + uint8_t *tmp_cr = sl->bipred_scratchpad + (8 << pixel_shift +
> (chroma_idc == 3));
> uint8_t *tmp_y = sl->bipred_scratchpad + 16 * sl->mb_uvlinesize;
> int refn0 = sl->ref_cache[0][scan8[n]];
> int refn1 = sl->ref_cache[1][scan8[n]];
>
> [...]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
