Opened 10 months ago

Last modified 10 months ago

#10514 new defect

swscale alignment requirements (and issues) in YUV420P to RGB conversions

Reported by: vkonovets Owned by:
Priority: normal Component: undetermined
Version: 4.3.6 Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description (last modified by vkonovets)

I'm working on an API which wraps around swscale to do conversions on memory not strictly allocated for AVFrame (i.e my own allocations). For some image dimensions I noticed visual artifacts and memory corruption. Although there are no alignment requirements for rows stated in the documentation, reading from Stack Overflow and various tickets here I understood that there is a minimum alignment requirement for allocations and padding at the end of the row (although this does not solve the issue).

https://trac.ffmpeg.org/ticket/9331 points out what I believe is the issue, however that ticket was dismissed because the memory allocation was not aligned (I ensure it is in my sample). To quote:

the assembly code accelerates the process of convert yuv to rgb by using sse2 register... when the width of video frame is not a multiply of 16, extra wrong yuv pixel is read from the memory and writing extra rgb value to the memory allocated for the output image and cause the invalid writing.

From that ticket and get_video_buffer() source code (https://ffmpeg.org/doxygen/4.3/frame_8c_source.html#l00109), which allocates memory for AVFrame, it follows that not just the row that needs to be padded, but also the row dimension needs to be padded.

I attach to this ticket sample code which takes width as an argument which you can use to observe the effects:

#include <stdlib.h>
extern "C"
{
#include <libswscale/swscale.h>
}

bool absDiffTol(unsigned char a, unsigned char b, int tol)
{
    return abs((int)a - (int)b) <= tol;
}

int main(int argc, char** argv)
{
    int width = atoi(argv[1]);
    int height = 8;
    int alignment = 64;

    // YUV420P image
    int srcLinesize[8] = {width - width % alignment + alignment,
                          width / 2 - (width / 2) % alignment + alignment,
                          width / 2 - (width / 2) % alignment + alignment,
                          0, 0, 0, 0, 0};

    // RGB image
    int dstLinesize[8] = {width * 3 - (width * 3) % alignment + alignment,
                          0, 0, 0, 0, 0, 0, 0};


    unsigned char* srcData[8];
    srcData[0] = (unsigned char*)aligned_alloc(alignment, srcLinesize[0] * height);
    srcData[1] = (unsigned char*)aligned_alloc(alignment, srcLinesize[1] * height);
    srcData[2] = (unsigned char*)aligned_alloc(alignment, srcLinesize[2] * height);

    unsigned char* dstData[8];
    dstData[0] = (unsigned char*)aligned_alloc(alignment, dstLinesize[0] * height);

    // Fill YUV420 with green
    for (int y(0); y < height; y++)
    {
        for(int x(0); x < width; x++)
        {
            srcData[0][y * srcLinesize[0] + x] = 145;
            srcData[1][(y / 2) * srcLinesize[1] + x / 2] = 54;
            srcData[2][(y / 2) * srcLinesize[2] + x / 2] = 34;
        }
    }

    SwsContext* ctx = sws_getContext(width, height, AV_PIX_FMT_YUV420P,
                                     width, height, AV_PIX_FMT_RGB24,
                                     0, nullptr, nullptr, nullptr);
    if (!ctx)
    {
        printf("Could not allocate context\n");
        return -1;
    }

    if(sws_scale(ctx, srcData, srcLinesize, 0, height, dstData, dstLinesize) != height)
    {
        printf("sws_scale returned with error.\n");
        return -1;
    }

    // Check RGB is in fact green
    for (int y(0); y < height; y++)
    {
        for (int x(0); x < width; x++)
        {
            unsigned char r = dstData[0][y * dstLinesize[0] + x*3];
            unsigned char g = dstData[0][y * dstLinesize[0] + x*3 + 1];
            unsigned char b = dstData[0][y * dstLinesize[0] + x*3 + 2];
            if (!absDiffTol(r, 0, 1) || !absDiffTol(g, 255, 1) || !absDiffTol(b, 0, 1))
            {
                printf("Pixel at (%d, %d) has unexpected value [%d, %d, %d]\n", x, y, r, g, b);
            }
        }
    }

    return 0;
}

The sample program was linked against ffmpeg version:

ffmpeg version 4.3.6-0+deb11u1 Copyright (c) 2000-2023 the FFmpeg developers
  built with gcc 10 (Debian 10.2.1-6)

You can run

for ((i=16;i<=512;i+=2)); do valgrind --tool=memcheck ./sample $i |& tee out$i.log; done

and grep "Invalid " ./* in the generated logfiles to observe which widths produce invalid reads and writes.

I would also appreciate the following clarifications:

  • get_video_buffer() pads the height during allocation. Why is this necessary? Is this to do with swscale or another component of libav?
  • Are there padding requirements for grayscale, grayscale+alpha, RGBA formats?
  • Is there some source I could refer to for alignment requirements on memory (forum posts, tickets, documentation) for any generic input/output pixel format?

Change History (1)

comment:1 by vkonovets, 10 months ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.