Optimization of YUV422 to RGB

Question

I have a stream of YUV422 encoded images coming in from a camera using Direct Show. I then run this byte array through a conversion method and return the RGB array to C# to display in a WPF app.

My problem is that running the application for a 10 minute period and profiling it, the CPU usage is too high for my usage, and it spends around 70-80% of the time in the YUV422 to RGB conversion method.

The method works really well but is just too slow.

Are there any optimizations I am missing here? (I am mainly a C# programmer so I can easily miss C++ optimizations)

*nb this method is in a C++ dll that is "PInvoked" from C# - the reason for C++ is that a similar method takes twice as long in C# than C++

const int STANDARD_SIZE = 1036800;

// Clamp out of range values
#define CLAMP(t) (((t)>255)?255:(((t)<0)?0:(t)))

// Color space conversion for RGB
#define GET_R_FROM_YUV(y, u, v) ((298*y+409*v+128)>>8)
#define GET_G_FROM_YUV(y, u, v) ((298*y-100*u-208*v+128)>>8)
#define GET_B_FROM_YUV(y, u, v) ((298*y+516*u+128)>>8)


BOOL __stdcall YUV422toRGB888(unsigned char *d, unsigned char *s)
{
    for (unsigned int i = 0; i < STANDARD_SIZE; ++i)
    {
        int y0 = *s++ - 16;
        int u0 = *s++ - 128;
        int y2 = *s++ - 16;
        int v = *s++ - 128;

        // BGR
        *d++ = CLAMP(GET_B_FROM_YUV(y0, u0, v));
        *d++ = CLAMP(GET_G_FROM_YUV(y0, u0, v));
        *d++ = CLAMP(GET_R_FROM_YUV(y0, u0, v));

        // BGR
        *d++ = CLAMP(GET_B_FROM_YUV(y2, u0, v));
        *d++ = CLAMP(GET_G_FROM_YUV(y2, u0, v));
        *d++ = CLAMP(GET_R_FROM_YUV(y2, u0, v));
    }

    return true;
}

Answer 1

Be mindful of endianess, and also, some systems are UYUV instead of YUVU.

Sometimes it just takes some trial and error if your docs aren't very good :-(. For myself, the following ordering of YUYV worked great with the rest of the code:

int u0 = *yuv_in++ - 128;
int y0 = *yuv_in++ - 16;
int v = *yuv_in++ - 128;
int y2 = *yuv_in++ - 16;

Answer 2

const int STANDARD_SIZE = 1036800;

Don't use all-caps for identifiers; the convention is that we reserve that for macros. Macros need extra attention: being text substition, they don't obey the rules of scope or precedence.

#define CLAMP(t) (((t)>255)?255:(((t)<0)?0:(t)))

Simpler to use std::clamp() here. And no reason to be a macro rather than an (inlineable) function.

#define GET_R_FROM_YUV(y, u, v) ((298*y+409*v+128)>>8)
#define GET_G_FROM_YUV(y, u, v) ((298*y-100*u-208*v+128)>>8)
#define GET_B_FROM_YUV(y, u, v) ((298*y+516*u+128)>>8)

These should all be functions too, and it's probably easier to clamp them at this point rather than at the call sites. Also, we could reorder the expressions to put the offset of 128 first, to allow the compiler to extract the common subexpression 128+298*y. Also, right shift isn't defined for negative values, so use division instead and/or clamp before shifting.

for (unsigned int i = 0; i < STANDARD_SIZE; ++i)

Why is i unsigned, and being compared with a (signed) int?

Instead of introducing a new variable, consider using one of the existing variables (s or d) to count the loop. Perhaps

auto *const e = d + 6 * STANDARD_SIZE;

while (d < e)

On the other hand, it may be worth keeping the loop iterator, and using that to divide the work between threads, using OpenMP or similar.

    int y0 = *s++ - 16;
    int u0 = *s++ - 128;
    int y2 = *s++ - 16;
    int v = *s++ - 128;

Instead of subtracting here, why not leave it until the calculation functions, where we are adding a constant to the result?

    return true;

This seems useless, since we never return any other value. We should declare the function to return void.

---

Modified code

Re-written in more modern C++:

#include <algorithm>

constexpr unsigned int image_length = 1036800;

namespace {
    void write_bgr(unsigned char *d, int y, int u, int v) {
        *d   = std::clamp(298*y + 516*u - 70688, 0, 65535) / 256;
        *++d = std::clamp(298*y - 100*u - 208*v - 44064, 0, 65535) / 256;
        *++d = std::clamp(298*y + 409*v - 56992, 0, 65535) / 256;
    }
}

void yuv422_to_bgr888(unsigned char *dest, unsigned char *src)
{
#pragma omp parallel for
    for (unsigned int i = 0u;  i < image_length;  ++i) {
        auto *s = src + 4 * i;
        int y0 = *s;
        int u0 = *++s;
        int y2 = *++s;
        int v =  *++s;

        auto *d = dest + 6 * i;
        write_bgr(d,   y0, u0, v);
        write_bgr(d+3, y2, u0, v);
    }
}