We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

I've added SDL_UpdateNVTexture()
for software/opengl/opengles2 only
https://hg.libsdl.org/SDL/rev/d3c71331483d

Created attachment 4639 [details]
test-case

Test-case

When using the software renderer, there is an invalid read:

==75622== Invalid read of size 16
==75622==    at 0x4A70357: yuvnv12_argb_sseu (SDL_hg/src/video/yuv2rgb/yuv_rgb_sse_func.h:432)
==75622==    by 0x49BCFBB: yuv_rgb_sse (SDL_hg/src/video/SDL_yuv.c:279)
==75622==    by 0x49BC173: SDL_ConvertPixels_YUV_to_RGB (SDL_hg/src/video/SDL_yuv.c:416)
==75622==    by 0x49AD462: SDL_ConvertPixels_REAL (SDL_hg/src/video/SDL_surface.c:1396)
==75622==    by 0x48ED210: SDL_SW_CopyYUVToRGB (SDL_hg/src/render/SDL_yuv_sw.c:426)
==75622==    by 0x48E48D0: SDL_UpdateTextureNVPlanar (SDL_hg/src/render/SDL_render.c:1674)
==75622==    by 0x48E44FB: SDL_UpdateNVTexture (SDL_hg/src/render/SDL_render.c:1794)
==75622==    by 0x4014A9: main (test_bug_5430_yuv.c:90)

fixed the read access by using align memory: https://hg.libsdl.org/SDL/rev/c124e24fcf74
Not absolutly sure if there isn't something wrong

Add the D3D11 UpdateNVTexture: https://hg.libsdl.org/SDL/rev/7f3f477d9101
but this isn't tested

@cmediaplayer:

can you give a try to use the SDL_UpdateNVTexture():

first using the software renderer.

SDL_SetHint(SDL_HINT_RENDER_DRIVER, "software");


when it works, use the D3D11 renderer. It should work immediately.
it not, maybe some offset to play with:
https://hg.libsdl.org/SDL/rev/7f3f477d9101#l1.24
(UVpitch vs UVpitch/2 ...)

fix compilation:
https://hg.libsdl.org/SDL/rev/2f39e7dee61f
https://hg.libsdl.org/SDL/rev/fa6f0a8d7420

Also added for METAL: https://hg.libsdl.org/SDL/rev/724e9980492e
(not tested)

Fix warning on METAL: https://hg.libsdl.org/SDL/rev/6bdd3c5638e1

Created attachment 4640 [details]
test_direct3d11_nv12

Hi, thanks for the patches. When i testing the new functions for direct3d11 renderer, half of screen becomes green. You can see it at the attachment.

By the way, I have tested opengl and opengles2 and works properly.

what if you change: 
https://hg.libsdl.org/SDL/rev/7f3f477d9101#l1.24

UVpitch / 2 by UVpitch ?

The correct parameters have to be like this:

if (D3D11_UpdateTextureInternal(rendererData, textureData->mainTextureNV, SDL_BYTESPERPIXEL(texture->format), rect->x, rect->y, rect->w, rect->h / 2, UVplane, UVpitch) < 0) {
        return -1;
}

After this change, it works perfectly :)

(In reply to cmediaplayer from comment #14)
> The correct parameters have to be like this:
> 
> if (D3D11_UpdateTextureInternal(rendererData, textureData->mainTextureNV,
> SDL_BYTESPERPIXEL(texture->format), rect->x, rect->y, rect->w, rect->h / 2,
> UVplane, UVpitch) < 0) {
>         return -1;
> }
> 
> After this change, it works perfectly :)

I'm pretty sure it should be this:
if (D3D11_UpdateTextureInternal(rendererData, textureData->mainTextureNV, SDL_BYTESPERPIXEL(texture->format), rect->x, rect->y / 2, rect->w, rect->h / 2, UVplane, UVpitch) < 0) {
    return -1;
}

The NV plane is full width, half height.

I commited cmediaplayer tested https://hg.libsdl.org/SDL/rev/84ac170523cc

But I thought it should be the same as:
SDL_UpdateTexture path
https://hg.libsdl.org/SDL/file/7f3f477d9101/src/render/direct3d11/SDL_render_d3d11.c#l1368

D3D11_UpdateTextureInternal(rendererData, textureData->mainTextureNV, 2, rect->x / 2, rect->y / 2, rect->w / 2, rect->h / 2, UVplane, UVpitch) 


 
If you have the possibility there is METAL renderer to double-check

