3
\$\begingroup\$

I have a C++ method that uses the Intel performance primitives library to convert an image from YUV422 format into RGB 24. I am PInvoking this method from C# (WPF) where the YUV images are coming from a high frame rate camera. The method takes a pointer to the image data, and the width and height and returns a pointer to the modified data.

This single method is taking up 23% of my entire application CPU and must be running at all times (the video is always visible).

I am not a C++ maestro and I am sure that this code could be improved significantly in terms of performance, but I am not sure where to start.

Any suggestions for improvements would be most appreciated

extern "C" __declspec(dllexport)unsigned char* YUV422ToRGB24(unsigned char* input, int width, int height)
    {
        const int       ALIGNMENT_64BIT_MASK = 64u;
        int             stepRGB, stepYUV;

        Ipp8u           *rgbImg, *imgYUV422_IPP;

        bool            success = false;
        IppiSize        size;

        UINT32 width3 = width * 3;
        UINT32 width2 = width * 2;

        unsigned char *ptr = NULL;

        unsigned char* pBGR = new unsigned char[(width * height) * 3];

        // ippi functions require 32Bit alignment, if not a

        if ((((uintptr_t)pBGR % ALIGNMENT_64BIT_MASK) == 0) && (width3 % ALIGNMENT_64BIT_MASK == 0))
        {
            rgbImg = pBGR;
            stepRGB = width3;
        }
        else
        {
            rgbImg = ippiMalloc_8u_C3(width, height, &stepRGB);
            if (rgbImg == NULL)
                return false;
        }

        if ((((uintptr_t)input % ALIGNMENT_64BIT_MASK) == 0) && (width2 % ALIGNMENT_64BIT_MASK == 0))
        {
            imgYUV422_IPP = input;
            stepYUV = width2;
        }
        else
        {
            imgYUV422_IPP = ippiMalloc_8u_C2(width, height, &stepYUV);

            if (imgYUV422_IPP == NULL)
            {
                ippiFree(imgYUV422_IPP);
                return false;
            }

            // copy source to yuv
            ptr = imgYUV422_IPP;

            // simple case where step is compatable, can copy entire row
            if (stepYUV == width2) {
                memcpy(ptr, input, height * stepYUV);
            }
            else {
                for (int col = 0; col < height; ++col)
                {
                    memcpy(ptr, input, width2);
                    input += width2;
                    ptr += stepYUV;
                }
            }
        }

        // convert yu422 to rgb
        size.width = width;
        size.height = height;

        if (ippiYCbCr422ToBGR_8u_C2C3R(imgYUV422_IPP, stepYUV, rgbImg, stepRGB, size) == 0)
        {
            // ouptut destination not same location as result, so copy over memory
            if (pBGR != rgbImg) {
                // copy memory to destination
                ptr = rgbImg;

                // simple case where step is compatable, can copy entire row
                if (stepRGB == width3) {
                    memcpy(pBGR, ptr, height * stepRGB);
                }
                else {
                    for (int col = 0; col < height; ++col) {
                        memcpy(pBGR, ptr, width3);
                        pBGR += width3;
                        ptr += stepRGB;
                    }
                }
            }
            success = true;
        }

        // only free memory if not same as pointers provided
        if (pBGR != rgbImg)         ippiFree(rgbImg);
        if (input != imgYUV422_IPP) ippiFree(imgYUV422_IPP);

        return pBGR;
    }
\$\endgroup\$
5
\$\begingroup\$

If your function is called with non-aligned memory, it will allocate two buffers, copy, do the simple fast thing, copy again, and free. That’s a whole lotta work for something simple and fast. And if the step sizes don’t match, the copy will be even slower.

If this the normal execution case, I can imagine this being a significant drain on resources.

You are processing video. Each frame will have the same size. You can keep the internal buffers in this function around from call to call, so you can reuse them and avoid the mallocs.

But even better would be to directly read your video into aligned memory, in the right format for the IPP library. Your functions should just error out if the input pointers are not aligned.

Furthermore, the output data is stored in memory newed. You should pass a data pointer from the caller to write in. If you don’t want to do that, why not directly allocate this output buffer with the right alignment?

I would recommend skipping all the step computations and worries, simply setting

size.width = width * height;
size.height = 1;

If the IPP function believes all pixels are on a single row, then you don't need those padding bytes at the end of odd-sized rows.


There is a bug: In the case that malloc fails, (imgYUV422_IPP == NULL), you free a NULL pointer. Here you probably intended to free the rgbImg buffer if it was allocated earlier.

Similarly, pBGR is never freed on error.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.