GNOME Bugzilla – Bug 779058
caps: simplify creates invalid caps with sets or ranges, intersect fails even more on them
Last modified: 2018-11-03 12:39:35 UTC
intersect() of sets of ranges is broken: "video/x-h264, framerate=(fraction){ [ 5/1, 240/1 ], [ 3/1, 5/1 ] }" intersected with itself gives "video/x-h264, framerate=(fraction){ [ 5/1, 240/1 ], 5/1, { 5/1, [ 3/1, 5/1 ] } }" There should not be ranges inside sets to begin with, but the intersection result is even worse than that. It's of a different type than the input, and the set contains elements of different types. And these sets of ranges are caused by simplify(): "video/x-raw, framerate=(fraction) [3/1, 30/1]; video/x-raw, framerate=(fraction) [3/1, 120/1]" gives simplified "video/x-raw, framerate=(fraction){ [ 30/1, 60/1 ], [ 3/1, 30/1 ], [ 60/1, 120/1 ] }"
I don't quite see that here (on top of 586b34436e889c57f3108e6160f20beb55e61cd7): #include <gst/gst.h> #define CAPS "video/x-raw, framerate=(fraction) [3/1, 30/1]; video/x-raw, framerate=(fraction) [3/1, 120/1]" int main (int argc, char **argv) { GstCaps *caps; char *s; gst_init(&argc, &argv); caps = gst_caps_from_string(CAPS); s = gst_caps_to_string (caps); g_print("caps: %s\n", s); g_free (s); caps = gst_caps_simplify(caps); s = gst_caps_to_string (caps); g_print ("simplified: %s\n", s); g_free (s); gst_caps_unref (caps); return 0; } $ gcc -Wall -W simplify.c `pkg-config --cflags --libs gstreamer-1.0 glib-2.0` && ./a.out caps: video/x-raw, framerate=(fraction)[ 3/1, 30/1 ]; video/x-raw, framerate=(fraction)[ 3/1, 120/1 ] simplified: video/x-raw, framerate=(fraction){ [ 30/1, 120/1 ], [ 3/1, 30/1 ] } The non-simplified part above is due to: #if 0 /* Implement these if needed */ gst_value_register_union_func (GST_TYPE_FRACTION, GST_TYPE_FRACTION_RANGE, gst_value_union_fraction_fraction_range); gst_value_register_union_func (GST_TYPE_FRACTION_RANGE, GST_TYPE_FRACTION_RANGE, gst_value_union_fraction_range_fraction_range); #endif I still found a bug in the area though.
Created attachment 347834 [details] [review] fix union of int range and int when extending range This doesn't fix the bug, just something I found while looking for it.
Created attachment 347842 [details] [review] implement fraction range union
$ gcc -Wall -W simplify.c `pkg-config --cflags --libs gstreamer-1.0 glib-2.0` && GST_DEBUG=1 ./a.out caps: video/x-raw, framerate=(fraction)[ 3/1, 30/1 ]; video/x-raw, framerate=(fraction)[ 3/1, 120/1 ] simplified: video/x-raw, framerate=(fraction)[ 3/1, 120/1 ] Now, since I did not have your original full problem, this might not fix it all.
When intersecting the original caps, I get: $ gcc -Wall -W intersect.c `pkg-config --cflags --libs gstreamer-1.0 glib-2.0` && GST_DEBUG=1 ./a.out caps: video/x-h264, framerate=(fraction){ [ 5/1, 240/1 ], [ 3/1, 5/1 ] } intersected: video/x-h264, framerate=(fraction){ [ 5/1, 240/1 ], [ 3/1, 5/1 ] } (could be simplified, but _simplify is probably not called there)
Comment on attachment 347842 [details] [review] implement fraction range union Please add some tests for this while you're at it :)
(In reply to Vincent Penquerc'h from comment #1) > I don't quite see that here (on top of > 586b34436e889c57f3108e6160f20beb55e61cd7): Let's keep it open after merging your patches then, I'll double check and write test-case if it's still a problem after your patches. But good bugs you found there :)
Created attachment 347923 [details] [review] implement fraction range union
Comment on attachment 347923 [details] [review] implement fraction range union maybe some more tests than this single one, similar to what is there for other types already. Also if you have that now already, seems trivial to also add gst_value_union_fraction_fraction_range() or not? :)
One thing that's not clear to me is whether fractions set operations should operate like mathematical fractions. I'm guessing yes, but when used as framerate, it kinda makes a difference if the denominators are not the same, and I've previously treated them as being "numerator between bounds, but same denominator". The unit tests for fraction values treats them as mathematical. My union code doesn't (which is fine, as it's conservative, just not optimal).
Fractions are everywhere considered like mathematical fractions. 300/10 is the same as 30/1.
Hrm. Then this means that my "extending the range" is wrong in the patches above. ie, I consider [10/20 - 30/20] union [31/20 - 60/20] to be [10/20 - 60/20], which isn't right for mathematical fractions... I will fix.
Created attachment 348011 [details] [review] implement fraction range union
-- 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/221.