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 796898 - pad: Update pad offsets on the current event if the offset changed in pad probes
pad: Update pad offsets on the current event if the offset changed in pad probes
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal normal
: 1.14.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-07-31 13:46 UTC by Sebastian Dröge (slomo)
Modified: 2018-08-02 07:59 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
pad: Update pad offsets on the current event if the offset changed in pad probes (3.92 KB, patch)
2018-07-31 13:47 UTC, Sebastian Dröge (slomo) 		none Details | Review
pad: Update pad offsets on the current event if the offset changed in pad probes (4.03 KB, patch)
2018-07-31 15:36 UTC, Sebastian Dröge (slomo) 		committed Details | Review

Description Sebastian Dröge (slomo) 2018-07-31 13:46:54 UTC 
Should be self-explanatory
Comment 1 Sebastian Dröge (slomo) 2018-07-31 13:47:00 UTC 
Created attachment 373226 [details] [review]
pad: Update pad offsets on the current event if the offset changed in pad probes
Comment 2 Sebastian Dröge (slomo) 2018-07-31 13:49:56 UTC 
This is kind of related to https://bugzilla.gnome.org/show_bug.cgi?id=765049 and the new unit test there can actually be merged then. That other bug is still relevant though, for sink pads the sticky event handling with regards to pad probes is a bit inconsistent.
Comment 3 Sebastian Dröge (slomo) 2018-07-31 13:52:16 UTC 
Actually not, because this still does not actually update *other* sticky events on the sinkpads when the offset is changing.
Comment 4 Jan Schmidt 2018-07-31 14:29:27 UTC 
Review of attachment 373226 [details] [review]:

Looks OK, except for one thing

::: gst/gstpad.c
@@ +5633,3 @@
   GstPadEventFullFunction eventfullfunc = NULL;
   GstObject *parent;
+  gint64 old_pad_offset = pad->offset;

pad->offset is protected by the OBJECT_LOCK, that's not taken until the next line?
Comment 5 Sebastian Dröge (slomo) 2018-07-31 15:36:31 UTC 
Created attachment 373228 [details] [review]
pad: Update pad offsets on the current event if the offset changed in pad probes
Comment 6 Sebastian Dröge (slomo) 2018-08-01 14:29:38 UTC 
Attachment 373228 [details] pushed as babd0e5 - pad: Update pad offsets on the current event if the offset changed in pad probes