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 796558 - event: Unset SNAP flags when creating a new seek event without KEY_UNIT flag
event: Unset SNAP flags when creating a new seek event without KEY_UNIT flag
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal normal
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-06-11 07:23 UTC by Sebastian Dröge (slomo)
Modified: 2018-06-18 07:32 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
event: Unset SNAP flags when creating a new seek event without KEY_UNIT flag (1.60 KB, patch)
2018-06-11 07:23 UTC, Sebastian Dröge (slomo) 		none Details | Review
event: Unset SNAP flags when creating a new seek event without KEY_UNIT flag (1.54 KB, patch)
2018-06-14 13:11 UTC, Sebastian Dröge (slomo) 		committed Details | Review

Description Sebastian Dröge (slomo) 2018-06-11 07:23:00 UTC 
See commit message for rationale
Comment 1 Sebastian Dröge (slomo) 2018-06-11 07:23:07 UTC 
Created attachment 372629 [details] [review]
event: Unset SNAP flags when creating a new seek event without KEY_UNIT flag

The SNAP flags only make sense in combination with the KEY_UNIT flag,
and without they expose all kinds of unexpected behaviour in various
elements that don't expect this from happening.

Also warn if this ever happens.
Comment 2 Tim-Philipp Müller 2018-06-11 08:05:49 UTC 
Comment on attachment 372629 [details] [review]
event: Unset SNAP flags when creating a new seek event without KEY_UNIT flag

>+  /* SNAP flags only make sense in combination with the KEYUNIT flag. Warn
>+   * and unset the SNAP flags if they're set without the KEYUNIT flag */
>+  if (!(flags & GST_SEEK_FLAG_KEY_UNIT) &&
>+      (flags & (GST_SEEK_FLAG_SNAP_BEFORE | GST_SEEK_FLAG_SNAP_AFTER |
>+              GST_SEEK_FLAG_SNAP_NEAREST))) {
>+    g_warn_if_fail ((flags & GST_SEEK_FLAG_KEY_UNIT)
>+        || !(flags & (GST_SEEK_FLAG_SNAP_BEFORE | GST_SEEK_FLAG_SNAP_AFTER |
>+                GST_SEEK_FLAG_SNAP_NEAREST)));

Should we maybe just do a g_warning() with some text here? :)
Comment 3 Sebastian Dröge (slomo) 2018-06-11 09:42:19 UTC 
(In reply to Tim-Philipp Müller from comment #2)

> Should we maybe just do a g_warning() with some text here? :)

I thought about that but uninventive with the text. Any suggestion? :)
Comment 4 Tim-Philipp Müller 2018-06-11 10:10:29 UTC 
"SNAP flags only make sense for KEY_UNIT seeks." (maybe + "ignoring SNAP flag").

Question is also if g_warn_if_fail() is compiled out in case of -DG_DISABLE_CHECKS or somesuch whilst g_warning may not be compiled out then.
Comment 5 Sebastian Dröge (slomo) 2018-06-12 15:57:39 UTC 
They are not compiled out, both of them. So you would prefer that over the current one?
Comment 6 Sebastian Dröge (slomo) 2018-06-14 13:11:54 UTC 
Created attachment 372685 [details] [review]
event: Unset SNAP flags when creating a new seek event without KEY_UNIT flag

The SNAP flags only make sense in combination with the KEY_UNIT flag,
and without they expose all kinds of unexpected behaviour in various
elements that don't expect this from happening.

Also warn if this ever happens.
Comment 7 Sebastian Dröge (slomo) 2018-06-14 13:12:00 UTC 
Let's go with this then?
Comment 8 Tim-Philipp Müller 2018-06-14 13:21:53 UTC 
Works for me, but whatever you prefer really :)
Comment 9 Sebastian Dröge (slomo) 2018-06-18 07:32:06 UTC 
Attachment 372685 [details] pushed as 8f496b7 - event: Unset SNAP flags when creating a new seek event without KEY_UNIT flag