GNOME Bugzilla – Bug 322587
[API 0.11] change gst_value_union() return value to void
Last modified: 2012-01-24 14:21:24 UTC
gst_value_union() always succeeds and always returns TRUE, so we might just as well change the return value to void.
Shouldn't it fail and return FALSE if the values are of incompatible type, such that we can't represent a union of them? (i.e. isn't this an implementation bug, not an API problem?)
Right now if the values are of incompatible types it puts them in a list. So yes it does always succeed. Question would be, does putting different types in a list make sense. Can't change this now though -- probably best off putting a maybe-fixme in the source linking to this bug, because 0.11 is a ways off.
Ok, so I think we should keep the return value and enforce that lists + arrays always contain elements of the same type for 0.11.
Created attachment 200701 [details] [review] gstvalue: enforce identical basic types in lists, arrays
Created attachment 200702 [details] [review] tests: check return value of append routines as they now can fail.
I'd say put it in and see if there's any fallout :) However, on second thought I wonder if we shouldn't just declare it a programming error to put values of different types into a list/array ? (ie. just add guards for it that print nasty warnings, instead of adding return values no one in the world will ever check anyway)
Unless someone can think of a use case where that's useful. (bindings maybe?)
Yes, make it a programming error and document that this shouldn't be done.
Now, there's one extra reason why a union call might fail: if one tries to union [0..3] with [6..9], say. A theoretical solution would be to expand that into {0, 1, 2, 3, 6, 7, 8, 9}, but that does not scale very well at all. Not sure what to do here. Erroring out above a certain "resulting list size" threshold seems wrong. Alternatively, keep the return codes for such cases, but fail hard on type mismatches instead of returning errors. Not sure I like that either.
What's wrong with the 'return a list of the two values' approach then? We are still talking about values of the same type after all. Might be trickier if you wanted to union [0, 3] and { 15000, 19000 } with the new requirements.
That's exactly that. I'm thinking of ranges such as [16..4096] which are large, and common in video width/height. Unlikely to be unioned with a non intersecting range, though. Audio might have separate caps for 1, 2, and more channels, so you might end up unioning 1 and [3..255]. For audio sampling rate, the values would go even higher. What if you try to union caps for wideband audio and narrowband audio ? You'd end up with a LOT of values.
I don't think an union of [3,255] and 1 should fail, instead it should just do nothing and keep the two separate structures
What do you mean ? Doing nothing means not initializing the returned GValue to anything. It is akin to failing.
Just so I understand correctly: The Problem is that we've changed our list to only accept child values of the same type, right? That means that the _union() function can't union an int range and an int list any longer by just putting those two into a list, because then you'd have different child types (int range + list of ints), right?
(In reply to comment #13) > What do you mean ? > Doing nothing means not initializing the returned GValue to anything. > It is akin to failing. Nevermind, I thought we're talking about gst_caps_union(). There's a problem for gst_value_union() here now and it should fail. OTOH nothing will have ever worked with the result of such a union before so it shouldn't be a problem to fail now.
(In reply to comment #14) > Just so I understand correctly: > > The Problem is that we've changed our list to only accept child values of the > same type, right? That means that the _union() function can't union an int > range and an int list any longer by just putting those two into a list, because > then you'd have different child types (int range + list of ints), right? I'm not 100% sure, but I don't think you could have a list of lists of ranges and such other complex setups, could you ? Well, you *could*, but I don't think it would work with intersection, union, etc. If it does, then yes, I can just return a list of ranges or whatever.
Created attachment 201223 [details] [review] gstvalue: enforce identical basic types in lists, arrays Need to decide what to do for disjoint union/union, otherwise the rest is ready.
Talking to slomo on IRC, we want something like the union of ranges 1-10 and 20-30 to fail, instead of returning a list of ranges. Such cases are returned as FALSE return value. Mismatch in basic types is not, but asserted instead, as it is deemed a programming error. New patch coming up.
Created attachment 202367 [details] [review] gstvalue: enforce identical basic types in lists, arrays
commit b6abb9794c8100bf206b62ff863097343ba854c7 Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Fri Nov 4 18:26:15 2011 +0000 gstvalue: enforce identical basic types in lists, arrays https://bugzilla.gnome.org/show_bug.cgi?id=322587