[Bug 668483] video: add support for global-alpha multiplicator for overlay rectangles

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=668483
  GStreamer | gst-plugins-base | git

--- Comment #14 from Holger Kaelberer <hk at getslash.de> 2012-03-16 08:40:41 UTC ---
> 
> However, from a consumer/overlay perspective, what one actually wants to know
> if whether the underlying pixel data changed, right? For a consumer that
> doesn't support global alpha, it doesn't make a difference. But for a consumer
> who does support global alpha - they want to know that the pixels are the same
> as last time, and only the global alpha changed, so they can use the old object
> and transform it accordingly. Or at least that's how I imagine it. Is this what
> APIs would support? Or does it not work like that anyway? 

Yes, I think it does, good point! A renderer supporting global-alpha itself
should not need to re-upload unchanged pixel-data, This should be handled.

> (Ultimately it's just
> an optimisation, so not a blocker). If so, one could introduce an additional
> seqnum for the underlying pixels excl. global alpha basically. 

... and maybe add GstVideoOverlayFormatFlags param to
gst_video_overlay_rectangle_get_seqnum(). (Although the API has already been
released?)

> But then,
> perhaps a consumer that does support global alpha should just check the
> GstBuffer pointers it gets 

Also a possibility, one that would already be possible without further changes,
but maybe a bit hackish? 

> - we could put a seqnum into GST_BUFFER_OFFSET() or
> so which could be used along the same lines.

You lost me here.

> 
> On a side note, if the seqnum of the rectangle is updated like that in
> _rectangle_set_global_alpha(), the composition the rectangle belongs to (if
> any) won't know about the change, so min_seq_num_used might not be as high as
> it could be. Having said that, we don't provide API to query min_seq_num yet,
> so it doesn't matter anyway.

True, but this depends also on the usage of the API on the producer side.
As there is no _composition_remove_rectangle() the only way for the producer to
update an already existing rectangle for changed data is 

_composition_unref(cached_comp);
_composition_new(cached_rect);
foreach (cached_rects as r)
... _add_rectangle(r)

and practically (as does textoverlay and our plugin by now), for convenience
these steps are done per _chain() call, assuming that the renderer/vaapisink
checks seq_nums per rectangle.

This is, btw, a bit unclear in the API description. Also
draft-subtitle-overlays.txt is not very clear about the expected handling of
composition-lifetime vs. rectangle-lifetime.

> 
> 
> > - The current implementation is 'optimized' for speed and saving memory. I
> > first tried an implementation integrating nicely into the internal caching of
> > preprocessed/scaled rectangles, meaning that after each application of
> > global_alpha the resulting pixel-data was added to the rectangle cache. This
> > turned out to be too inefficient in terms of CPU-usage and memory consumption
> 
> Why inefficient in terms of CPU usage?
> 
> As for memory consumption: there's no reason why one can't remove cached
> rectangles. It would be perfectly reasonable to only keep "the last" modified
> rectangle around.

Well, the showstopper when testing the first code was actually CPU usage due to
the massive memdup/memcpy done in each fading step/per frame for multiple
composition rects. Therefore keeping "only the last" in the scaled_rectangles
list is already too expensive as copying memory is involved. But "the last" is
actually already always there, because the result of the last global_alpha
multiplication is always in GstOverlayRectangle.pixels ;-)


Ok, for the rest: will change or verify. 

Will also try again for the test-cases, but your hints for getting ORC work did
not work here (timeout due to non-returning calls), so will have problems
testing the tests :-)

Please let me know how you decide for the seq_num issue, to avoid further
re-coding ;-)

Thanks,
  Holger

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.