SDL_Surface should handle non-contiguous planar data

[icculus]

Maybe we change SDL_Surface to permit multiple planes/pitches?

I think we need to do this for SDL3, to clearly represent YUV surfaces.

Originally posted by @slouken in https://github.com/libsdl-org/SDL/issues/8633#issuecomment-1858873878

[icculus]

(I'm going to assign this to @1bsyl because he was interested, but let me know if you want me to mess with it instead, Sylvain!)

[1bsyl]

ok, so I gave try, and it's kind of complicated if we want to handle internally yuv non-contiguous surface because there is a bunch of (complicated) yuv conversion functions in SDL_yuv.c that would need to be changed and re-validated.

instead, maybe: we can add pitch2/3 pixels2/3 (as suggested ryan) so existing code still compile. internally everything keep expecting and handling contiguous data (eg yuv surface creation, etc) but we allow non contiguous yuv surfaces for some API. eg SDL_ConvertSurface* by creating a temporary "packed" surface.

so that:

• video capture can create manually non contiguous surfaces. so, no overhead of re-packing.
• people can convert this surface if they want (with an overhead of re-packing).
• people can use this surface (pixels, pixel2, pixel3) with SDL_UpdateYUVTexture (no overhead).

not tested at all.. maybe some function need to block or updated for YUV non contiguous surface :/ fillrect / blit / create texture from surface

[icculus]

Do we have an automated test that can check these cases (filling a YUV surface, etc) so we can just run down this as a checklist and make sure it all works, once we make changes?

[1bsyl]

not really, (testyuv, testoverlay) exists, but that's better to create a new one.

we need, to check all combinations of format conversion of { RGB, YUV, ... } (maybe not all RGB formats should be involved, just a few I guess).

as input, we should have YUV contiguous, and YUV non contiguous and with various pitch maybe.

I can start to see how it goes, but I am also going to short of time in the next few weeks.

[1bsyl]

proposing a specific test-case https://github.com/libsdl-org/SDL/pull/8715
to test conversion and mode (contiguous/non contiguous conversion).

(probably partially redundant, with testyuv, but simpler also.)

[slouken]

I took a look at this as well, and while I have a decent plan, I'm not sure where this is applicable in the SDL API. SDL software rendering with YUV textures already uses a multi-planar internal representation, and we don't currently support blitting YUV surfaces.

We have a pixel format conversion function, but it takes raw pointers instead of an SDL surface, and we would just create a new function that takes a set of planes rather than a surface.

What was the use case for this again? :)

[icculus]

What was the use case for this again? :)

Cameras on Android, etc, that provide YUV planar data but not necessarily in a contiguous block.

If the SDL_Surface could represent this directly, instead of assuming the U data immediately follows the Y data and V follows last without a gap, then we could have data in an SDL_Surface go from a DMA transfer from the camera all the way to the app without a single CPU copy.

As it stands now, we'll have to see if the incoming data is in separate buffers and pack it down to one memory block of planar data. Which might be fine!

But since we'll need to change the SDL_Surface structure to support non-contiguous data, the question is if this is worth doing before the ABI freeze, or not doing at all.

[1bsyl]

And if we allow SDL_Surface with multi planes, other SDL api has to handle it. at least SDL_ConvertSurface().

We don't need to change SDL_ConvertPixels public API, it can still handle 1 buffer.

but internally SDL_ConvertSurface use SDL_ConvertPixels. so we need to change internal SDL_ConvertPixels / make an internal more generic function that takes multiple planes. and forward the planes, to all sub SDL_yuv.c convert function. (and fix them).

(OR: we pack the surface in ConvertPixels temporary to make it contiguous, that was the previous PR / attempt. much simpler)

.

[icculus]

Oh, and it needs scaling and conversion, but not blits nor fills.

[1bsyl]

it's possible that scaling goes with an intermediat RGB format .. not sure ..

[1bsyl]

BTW, there's also the 0-copy patch, that could also be interesting. it add 1 id per-frame. so that a frame can be used as a texture. (this could be a SDL_Properties maybe)

[slouken]

What was the use case for this again? :)

Cameras on Android, etc, that provide YUV planar data but not necessarily in a contiguous block.

If the SDL_Surface could represent this directly, instead of assuming the U data immediately follows the Y data and V follows last without a gap, then we could have data in an SDL_Surface go from a DMA transfer from the camera all the way to the app without a single CPU copy.

As it stands now, we'll have to see if the incoming data is in separate buffers and pack it down to one memory block of planar data. Which might be fine!

But since we'll need to change the SDL_Surface structure to support non-contiguous data, the question is if this is worth doing before the ABI freeze, or not doing at all.

Okay, let's say you have a surface with planar data, what is the flow of SDL API calls that you would expect to do with it?

[icculus]

Right now we feed the camera backend an SDL_Surface without any backing (SDL_CreateSurfaceFrom with NULL pixels), and the backend sets the surface's pixels and pitch.

On Linux at least, this is often an mmap'd buffer the device DMA'd data directly into.

If we need to convert format or scale the image on behalf of the app, we do so into a surface with backing (SDL_CreateSurface). In those cases, we obviously have to copy, but if not, the app gets a Surface with the mmap'd buffer directly.

The surfaces are created in a pool at device open time, so in the optimal case, we're just setting a pointer and handing it to the app.

[icculus]

For my needs, it only needs to understand planar data enough to convert to/from it, and resize it with nearest-neighbor scaling.

What apps want to do might be more. But they can just ask for RGB data, so it's just a "normal" surface when it arrives.

[slouken]

Can you outline a sequence of SDL calls, so we know what APIs could be affected?

[slouken]

Okay, I have an idea. Rather than extend the surface structure, which causes lots of headaches for binary compatibility and sdl2-compat, let's use properties.

SDL_Surface, no properties, code assumes compact YUV planes, the way it currently works. SDL_Surface with SDL.surface.plane0, SDL.surface.pitch0, SDL.surface.plane1, SDL.surface.pitch1, etc. properties could be separate plane representation of the surface, with pixels being NULL. We could even name the planes appropriately for the surface format if we wanted to be fancy, e.g. SDL.surface.plane_y, SDL.surface.pitch_y, SDL.surface.plane_uv, SDL.surface.pitch_uv for NV12 data, etc.

We could even stuff things like DRM handles and so forth in properties, if we really wanted.

[slouken]

@icculus and I discussed this, and we think that we want to keep surfaces simple and add an advanced API that can return an object with multiple YUV plans and/or hardware surface representations.