Created attachment 313170 [details] [review]
gl: fix leak in gst_gl_insert_debug_marker()

The string allocated by g_vasprintf() was leaked.

Reproduced using the
validate.file.compositor.simple.play_15s.synchronized validate scenario.

The spec does not say if the string is copied internally or needs to be valid forever... what should we do? :)

The header in mesa is defined as "void APIENTRY glInsertEventMarkerEXT (GLsizei length, const GLchar *marker);". The "const" is making clear that the string isn't consummed but yeah, I didn't check the opengl spec.

The const might also mean that it assumes the string pointer to be valid forever.

I don't see any cases where the passed data would need to be live forever.  This functionality would be entirely CPU side so does not need any synchronisation on the GPU.  If there's a debug callback (which gstgl adds), the message is passed directly to the callback otherwise it will be copied when added to the debug message queue (retrieved by glGetDebugMessageLog).

Typically the convention is that most data that is passed to GL can be freed when the GL function finishes so this patch is correct.  The only cases I know of where this is false are related to buffer transfers.

commit 5b30a8e9b23578ea5c7a327d6bb8307f9b7de63f
Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date:   Tue Oct 13 12:40:04 2015 +0200

    gl: fix leak in gst_gl_insert_debug_marker()
    
    The string allocated by g_vasprintf() was leaked.
    
    Reproduced using the
    validate.file.compositor.simple.play_15s.synchronized validate scenario.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=756492

Isn't that something we'd to fix in the stable branch as well?

gst_gl_insert_debug_marker doesn't exist in stable.