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 763973 - qtdemux: Fix qtdemux memory leak
qtdemux: Fix qtdemux memory leak
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.8.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-03-21 06:05 UTC by Jimmy Ohn
Modified: 2016-03-25 10:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
qtdemux: Fix qtdemx memory leak (1.55 KB, patch)
2016-03-21 06:06 UTC, Jimmy Ohn
none Details | Review
qtdemux: Fix memory leak (1.92 KB, patch)
2016-03-24 06:31 UTC, Jimmy Ohn
none Details | Review
qtdemux: Fix memory leak (2.80 KB, patch)
2016-03-24 06:37 UTC, Jimmy Ohn
committed Details | Review

Description Jimmy Ohn 2016-03-21 06:05:43 UTC
qtdemux isn't being freed if we can't find right index in src_convert function.
So, we should free it when it can't find right index.
Comment 1 Jimmy Ohn 2016-03-21 06:06:16 UTC
Created attachment 324410 [details] [review]
qtdemux: Fix qtdemx memory leak
Comment 2 Sebastian Dröge (slomo) 2016-03-21 08:45:16 UTC
Review of attachment 324410 [details] [review]:

Instead pass qtdemux (aka parent) as a parameter into gst_qtdemux_src_convert() from gst_qtdemux_handle_src_query().
Comment 3 Jimmy Ohn 2016-03-22 08:00:35 UTC
@slomo
I'm sorry about that I don't understand for your comment. As you know, qtdemux is freed in goto statement in src_convert function. could you please explain more detail?
Comment 4 Sebastian Dröge (slomo) 2016-03-22 08:45:46 UTC
This calls _convert(): https://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/gst/isomp4/qtdemux.c?id=d8fb7a9c96b108814beeaa0e63f818d4648c7fe9#n804

It has qtdemux/parent there already. You could just pass that as an argument to _convert().
Comment 5 Jimmy Ohn 2016-03-24 06:31:30 UTC
(In reply to Sebastian Dröge (slomo) from comment #4)
> This calls _convert():
> https://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/gst/isomp4/
> qtdemux.c?id=d8fb7a9c96b108814beeaa0e63f818d4648c7fe9#n804
> 
> It has qtdemux/parent there already. You could just pass that as an argument
> to _convert().

Thanks for your reply. I'll upload it!
Comment 6 Jimmy Ohn 2016-03-24 06:31:54 UTC
Created attachment 324649 [details] [review]
qtdemux: Fix memory leak
Comment 7 Jimmy Ohn 2016-03-24 06:37:32 UTC
Created attachment 324650 [details] [review]
qtdemux: Fix memory leak
Comment 8 Sebastian Dröge (slomo) 2016-03-24 12:38:58 UTC
commit 206e24855a8cc6e8da946da6239f8d4a83b28718
Author: Jimmy Ohn <yongjin.ohn@lge.com>
Date:   Thu Mar 24 15:14:23 2016 +0900

    qtdemux: Fix qtdemux memory leak in src_convert function
    
    If we don't find the index of the sample correctly in src_convert function,
    we have to unref about the qtdemux before returning value.
    So, I have modify it about instead pass qtdemux as a parameter into
    src_convert function.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=763973