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 746011 - gstpad : refactor gst_pad_link_prepare function
gstpad : refactor gst_pad_link_prepare function
Status: RESOLVED WONTFIX
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Mac OS
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-03-11 05:37 UTC by Changbok
Modified: 2015-06-02 15:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch diff file (2.03 KB, patch)
2015-03-11 05:37 UTC, Changbok
none Details | Review
gstpad: refactor gst_pad_link_prepare function (2.02 KB, patch)
2015-03-11 09:58 UTC, Changbok
needs-work Details | Review

Description Changbok 2015-03-11 05:37:16 UTC
Created attachment 299071 [details] [review]
patch diff file

In gst_pad_link_prepare function, GST_OBJECT_UNLOCK is not called about OK case.
In other function that uses gst_pad_link_prepare(), should be called GST_OBJECT_UNLOCK about pad if gst_pad_link_prepare() will return OK.

So I think other function's logic can become complex.

(This patch is one of gstpad FIXME list.)
Comment 1 Changbok 2015-03-11 05:37:47 UTC
Please review this patch.
Comment 2 Changbok 2015-03-11 09:58:00 UTC
Created attachment 299082 [details] [review]
gstpad: refactor gst_pad_link_prepare function

I re-attach patch file
Comment 3 Changbok 2015-03-18 00:24:40 UTC
Add reviewer.

Please review this patch
Comment 4 Olivier Crête 2015-03-18 00:39:00 UTC
Review of attachment 299082 [details] [review]:

Unlocking at the end of

::: gst/gstpad.c
@@ +2350,3 @@
 
+  GST_OBJECT_LOCK (srcpad);
+  GST_OBJECT_LOCK (sinkpad);

You can't unlock in gst_pad_link_prepare() and re-lock here, stuff may have changed in between. How to make this more simple is not obvious to me, and I assume was not obvious to the person who wrote this.
Comment 5 Changbok 2015-03-18 01:47:49 UTC
Thank you for your opinion.

gst_pad_link_prepare() return OK means we can use srcpad,sinkpad(to link them). So maybe GST_OBJECT_UNLOCK is not called at gst_pad_link_prepare() in OK case.

But if we use gst_pad_link_prepare() just for checking(for example, gst_pad_can_link() API), we should unnecessarily add code to unlock pad for OK case. Also unnecessarily need comments for description.
Comment 6 Tim-Philipp Müller 2015-05-22 15:10:32 UTC
What's up with this? Can it be closed?
Comment 7 Changbok 2015-06-02 15:42:37 UTC
@Olivier, @Tim-Philipp,
I was brought the wrong problem from my wrong understanding about this function.
Sorry for the confusion.
So I will close this issue.