Apparent interlacing

[padreputativo]

The videos that don’t appear interlaced in other players, or even in the library’s own player, do appear interlaced in Defold. I think it might have to do with the fact that if the texture isn’t perfectly square, the video doesn’t display fully, and if it is, this effect might appear. It’s probably related to printing a texture using UV mapping.

https://github.com/user-attachments/assets/cf5adc01-5941-4429-9224-df736203c99c

[britzl]

I think it might have to do with the fact that if the texture isn’t perfectly square, the video doesn’t display fully, and if it is, this effect might appear. It’s probably related to printing a texture using UV mapping.

Have you tried changing how it is rendering the video texture?

[padreputativo]

The file is not interlaced, I’ve checked it with FFmpeg and Blender. According to the library’s author, it doesn’t use interlacing either. It must be that when extracting the texture, the calculation isn’t being done correctly, as you can see in this image with the scale set to 4.

[padreputativo]

I think it might have to do with the fact that if the texture isn’t perfectly square, the video doesn’t display fully, and if it is, this effect might appear. It’s probably related to printing a texture using UV mapping.

Have you tried changing how it is rendering the video texture?

I'm thinking that maybe it's because I had already modified the script before, as it was set to a different resolution than mine and was distorting the image.

[padreputativo]

It seems that's not the case; I just tried the version from the repository, and it's doing the same thing.

I suspect that the values in mpeg.cpp, since they aren't variables, might not be considering higher resolutions than the standard or something similar.

[padreputativo]

It seems that's not the case either; it also appears in the Big Buck Bunny video (though I’m not sure what resolution it has).

[padreputativo]

I've tried changing sizes, scale, the atlas, the material... but I suppose that if increasing the scale also increases the size of the interlacing, it has more to do with the frame extraction itself.

[padreputativo]

More information from the library creator: https://github.com/phoboslab/pl_mpeg/issues/44

[padreputativo]

I tried dumping the frame information to a PNG file with the extension, and it seems to be corrupted.

https://github.com/britzl/defold-png

Although it's likely that I am not processing the information correctly.

[JCash]

I'd say that since we can produce a texture in the scripts, the data is ok, and you need to look at how you produce the data for the png writer.

[padreputativo]

Lua interprets the type as userdata and I'm not sure if it has methods or if it's a table or what is inside it, but converting it to a string to pass it through the extension might result in losing all the information.

[padreputativo]

Got it:

local text = buffer.get_bytes(videoframe, "<unknown>")

This means that the error is in mpeg.cpp.

[padreputativo]

It seems to be the line that flips the image:

//int d_index = (rows - row - 1) * 2 * stride; int d_index = row * 2 * stride;

It can be fixed by applying a scale -1 to the sprite, but that's probably not the expected solution to consider the bug resolved xD In that case, it might be better to create an RGB conversion shader as recommended by the library itself.

[padreputativo]

I managed to do it, but I'm not sure if you'll like the way I've done it. Since some information is calculated every two rows by default in MPEG, what I did was use the plm_frame_to_rgb function from the library itself and then add an additional step to perform a flip with the following code:

void frame_flip(plm_frame_t *frame, uint8_t *dest, int stride) {
    int row_size = frame->width * 3;
    uint8_t *temp_row = new uint8_t[row_size];

    for (int row = 0; row < frame->height / 2; row++) {
        uint8_t *top_row = dest + row * stride;
        uint8_t *bottom_row = dest + (frame->height - row - 1) * stride;

        memcpy(temp_row, top_row, row_size);

        memcpy(top_row, bottom_row, row_size);
        memcpy(bottom_row, temp_row, row_size);
    }

    delete[] temp_row;
}

And then where the call was made...

    // decode frame to buffer
    int w = plm_get_width(video->m_plm);
    plm_frame_to_rgb(frame, data, w * 3);
    frame_flip(frame, data, w * 3);

[JCash]

Nice find.

However, as we're concerned about performance, I do wonder if 1) the decoder has a flip options, or 2), we should simply use a separate shader, letting the GPU flip the uv coordinates.

The GPU is a lot faster than our CPU flip, so it might make sense to use a separate shader instead.

[padreputativo]

I totally agree; in fact, I had it implemented in my case with the scale set to -1 on the sprite precisely because I felt it would consume less, as those calculations would still be handled by OpenGL when placing the sprite, even with the scale set to -1. So I had it done that way, but I assumed that perhaps this option was more user-friendly. However, the -1 scale trick is very simple. I'm not sure about that, although in that case, the YUV to RGB conversion and the flip would be done simultaneously by the GPU, potentially making it even faster than the -1 scale option and CPU conversion.

[britzl]

I'm also thinking that it's better to do it in a shader. And like you said @padreputativo I think there was some mention about doing the YUB to RGB conversion using a shader instead. Flipping at the same time would be a nice bonus.

[padreputativo]

I've been trying for a while to extract the data directly in YUV, but I haven't been able to do it. I also haven't seen a way to do it without converting to RGB, but it's probably because I'm missing something... If looping is mandatory, I don't think it would make much of a difference whether the calculations are done there or in a shader...

[JCash]

Doing the work in a shader is a huge performance win.

[padreputativo]

