After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 766661 - gltestsrc,glmosaic: fix shader leaks
gltestsrc,glmosaic: fix shader leaks
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.9.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-05-19 14:15 UTC by Guillaume Desmottes
Modified: 2016-05-24 13:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
glmosaic: fix shader leak (948 bytes, patch)
2016-05-19 14:16 UTC, Guillaume Desmottes
committed Details | Review
gltestsrc: free the src_impl when finalizing (1.31 KB, patch)
2016-05-19 14:16 UTC, Guillaume Desmottes
none Details | Review
gltestsrc: fix shaders ref counting (2.58 KB, patch)
2016-05-19 14:16 UTC, Guillaume Desmottes
committed Details | Review
gltestsrc: fix src_impl leak (716 bytes, patch)
2016-05-23 07:41 UTC, Guillaume Desmottes
committed Details | Review

Description Guillaume Desmottes 2016-05-19 14:15:12 UTC
.
Comment 1 Guillaume Desmottes 2016-05-19 14:16:04 UTC
Created attachment 328190 [details] [review]
glmosaic: fix shader leak

gst_gl_mosaic_init_shader() is called twice with test_glmosaic so the
first shader was leaked.
Comment 2 Guillaume Desmottes 2016-05-19 14:16:09 UTC
Created attachment 328191 [details] [review]
gltestsrc: free the src_impl when finalizing

This one was never freed when the element was destroyed.
Comment 3 Guillaume Desmottes 2016-05-19 14:16:15 UTC
Created attachment 328192 [details] [review]
gltestsrc: fix shaders ref counting

The gltestsrc element uses two shaders: color_shader and snow_shader.
Those are alternatively assigned to the SrcShader->shader pointer and
their reference was transferred to it. Only the SrcShader->shader was
unreffed (in _src_shader_deinit()) so only one shader was properly
freed, the other one was leaked.

Fixed this by giving an extra ref to SrcShader->shader and unreffing the
2 shaders in _src_smpte_free().
Comment 4 Matthew Waters (ystreet00) 2016-05-20 15:11:28 UTC
Review of attachment 328191 [details] [review]:

This should be done earlier, say in _stop() instead.
Comment 5 Matthew Waters (ystreet00) 2016-05-20 15:14:10 UTC
commit fe7dd131f5a441fc3f73790bc8447cb454362dc9
Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date:   Thu May 19 17:05:33 2016 +0300

    gltestsrc: fix shaders ref counting
    
    The gltestsrc element uses two shaders: color_shader and snow_shader.
    Those are alternatively assigned to the SrcShader->shader pointer and
    their reference was transferred to it. Only the SrcShader->shader was
    unreffed (in _src_shader_deinit()) so only one shader was properly
    freed, the other one was leaked.
    
    Fixed this by giving an extra ref to SrcShader->shader and unreffing the
    2 shaders in _src_smpte_free().
    
    https://bugzilla.gnome.org/show_bug.cgi?id=766661

commit 2572e3d25c1192faba7bc89d26d26192efdd0481
Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date:   Thu May 19 16:55:31 2016 +0300

    glmosaic: fix shader leak
    
    gst_gl_mosaic_init_shader() is called twice with test_glmosaic so the
    first shader was leaked.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=766661
Comment 6 Guillaume Desmottes 2016-05-23 07:41:56 UTC
Created attachment 328369 [details] [review]
gltestsrc: fix src_impl leak
Comment 7 Matthew Waters (ystreet00) 2016-05-24 13:36:10 UTC
commit dff46e3239e4337de37b25ad8970af2335ad74cf
Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date:   Mon May 23 10:40:58 2016 +0300

    gltestsrc: fix src_impl leak
    
    https://bugzilla.gnome.org/show_bug.cgi?id=766661