Created attachment 29672 [details] [review]
patch

Hehe, I bet subtractions don't work either.
Nobody bothered to write intersection or subtraction functions for "fixed lists".
So it takes the standard approach of comparing and deciding based on that.

I'm also wondering if gst_caps_is_fixed works correctly for arrays.

After looking at our is_fixed API, it looks like the API is not up to what is
needed in this case, because gst_type_is_fixed (GST_TYPE_FIXED_LIST) cannot be
said to be TRUE or FALSE before looking at the value, since an array is fixed,
when all its entries are fixed and not fixed otherwise.

I'm changing the topic to reflect these other problems.

Created attachment 29686 [details] [review]
Patch to test substraction of fixed lists (fails)

Intersection works now. I didn't implement subtraction (minus s, Benjamin ;) ).
Any mathematical mind, feel free to implement that.

Also, I implemented the gst_value_is_fixed() function and deprecated
gst_type_is_fixed(). With that, Benjamin's subtraction should be easy to
implement. Changing topic to reflect new situation.

Marking patches as they don't address the actual issue.

Removing [api] from the summary, as no api is actually required.

GstFixedList -> GstArray

Is this still valid?

Ok, so this is probably fixed now as the test doesn't fail anymore:

        * tests/check/gst/gstcaps.c: (test_intersect2), (gst_caps_suite):
          Add David's and Benjamin's tests for array subtraction to the
          unit test suite, which suggests that #147931 is fixed these days.


Please reopen otherwise.

Actually, the way I read it is that the intersection in the tests works now (it does, I checked), but according to comments 5 through 7 these tests don't cover subtraction, so I guess someone needs to come up with some test cases for subtraction, whatever that means in this case (do we actually need that for anything?)

Re-opening, since I don't think it's actually fixed. (We may want to WONTFIX it though if no one can think of a use case for this).

Ok, I guess I found a case where it still doesn't work correctly...

  c2 = gst_caps_from_string ("audio/x-raw-int,channels=1;audio/x-raw-int,channels=2");
  c1 = gst_caps_from_string ("audio/x-raw-int,channels=[1,2]");
  gst_caps_merge (c2, c1);
  GST_DEBUG ("merged: (%d) %" GST_PTR_FORMAT, gst_caps_get_size (c2), c2);
  fail_unless (gst_caps_get_size (c2) == 2, NULL);
  gst_caps_unref (c2);



This can be fixed by using simplified caps for checking if a structure is a subset of caps in gst_caps_merge_structure() but this will make negotiation even slower than it already is.

(In reply to comment #14)
> Ok, I guess I found a case where it still doesn't work correctly...
> 
>   c2 = gst_caps_from_string
> ("audio/x-raw-int,channels=1;audio/x-raw-int,channels=2");
>   c1 = gst_caps_from_string ("audio/x-raw-int,channels=[1,2]");
>   gst_caps_merge (c2, c1);

  This results in "audio/x-raw-int,channels=[1,2]" ... which sounds correct to me.

  Why is this considered wrong ?

>   GST_DEBUG ("merged: (%d) %" GST_PTR_FORMAT, gst_caps_get_size (c2), c2);
>   fail_unless (gst_caps_get_size (c2) == 2, NULL);
>   gst_caps_unref (c2);
> 
> 
> 
> This can be fixed by using simplified caps for checking if a structure is a
> subset of caps in gst_caps_merge_structure() but this will make negotiation
> even slower than it already is.

Well, merge("channels=1;channels=2", "channels=[1,2]") should return "channels=1;channels=[1,2]" or "channels=1;channels=2" (which are equivalent). Merge should keep the preferences and order, which in this case means: Prefer 1 channel, otherwise I can also do 2 channels.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/1.