GNOME Bugzilla – Bug 749243
textoverlay: gltestsrc ! textoverlay ! fakesink does not work
Last modified: 2015-06-13 00:26:52 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, GST_VIDEO_OVERLAY_COMPOSITION_META_API_TYPE, NULL)) 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.
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
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.
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 blitter.
Seems like a reasonable approach as discussed on IRC.
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.
Attachment 303278 [details] pushed as 7e47aaf - basetextoverlay: Rewrite negotiation method
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).