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 678384 - [API] [GstVideoOverlayComposition] port to 0.11
[API] [GstVideoOverlayComposition] port to 0.11
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.11.x
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 679145
Blocks: 678389
 
 
Reported: 2012-06-19 10:23 UTC by Mark Nauwelaerts
Modified: 2012-07-05 14:41 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Mark Nauwelaerts 2012-06-19 10:23:56 UTC
Subject says it all, and pretty much needed if more elements are to use it.
This will presumably involve GstMeta (also according to IRC):
----
<__tim>       mnauw, for GstVideoOverlayComposition I think the idea was to somehow make 
a meta for it
<mnauw>       yeah, that seems like a pretty "straightforward" map
<mnauw>       that is, stick something on a buffer -> put it in meta
<__tim>       mnauw, however, it kind of serves two purposes: (1) as a container for overlay info, to attach to buffers and let some other component do the blending, and (2) just helper/convenience API for elemen
ts that want to blend ARGB data onto raw pixels
<__tim>       so the easiest would imho be to just make a meta with one of those objects in it, but perhaps there's something more clever
<__tim>       there's also the issue of a more general-purpose video mixing kind of thing, but IMHO we should keep that generala thing separate from this mostly subtitle-targetted API
<mnauw>       makes sense
<__tim>       mnauw, there's some possibly related interesting stuff there that should ideally be queryable, like if the downstream component can handle overlays (textoverlay could query that and then decide whether to blend or just attach), whether it supports global alpha, whether it supports/needs premultiplied alpha or not; in future perhaps whether it can do text/pango markup rendering directly; an allocator for the pixels might be nice etc. :)
----

Note also that if overlay elements convert to using GstVideoOverlayComposition, they become pretty much separated into a "decoder" phase, followed by an overlay phase.  The latter phase is then pretty generic, both in the execution (using overlay composition helper code) and in the algorithm on what data to overlay onto what (based on timestamp comparison).

So one school of thought here might lead to introducing an overlay baseclass (see e.g. suggested in bug #614612), which is re-use through inheritance.

Alternatively, one might go for re-use through composition (sort-of) and introduce a separate caps type (or so) to carry basically empty buffers (or so) with attached overlay composition data.  This could then be handed to a generic overlay element that performs appropriate timestamp comparison and then either performs the actual overlaying or attaches the overlay composition data to a real video buffer (or sort of real in video/x-surface like case).  Perhaps supporting 'relative' coordinates or so (in 0 ... 1.0 scale) is also useful in this case.
Comment 1 Tim-Philipp Müller 2012-06-19 14:14:13 UTC
You are certainly right about the effective split into a decoding stage and a blending stage. And there have been suggestions that the 0.10 overlay composition API is not very 'GStreamer-y' for some reason, whatnot with attaching metadata to video buffers instead of sending it in separate streams.

However, I'm not sure that splitting the decoding stage and the overlay is the most desirable way here.

There is of course some appeal in just needing one generic overlay/mixer element, and just using decoders otherwise, but I don't know how well that'll work with extra features that we might want. If you have an overlay, the decoder/overlay stage is tightly coupled, and can interact much more naturally with the requirements of the downstream video renderer. In terms of negotiation, supported features, etc. If you have the decoder split out, you need to basically proxy that to the decoder, which feels unnatural. And a dummy format just for the meta seems a bit weird too. I feel that the extremely limited format-support (ARGB only) and the convenient and automatic conversions/scaling under the hood are a strength.

I think there's also a case to be made for some kind of delta-video format, so that e.g. dvdsubdec can output raw video without allocating a whole frame; or ximagesrc can only output regions that changed since the last capture. I think that needs to be more generic and flexible than an overlay format though, with more negotiation etc. and should be handled independently.

Anyway, a lot of things *can* be done, but personally I think we should for now continue with the approach we know works well and that doesn't require redesign of various important bits of infrastructure and elements for no apparent (to me anyway) gain. A generic overlay base class would be ace of course, but that also can still be added later, no?

It would be more interesting to make sure all of this integrates nicely with e.g. cluttersink and applications (so e.g. totem can render text/pango markup itself via cluttersink, and textoverlay can render things in the display resolution rather than the resolution of the video, etc.)
Comment 2 Mark Nauwelaerts 2012-06-28 16:23:58 UTC
Guess following commit(s) make this resolved.

Some of the above points may still be open, but might deserve a bug and/or title of their own ...

commit e94022806ffaff3075d1494a9e0f96ae6791cb47
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Thu Jun 28 18:16:20 2012 +0200

    videooverlaycomposition: port to 0.11
    
    ... which also entails porting video-blend
    
    Fixes #678384.
Comment 3 Mark Nauwelaerts 2012-07-02 12:37:05 UTC
After some more tweaking, looks like following is needed for now, so re-opening to track dependency:

commit 1d413ace640c679ba7fbecec07f2bea3d98360b2
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Mon Jul 2 14:26:50 2012 +0200

    videooverlaycomposition: ensure proper buffer copy
    
    This is only temporary and could and should be modified to use
    regular buffer copy once https://bugzilla.gnome.org/show_bug.cgi?id=679145
    is resolved.
Comment 4 Mark Nauwelaerts 2012-07-05 14:41:20 UTC
And resolved again ...

commit abea4324091f1ec50d9212f00a39ae8c85126364
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Thu Jul 5 16:29:42 2012 +0200

    Revert "videooverlaycomposition: ensure proper buffer copy"
    
    This reverts commit 1d413ace640c679ba7fbecec07f2bea3d98360b2.
    
    Plain gst_buffer_copy() is now doing the expected ...
    
    See https://bugzilla.gnome.org/show_bug.cgi?id=678384.