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 746442 - rtprtxsend: Maybe broken stream lock handling
rtprtxsend: Maybe broken stream lock handling
Status: RESOLVED NOTABUG
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other All
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-03-19 11:21 UTC by Sebastian Dröge (slomo)
Modified: 2015-03-19 11:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rtprtxsend: Take the stream lock before doing anything with the srcpad (3.88 KB, patch)
2015-03-19 11:22 UTC, Sebastian Dröge (slomo)
rejected Details | Review

Description Sebastian Dröge (slomo) 2015-03-19 11:21:57 UTC
Currently rtprtxsend is sending data and events from the sinkpad streaming
thread, and from its own srcpad task. The srcpad task holds the stream lock of
the srcpad all the time, and pushing data/events will take the stream lock of
the peer pad.

I'm not sure if the attached patch is really necessary, maybe by taking the
peer pad stream lock everything is good already... but maybe not. Maybe what
can happen is that we store serialized events out of order on the srcpad
itself or something.
Comment 1 Sebastian Dröge (slomo) 2015-03-19 11:22:03 UTC
Created attachment 299799 [details] [review]
rtprtxsend: Take the stream lock before doing anything with the srcpad
Comment 2 Wim Taymans 2015-03-19 11:32:42 UTC
I don't think this is needed. The srcpad has it's own lock and pushes independently from the other threads, there is no need to serialize things beyond that, AFAICS.
Comment 3 Sebastian Dröge (slomo) 2015-03-19 11:34:12 UTC
But the srcpad stream lock will only be taken by the task, not by gst_pad_push/gst_pad_push_event. So only the task will ever take that, while the chain/event function will just push things without.

However gst_pad_push/push_event will take the stream lock of the peer, so maybe it's all good.
Comment 4 Wim Taymans 2015-03-19 11:36:56 UTC
(In reply to Sebastian Dröge (slomo) from comment #3)
> However gst_pad_push/push_event will take the stream lock of the peer, so
> maybe it's all good.

Yes, we serialize buffers and events on the sinkpads of elements.
Comment 5 Sebastian Dröge (slomo) 2015-03-19 11:41:19 UTC
So probably no problem here then.