GNOME Bugzilla – Bug 788979
"format" parameter in gstsegment.c as unused
Last modified: 2017-12-26 10:31:43 UTC
Most of the methods in gstsegment.c take these parameters: GstSegment * segment; /* a #GstSegment structure. */ GstFormat format; /* the format of the segment. */ What puzzles me is that most (but not all methods) do g_return_val_if_fail (segment->format == format, FALSE); and thats the only use of the format parameter (except passing it to other methods in the same module). What about: 1.) marking all those 'format' args in the docs as 'unused' and G_GNUC_UNUSED on the declaration. 2.) removing all the g_return_val_if_fail() checks 3.) adding a 'FIXME: 2.0' to drop the parameter I'll post a patch later.
Are you completely sure it's never used ? Mixers often store information there.
Nicolas, I mean the parameter to the function calls is not used, except for the comparison. I don't intend to remove the field from the GstSegment structure. Maybe you can link to the usage you have in mind, so that I can comment on the concrete example.
In the past having this parameter allowed to find some bugs where the actual segment format was different to what the element expected.
Thanks for the explanation. That makes sense, although it is a little weird though.
Yes it is :) I'm fine with removing the parameter for 2.0, but for now let's just keep it.
Feel free to add a /* FIXME 2.0: */ comment, closing :)
The segment API is done quite nicely in the Rust bindings now btw, there's e.g. Segment<Time>. The format parameter is hidden away and the compiler prevent mixing of units.
commit c96901d4d997c486e2f41eae06670bb00464aac6 (HEAD -> master, origin/master, origin/HEAD) Author: Stefan Sauer <ensonic@users.sf.net> Date: Tue Dec 26 11:29:39 2017 +0100 segment: add a FIXME-2.0 for the format parameters Capture the somewhat not ordinary use of the extra format parameter in a comment. See https://bugzilla.gnome.org/show_bug.cgi?id=788979