(In reply to Sam Lantinga from comment #15)
> (In reply to cmediaplayer from comment #14)
> > The correct parameters have to be like this:
> > 
> > if (D3D11_UpdateTextureInternal(rendererData, textureData->mainTextureNV,
> > SDL_BYTESPERPIXEL(texture->format), rect->x, rect->y, rect->w, rect->h / 2,
> > UVplane, UVpitch) < 0) {
> >         return -1;
> > }
> > 
> > After this change, it works perfectly :)
> 
> I'm pretty sure it should be this:
> if (D3D11_UpdateTextureInternal(rendererData, textureData->mainTextureNV,
> SDL_BYTESPERPIXEL(texture->format), rect->x, rect->y / 2, rect->w, rect->h /
> 2, UVplane, UVpitch) < 0) {
>     return -1;
> }
> 
> The NV plane is full width, half height.

Yep, Sam is right. it should be like this:

if (D3D11_UpdateTextureInternal(rendererData, textureData->mainTextureNV, SDL_BYTESPERPIXEL(texture->format), rect->x, rect->y / 2, rect->w, rect->h / 2, UVplane, UVpitch) < 0) {
    return -1;
}

I tested and it works.

(In reply to Sylvain from comment #16)
> I commited cmediaplayer tested https://hg.libsdl.org/SDL/rev/84ac170523cc
> 
> But I thought it should be the same as:
> SDL_UpdateTexture path
> https://hg.libsdl.org/SDL/file/7f3f477d9101/src/render/direct3d11/
> SDL_render_d3d11.c#l1368
> 
> D3D11_UpdateTextureInternal(rendererData, textureData->mainTextureNV, 2,
> rect->x / 2, rect->y / 2, rect->w / 2, rect->h / 2, UVplane, UVpitch) 

That might be a bug in the SDL_UpdateTeture path. Can someone verify?

@cmediaplayer, are the Y and NV planes separately allocated chunks of memory? If they were contiguous, you could just use SDL_UpdateTexture(), as Sylvain pointed out.

I test with a non fullscreen rect, 
https://hg.libsdl.org/SDL/rev/38bf617e797a

D3D11_UpdateTextureInternal(rendererData, textureData->mainTextureNV, SDL_BYTESPERPIXEL(texture->format), rect->x / 2, rect->y / 2, rect->w, rect->h / 2, UVplane, UVpitch)



Also, fix software renderer:
https://hg.libsdl.org/SDL/rev/b836d9333e26

(In reply to Sam Lantinga from comment #19)
> @cmediaplayer, are the Y and NV planes separately allocated chunks of
> memory? If they were contiguous, you could just use SDL_UpdateTexture(), as
> Sylvain pointed out.

FFmpeg library separately allocates chunks of memory for the Y and UV(U and V together) planes. I use the function implemented like this:

int ret = SDL_UpdateNVTexture(pSDLTexture,NULL, frame->data[0], frame->linesize[0], frame->data[1], frame->linesize[1];

That's why I asked for kind of this function.

I've also tried METAL renderer which is ok with the test

(In reply to Sylvain from comment #20)
> I test with a non fullscreen rect, 
> https://hg.libsdl.org/SDL/rev/38bf617e797a
> 
> D3D11_UpdateTextureInternal(rendererData, textureData->mainTextureNV,
> SDL_BYTESPERPIXEL(texture->format), rect->x / 2, rect->y / 2, rect->w,
> rect->h / 2, UVplane, UVpitch)
> 
> 
> 
> Also, fix software renderer:
> https://hg.libsdl.org/SDL/rev/b836d9333e26

My media player automatically detects which renderers support which pixel formats. Software and opengles renderers don't support nv12/21 pixel formats in my app. Because those renderers do not return nv texture format. Others do. Should be something wrong software and opengles implementation. Can you check for it?

software render somehow supports all pixel format as an output.
(but blitting in-between differe YUV/NV surfaces I think).

maybe we could increase the list here:

https://hg.libsdl.org/SDL/file/38bf617e797a/src/render/software/SDL_render_sw.c#l871

Created attachment 4648 [details]
test-case

Update the test-case so it can  use SDL_UpdateTexture.

I've double-checked D3D11 syntax for SDL_UpdateTexture(), and it's ok.
The difference is with the 3rd paramterer BPP : 
2 in SDL_UpdateTexture vs SDL_BYTESPERPIXEL(texture->format) (eg 1).

I updated the code to match SDL_UpdateTexture notation:
https://hg.libsdl.org/SDL/rev/a89a79f8ac30


Marking the issue as resolved

fix for opengles2: 
https://hg.libsdl.org/SDL/rev/e10fb6ab0d77

when updating a non fullscreen rect