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 749445 - [patchset] Fix capabilities verification and unit tests as result
[patchset] Fix capabilities verification and unit tests as result
Status: RESOLVED NOTABUG
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-05-15 18:01 UTC by Volodymyr Riazantsev
Modified: 2015-05-15 22:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch-set and unit tests results (3.45 KB, application/gzip)
2015-05-15 18:01 UTC, Volodymyr Riazantsev
  Details
0001-Fix-condition-verification-for-subset-check.patch (994 bytes, patch)
2015-05-15 18:03 UTC, Volodymyr Riazantsev
rejected Details | Review
0002-structure-Improve-fix-is_subset-verification.patch (1.82 KB, patch)
2015-05-15 18:05 UTC, Volodymyr Riazantsev
rejected Details | Review
0003-tests-caps-Add-check-for-basic-gst_caps_is_subset-ca.patch (1.18 KB, patch)
2015-05-15 18:13 UTC, Volodymyr Riazantsev
rejected Details | Review
0004-tests-caps-Fix-test_subset-case.patch (1.09 KB, patch)
2015-05-15 18:13 UTC, Volodymyr Riazantsev
rejected Details | Review
0005-tests-caps-Fix-test_merge_subset-case.patch (1.28 KB, patch)
2015-05-15 18:14 UTC, Volodymyr Riazantsev
rejected Details | Review
0006-tests-caps-Fix-test_merge_subset-case.patch (1.20 KB, patch)
2015-05-15 18:14 UTC, Volodymyr Riazantsev
rejected Details | Review
0007-tests-elementfactory-Fix-test_can_sink_all_caps-case.patch (1014 bytes, patch)
2015-05-15 18:14 UTC, Volodymyr Riazantsev
rejected Details | Review
0008-caps-Simple-improvement-in-search-precedure.patch (1.28 KB, patch)
2015-05-15 18:15 UTC, Volodymyr Riazantsev
rejected Details | Review
Unit tests results (2.42 KB, text/plain)
2015-05-15 18:16 UTC, Volodymyr Riazantsev
  Details

Description Volodymyr Riazantsev 2015-05-15 18:01:39 UTC
Created attachment 303437 [details]
Patch-set and unit tests results

Hello,
  I'm a new in GStreamer, so sorry if my patch-set is because of my wrong understanding of ideas.
  During integration GS into some custom embedded system I observe some pipelines are not working for me. After some investigation I realized 'caps' comparison is not always success. I observe 'is_subset()' procedure is not working well (if I understood main idea correctly). I fixed it and this led to some unit tests ware failed. I fixed them as well since (i think) they were wrong as well.
  Please check the patches and let me know your comments.  

Sincerely,
  Volodymyr.
Comment 1 Volodymyr Riazantsev 2015-05-15 18:03:52 UTC
Created attachment 303438 [details] [review]
0001-Fix-condition-verification-for-subset-check.patch
Comment 2 Volodymyr Riazantsev 2015-05-15 18:05:19 UTC
Created attachment 303439 [details] [review]
0002-structure-Improve-fix-is_subset-verification.patch
Comment 3 Volodymyr Riazantsev 2015-05-15 18:13:14 UTC
Created attachment 303441 [details] [review]
0003-tests-caps-Add-check-for-basic-gst_caps_is_subset-ca.patch
Comment 4 Volodymyr Riazantsev 2015-05-15 18:13:43 UTC
Created attachment 303442 [details] [review]
0004-tests-caps-Fix-test_subset-case.patch
Comment 5 Volodymyr Riazantsev 2015-05-15 18:14:05 UTC
Created attachment 303443 [details] [review]
0005-tests-caps-Fix-test_merge_subset-case.patch
Comment 6 Volodymyr Riazantsev 2015-05-15 18:14:29 UTC
Created attachment 303444 [details] [review]
0006-tests-caps-Fix-test_merge_subset-case.patch
Comment 7 Volodymyr Riazantsev 2015-05-15 18:14:56 UTC
Created attachment 303445 [details] [review]
0007-tests-elementfactory-Fix-test_can_sink_all_caps-case.patch
Comment 8 Volodymyr Riazantsev 2015-05-15 18:15:23 UTC
Created attachment 303446 [details] [review]
0008-caps-Simple-improvement-in-search-precedure.patch
Comment 9 Volodymyr Riazantsev 2015-05-15 18:16:03 UTC
Created attachment 303447 [details]
Unit tests results
Comment 10 Sebastian Dröge (slomo) 2015-05-15 22:02:59 UTC
Review of attachment 303438 [details] [review]:

::: gst/gststructure.c
@@ +3339,3 @@
 {
   if ((superset->name != subset->name) ||
+      (gst_structure_n_fields (superset) < gst_structure_n_fields (subset)))

This change is not correct. The superset has to contain less or equal fields than the subset.

This is because if a field is missing, it is assumed that any possible value for that field would be possible. So for every field that is in the (potential) subset and not in the superset, the subset check would succeed.
Comment 11 Sebastian Dröge (slomo) 2015-05-15 22:04:52 UTC
Review of attachment 303441 [details] [review]:

::: tests/check/gst/gstcaps.c
@@ +295,3 @@
+  c2 = gst_caps_from_string ("video/x-raw, format=(string)YUY2");
+  fail_unless (gst_caps_is_subset (c2, c1));
+  fail_if (gst_caps_is_subset (c1, c2));

And this here is exactly such an example:

c1 is a subset of c2, but c2 is not a subset of c1. c2 has no alignment field, which means that the alignment field can have any possible value and thus covers *more* possible caps than c1, which only allows a single value for the alignment field.
Comment 12 Sebastian Dröge (slomo) 2015-05-15 22:06:04 UTC
Thanks for your patches and the analysis, but I think this is a misunderstanding of how caps work.

If you can give some further details about how this causes problems for you, we should be able to give you some ideas how to solve your use cases :)
Comment 13 Sebastian Dröge (slomo) 2015-05-15 22:08:26 UTC
See also http://cgit.freedesktop.org/gstreamer/gstreamer/tree/docs/design/part-caps.txt for details about how the caps operations are defined