SDL_CreateSurfaceFrom and pitch

What is pitch in SDL_CreateSurfaceFrom?
If I enter 0 it doesn’t work. With 8 it works, with certain pixel formats 4 also worked.

What does this pitch do?

Pitch is the number of bytes that it takes to go from the start of one row to the start of the next row.

In other words:

pitch = image-width × bytes-per-pixel + padding-between-rows

2 Likes

Thanks
I looked in the wiki again and it is described there if you look closely.

What I don’t quite understand is that it says that no copy of the pixel data is created.
But why doesn’t it work with SDL_DestroySurface?
As is the case here? [SDL 3.x] SDL_CreateSurface and SDL_DestroySurface = SIGSEV - #3 by Mathias

I have another suggestion for improvement. That SDL_CreateSurfaceFrom would calculate the pitch itself if you pass 0. Based on the width and the pixel format, this should work.

That’s because the SrcSurface->pixels pointer was loaded with a pointer to local data. That data in the original post was created on the stack (local memory), not in the heap (dynamic memory).
But memcpy copies data onto the heap so I believe that solved the issue.

SDL_DestroySurface expects the pixels pointer to be on the heap and de-allocates the pixels pointer, which causes the seg fault error.

In the case of SDL_CreateSurfaceFrom, it is not creating a copy of the data either, it is directly handing your pixel pointer to the surface->pixels handle. So if you hand it a pointer to local data, then it’s just the same problem only hidden inside of the source code which makes it harder to track down.

I don’t know of any standard way by looking at a pointer to tell if that memory exists on the stack vs the heap…

You’re not using SDL_CreateSurfaceFrom in your other thread.

That’s generally not possible because it doesn’t know if there is any padding (unused bytes) between rows.

It could for example be used to create a SDL_Surface that refers to a sub-rectangle of a bigger image.

There could even be padding when creating a fresh surface with SDL_CreateSurface:

SDL_Surface* s = SDL_CreateSurface(10, 100, SDL_PIXELFORMAT_RGB24);
printf("%d\n", s->w * 3); // prints 30
printf("%d\n", s->pitch); // prints 32 (at least for me) 

In this case I think the padding is added to make sure each row is 4-byte aligned which apparently makes certain things more efficient (don’t ask me about the details).

@GuildedDoughnut

I know the docs for SDL_Surface says pixels is “read-write” but I have always interpreted that to mean we’re allowed to write to the pixel data but I don’t think we’re supposed to modify the pixels pointer itself directly. To be able to do that we would need to know how SDL expects the pixel array to be allocated (does it use malloc, a memory pool or what?) so that the deallocation method that SDL_DestroySurface uses matches but the docs doesn’t seem to say anything about that.

When using SDL_CreateSurfaceFrom it doesn’t matter where the pixel array is allocated as long as it stays alive until after you’re finished using the surface because SDL_DestroySurface will not try to deallocate it.

The memory allocation is different everywhere.
malloc, new are not compatible with each other.
Or if you use Pascal, the Getmem and Freemem are incompatible with C malloc and free.
Therefore it is best to use the functions, assuming there are any, from the library you are using. E.g. SDL_malloc and SDL_free

But are you sure that is what SDL (always) uses (and promises to use even in the future) to allocate the pixels array for surfaces?

I did a little research in the sources.
SDL_surface.c line 237 says the following:

surface->flags |= SDL_PREALLOC;

And for SDL_DestroySurface this. So it is clear who has to be released or not.
line: 1919

if (surface->flags & SDL_PREALLOC) {
/* Don't free */
} else if (surface->flags & SDL_SIMD_ALIGNED) {
/* Free aligned */
SDL_aligned_free(surface->pixels);
} else {
/* Normal */
SDL_free(surface->pixels);
}
1 Like

Here’s the source for SDL_DestroySurface:

void SDL_DestroySurface(SDL_Surface *surface)
{
    if (!surface) {
        return;
    }
    if (surface->flags & SDL_DONTFREE) {
        return;
    }
    if (--surface->refcount > 0) {
        return;
    }

    if (surface->flags & SDL_SURFACE_USES_PROPERTIES) {
        SDL_PropertiesID props = (SDL_PropertiesID)(uintptr_t)surface->reserved;
        SDL_DestroyProperties(props);
    }

    SDL_InvalidateMap(surface->map);
    SDL_InvalidateAllBlitMap(surface);

    while (surface->locked > 0) {
        SDL_UnlockSurface(surface);
    }
#if SDL_HAVE_RLE
    if (surface->flags & SDL_RLEACCEL) {
        SDL_UnRLESurface(surface, 0);
    }
#endif
    if (surface->format) {
        SDL_SetSurfacePalette(surface, NULL);
        SDL_DestroyPixelFormat(surface->format);
        surface->format = NULL;
    }
    if (surface->flags & SDL_PREALLOC) {
        /* Don't free */
    } else if (surface->flags & SDL_SIMD_ALIGNED) {
        /* Free aligned */
        SDL_aligned_free(surface->pixels);
    } else {
        /* Normal */
        SDL_free(surface->pixels);
    }
    if (surface->map) {
        SDL_FreeBlitMap(surface->map);
    }
    SDL_free(surface);
}

@Peter87: Ah, I see you are correct because SDL_CreateSurfaceFrom does in fact set the flag that Mathias mentioned.

Otherwise the default behavior of SDL_DestroySurface would be to expect that the pixels pointer is allocated by SDL. That’s where we see /*Normal */ in the code above.

There is plenty of code out there in the wild that accesses the surface pixels data directly for streaming purposes, such as grabbing the webcam using OpenCV or using gstreamer to stream video(both from internet and local files). As long as the surface is set up correctly at the beginning and you follow the same size and padding rules etc. it works great… But there are a lot of old posts that followed those tutorials who say they are getting seg faults. I see that SDL_CreateRGBSurfaceFrom() has been around since SDL1 and it is probably the correct way of going about it. Sorry about this paragraph, I had to walk myself through the idea that “Just because I’ve seen it done in tutorials doesn’t mean it’s the right way to do it”.

@Mathias: I’ve never seen anyone using the SDL_PREALLOC flag before and did not know about it myself, I have to say that was a great find. It is also a good answer for all those old posts that I just mentioned, because it gives them the chance to do the memory cleanup using the library that generate the pixel data to begin with.

1 Like

That’s what happens when no flag is set but when looking inside SDL_CreateSurface it uses looks like it uses SDL_aligned_alloc / SDL_SIMD_ALIGNED.

I don’t have an issue with this as long as you just overwrite the already existing pixel data. In that case you don’t need to care whether it’s using SDL_malloc or SDL_aligned_alloc because it’s all taken care of internally by the SDL library.

What I questioned was the practice of reassigning the pixels pointer in the SDL_Surface struct so that it points somewhere else like Mathias did in his other thread.

2 Likes

I think it will be best to never change the pointer to the pixels themselves.
And if you want to set the pointer to the pixel data itself, use SDL_CreateSurfaceFrom and set the pointer from the beginning. E.g. a stream from a webcam.