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 749243 - textoverlay: gltestsrc ! textoverlay ! fakesink does not work
textoverlay: gltestsrc ! textoverlay ! fakesink does not work
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal enhancement
: 1.5.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2015-05-11 19:55 UTC by Nicolas Dufresne (ndufresne)
Modified: 2015-06-13 00:26 UTC
See Also:
GNOME target: ---
GNOME version: ---

pango: Attach meta if in the CAPS or the ALLOCATION (1.18 KB, patch)
2015-05-11 19:59 UTC, Nicolas Dufresne (ndufresne)
needs-work Details | Review
basetextoverlay: Rewrite negotiation method (6.08 KB, patch)
2015-05-12 18:20 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Nicolas Dufresne (ndufresne) 2015-05-11 19:55:55 UTC
With pipeline:

  gltestsrc ! textoverlay text=bla ! fakesink

It fails negotiation. It fails because of that condition:

  if (caps_has_meta && gst_query_find_allocation_meta (query,
      attach = TRUE;

Basically, that checks that composition meta is both in the caps and the allocation query, otherwise fail. I do think that if the caps OR the query says it is supported, we should understand that it is supported.

Also, I'm not sure if the CompositionMeta existed before the caps feature was added, but if it did, then this behaviour could be considered as a regression. At least that would explain such a redundancy.
Comment 1 Nicolas Dufresne (ndufresne) 2015-05-11 19:59:28 UTC
Created attachment 303234 [details] [review]
pango: Attach meta if in the CAPS or the ALLOCATION

We only attach the composition meta if both the caps and the ALLOCATION
query says it's supported. Instead, attach it if one of both says it is
supported. This makes the implementation a little more flexible, and
fixes pipeline like:

  gltestsrc ! textoverlay text=bla ! fakesink
Comment 2 Nicolas Dufresne (ndufresne) 2015-05-11 22:57:52 UTC
I must admit I didn't notice the huge hack later to try deal with fakesink in a special way. I asked around to understand this hack, and here's my conclusion.

When caps feature where introduced, caps ANY has been defined to also mean CapsFeature ANY. This was in fact an ABI break as a pipeline like:

  textoverlay ! fakesink
  textoverlay ! appsink

Would attach the composition instead of doing a blit (there is a chance that an old application would loose the text). It seems it was decided to fix the ABI break semantically, which resulted in a big hack to try and guess if downstream is appsink or fakesink. My patch breaks this hack and will upload again. I'll mark the patch as need-work for this reason.

Currently that hack is wrong, as it may fail the negotiation instead of uploading. With the split GL elements we can now have:

  gltestsrc ! textoverlay

In which case the textoverlay cannot downgrade to bliting, since in bliting mode it does not support memory:* (in this case it's GLMemory). Failing is wrong, because the downstream caps explictly says that it does not matter what meta are being sent.
Comment 3 Nicolas Dufresne (ndufresne) 2015-05-12 18:20:54 UTC
Created attachment 303278 [details] [review]
basetextoverlay: Rewrite negotiation method

This cleanup the negotiation function by properly splitting the probe
and the decisions. This allow handling correctly pipeline where upstream
caps have special memory type. An example pipeline is:

  gltestsrc ! textoverlay text=bla ! fakesink

The upstream caps will be memory:GLMemory, which isn't supported by the
Comment 4 Sebastian Dröge (slomo) 2015-05-13 10:54:53 UTC
Seems like a reasonable approach as discussed on IRC.
Comment 5 Nicolas Dufresne (ndufresne) 2015-05-16 22:25:15 UTC
Ok, the only remaining side effect, is that now there is one case where an allocation query will happen before the caps event is being sent (something that was avoided by sending caps event twice if needed). I think I need to bring that back because the feature exposed through the allocation query could vary depending on the chosen caps.
Comment 6 Nicolas Dufresne (ndufresne) 2015-06-13 00:24:47 UTC
Attachment 303278 [details] pushed as 7e47aaf - basetextoverlay: Rewrite negotiation method
Comment 7 Nicolas Dufresne (ndufresne) 2015-06-13 00:26:52 UTC
Going over the code again, I really think it's fine to query for meta's before set_format(). Specially that we are not requesting any pool here. Please ping me if I really missed the point, I'm sure we can find a way to send caps before allocation query, but it just sound silly).