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 745506 - mpegtsmux: duplicate code
mpegtsmux: duplicate code
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other All
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-03-03 09:36 UTC by Ilya Averyanov
Modified: 2015-03-04 11:00 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Patch (826 bytes, patch)
2015-03-03 09:36 UTC, Ilya Averyanov 		committed Details | Review
Patch (1.04 KB, patch)
2015-03-03 09:37 UTC, Ilya Averyanov 		rejected Details | Review

Description Ilya Averyanov 2015-03-03 09:36:09 UTC 
Created attachment 298392 [details] [review]
Patch

http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/gst/mpegtsmux/mpegtsmux.c#n315
call
mux->tsmux = tsmux_new ();
tsmux_set_write_func (mux->tsmux, new_packet_cb, mux);

in http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/gst/mpegtsmux/mpegtsmux.c#n330
call
mpegtsmux_reset
which also call
 mux->tsmux = tsmux_new ();
 tsmux_set_write_func (mux->tsmux, new_packet_cb, mux);

mpegtsmux_reset always free mux->tsmux.
but we can free only in mpegtsmux_dispose.
Comment 1 Ilya Averyanov 2015-03-03 09:37:04 UTC 
Created attachment 298393 [details] [review]
Patch
Comment 2 Ilya Averyanov 2015-03-04 10:01:22 UTC 
Sorry attachment 298393 [details] [review] patch not correct.
Because mpegtsmux_reset also call from mpegtsmux_change_state.
But first attachment 298392 [details] [review] still actual
Comment 3 Sebastian Dröge (slomo) 2015-03-04 10:09:39 UTC 
Comment on attachment 298393 [details] [review]
Patch

This does not look correct. In GST_STATE_CHANGE_PAUSED_TO_READY we would allocate a new instance without freeing the old one.

The current code should always free the instance already.
Comment 4 Sebastian Dröge (slomo) 2015-03-04 10:10:14 UTC 
Thanks for the patch :) Please reopen if you think something still needs to be done.

commit 2eac4232732912cff7ae6427b72c6b360c330daa
Author: Ilya Averyanov <i.averyanov@geoscan.aero>
Date:   Mon Mar 2 01:08:15 2015 +0300

    mpegtsmux: Remove duplicate code
    
    The muxer is already allocated in reset(), which is called soon afterwards.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=745506