Was the audio not implemented due to some limitation of the engine at that time? (I'm trying to understand all the code in case I can extend it myself)

It seems that in the official repository, there's a shader-based implementation of a video player, but half of the things are beyond my current level of C++

https://github.com/phoboslab/pl_mpeg/blob/master/pl_mpeg_player.c

[britzl]

We have no way for the native extension system to hook into the audio system in Defold. One option is to replace the Defold sound system and use something else. Examples of this:

• https://defold.com/assets/fmod/ (adds an additional sound system)
• https://defold.com/assets/blip/ (removes the Defold sound system and uses openal)

Another option is to extract the audio stream and play that separately as an ogg file.

[padreputativo]

I'm not sure if it's supposed to look like that in YUV, and I think it probably shouldn't, haha.

void plm_frame_to_yuv(plm_frame_t *frame, uint8_t *dest, int stride) {
    int y_size = frame->width * frame->height;
    int uv_size = (frame->width / 2) * (frame->height / 2);

    memcpy(dest, frame->y.data, y_size);
    memcpy(dest + y_size, frame->cb.data, uv_size);
    memcpy(dest + y_size + uv_size, frame->cr.data, uv_size);
}

[padreputativo]

To store YUV data in an RGB texture, a prior conversion of the data is necessary. This involves processing all the information through a couple of loops. The way it is done in the official repository is by separating the three layers into different textures (or rather data banks, since the U and V layers contain negative values), which makes it easier to convert them. However, using a single texture and having to convert it beforehand doesn’t seem to make much sense. (At least I’ve learned to create shaders in Defold.) Perhaps it would work with a YCRCB texture format instead of RGB. There is the option to use three textures, but I’m not familiar enough with Defold and/or C++ to know how to do it. It would be easier to read in the shader because it would involve using UVs from 0 to 1 and everything would always be where it should be. I don't know if the library itself would allow "sharing" those memory areas without the need to make copies for later use in the shader.

Maybe I’m mistaken, and it might be possible to store the data in an RGB 32 texture or something like that because the data ranges from 0 to 16k in the "Y" layer and from -128 to 128 in the "U" and "V" layers.

Another thing that would be very useful is to be able to use the repository as a dependency since, as there is currently no release, it is not possible.

[britzl]

I took another look at the YCrCb to RGB conversion and the flip I did was a bit naive, because the decoder is not purely writing pixels one line after the other. There is also a stride and it writes two color values in one row and two in another. When naively flipping the indices the result looks wrong.

This should be the correct conversion and flip if I'm not mistaken. Try this instead and see if you get the results you are after. To my eye it looks correct at least:

void plm_frame_to_rgb_flip(plm_frame_t *frame, uint8_t *dest, int stride) {
    int cols = frame->width >> 1;
    int rows = frame->height >> 1;
    int yw = frame->y.width;
    int cw = frame->cb.width;
    for (int row = 0; row < rows; row++) {
        int c_index = row * cw;
        int y_index = row * 2 * yw;
        int d_index = (rows - row - 1) * 2 * stride;
        for (int col = 0; col < cols; col++) {
            int y;
            int cr = frame->cr.data[c_index] - 128;
            int cb = frame->cb.data[c_index] - 128;
            int r = (cr * 104597) >> 16;
            int g = (cb * 25674 + cr * 53278) >> 16;
            int b = (cb * 132201) >> 16;
            y = ((frame->y.data[y_index + 0]-16) * 76309) >> 16;
            dest[d_index + stride + RI] = plm_clamp(y + r);
            dest[d_index + stride + GI] = plm_clamp(y - g);
            dest[d_index + stride + BI] = plm_clamp(y + b);
            y = ((frame->y.data[y_index + 1]-16) * 76309) >> 16;
            dest[d_index + stride + BYTES_PER_PIXEL + RI] = plm_clamp(y + r);
            dest[d_index + stride + BYTES_PER_PIXEL + GI] = plm_clamp(y - g);
            dest[d_index + stride + BYTES_PER_PIXEL + BI] = plm_clamp(y + b);
            y = ((frame->y.data[y_index + yw]-16) * 76309) >> 16;
            dest[d_index + 0 + RI] = plm_clamp(y + r);
            dest[d_index + 0 + GI] = plm_clamp(y - g);
            dest[d_index + 0 + BI] = plm_clamp(y + b);
            y = ((frame->y.data[y_index + yw + 1]-16) * 76309) >> 16;
            dest[d_index + BYTES_PER_PIXEL + RI] = plm_clamp(y + r);
            dest[d_index + BYTES_PER_PIXEL + GI] = plm_clamp(y - g);
            dest[d_index + BYTES_PER_PIXEL + BI] = plm_clamp(y + b);            c_index += 1;
            y_index += 2;
            d_index += 2 * BYTES_PER_PIXEL;
        }
    }
}

[britzl]

Another thing that would be very useful is to be able to use the repository as a dependency since, as there is currently no release, it is not possible.

It is still possible:

https://github.com/defold/extension-videoplayer-mpeg/archive/refs/heads/master.zip

But it is not recommended to depend on the master branch because it can change at any time. Let's make a release after we fix the interlacing!

[padreputativo]

Now it looks perfect! (or at least I think so) Yes, the problem was that the 'Y' layer is full resolution, but 'U' and 'V' are at half resolution.

[britzl]

Closed via https://github.com/defold/extension-videoplayer-mpeg/commit/59918fa5e030d1381540fd7a49b9b02e753b52c9

[britzl]

Let's make a release after we fix the interlacing!

Released 1.0.0: https://github.com/defold/extension-videoplayer-mpeg/releases/tag/1.0.0

[padreputativo]

Many thanks!