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 678433 - 0.11: x264enc broken after latest port
0.11: x264enc broken after latest port
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
0.11.x
Other All
: Normal normal
: 0.11.x
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-06-20 01:49 UTC by Matej Knopp
Modified: 2012-06-21 07:46 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Patch (1.45 KB, patch)
2012-06-20 01:49 UTC, Matej Knopp 		needs-work Details | Review
Fixes uninitialized variable and taglist (1.48 KB, patch)
2012-06-20 17:38 UTC, Matej Knopp 		committed Details | Review

Description Matej Knopp 2012-06-20 01:49:26 UTC 
Created attachment 216776 [details] [review]
Patch

Tag event handler attempts to modify taglist that is not modifiable. 
GstFlowReturn variable is returned uninitialized in some cases.

Btw. perhaps the demuxers could print a warning or error message if the GstFlowReturn value is too weird, right now they just quit silently which is not very nice.
Comment 1 Sebastian Dröge (slomo) 2012-06-20 06:43:52 UTC 
Review of attachment 216776 [details] [review]:

::: ext/x264/gstx264enc.c
@@ +1567,3 @@
+      tags = gst_tag_list_copy (tags);
+      s = gst_event_get_structure (event);
+      event = gst_event_new_tag (gst_structure_get_name (s), tags);

I don't think this is correct. The reference of the event passed to this function is owned by the function. Making writable should just work, and the way you're doing it now you're leaking the original event.
Comment 2 Matej Knopp 2012-06-20 06:50:15 UTC 
I must have overlooked the leak, the original code doesn't work though. gst_mini_object_make_writable doesn't make the tag-list inside the event writable. I can submit another patch that fixes the leak, I'm not sure how else do you want to fix this.
Comment 3 Matej Knopp 2012-06-20 06:54:45 UTC 
I must have overlooked the leak, the original code doesn't work though. gst_mini_object_make_writable doesn't make the tag-list inside the event writable. I can submit another patch that fixes the leak, I'm not sure how else do you want to fix this.
Comment 4 Sebastian Dröge (slomo) 2012-06-20 08:08:09 UTC 
Just fixing the leak is fine.
Comment 5 Matej Knopp 2012-06-20 17:38:00 UTC 
Created attachment 216856 [details] [review]
Fixes uninitialized variable and taglist

Also the leak from previous patch
Comment 6 Sebastian Dröge (slomo) 2012-06-21 07:45:38 UTC 
Comment on attachment 216856 [details] [review]
Fixes uninitialized variable and taglist

commit 711d2b18a7cccf8216cea133b5a0079296c680b4
Author: Matej Knopp <matej.knopp@gmail.com>
Date:   Wed Jun 20 13:36:25 2012 -0400

    x264enc: Fix unitialized variable and taglist event