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 663822 - [subtitleoverlay] should not hardcode to use textoverlay element
[subtitleoverlay] should not hardcode to use textoverlay element
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
0.10.x
Other Linux
: Normal normal
: 0.10.37
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-11-11 02:50 UTC by bcxa.sz
Modified: 2011-12-14 09:05 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description bcxa.sz 2011-11-11 02:50:03 UTC
subtitleoverlay bin is hardcoded to use textoverlay element when playbin2 detect subtitle in multimedia file.

The textoverlay is assumed that the upstream video element will provide decoded video buffer then blit the subtile buffer with the decoded video buffer.

This is not the case for embedded system. Some embedded system in order to use hardware accelation, the decoded video data can be in kernel space and send to display component from kernel to kernel. So in the user space, it can be the fake buffer from upsream video decoder element then it is not needed to blit the subtitle buffer. And the subtitle buffer can push to downstrean element (sink element) to display.

So I think subtitleovelay bin is hard coded to use textoverlay is not flexible.

One of basic idea of gstreamer is to select component with same caps in the sink pad according to RANK RPIORITY. But this is not the case for subtitleoverlay bin now. We provide own textoverlay element with same sink pad caps as gstreamer native textoverlay. But our own textoverlay element will not selected even though it has higher RANK.

Sorry, I am not native english speaker. Please let me know if you don't understand.
Comment 1 bcxa.sz 2011-11-11 02:57:26 UTC
someone recommend to use playbin2 text-sink property. And I tried it can work with some ugly code.

The text-sink property of playbin2 is just a 'text sink'. It has only one sink pad to receive text. Then this text sink bin will not get video height and width.

And video and text will not be in the same chain. To workaround this issue, upstream video decoder element will send heigh and width information to this text sink bin with event mechanism. But video decoder element should not have such kind of knowledge where the text sink bin is. Then it will be application to tell video decoder element who the text sink bin is. Then you see it make things complicated. And it will not be a good solution.

So my suggestion is that:
1. subtitleoverlay bin not hard code to use textoverlay
. developer can provide own textoverlay

or

2. playbin2 not hardcode to use subtitleoverlay bin
. developer can provide own subtitleoverlay bin
Comment 2 Vincent Penquerc'h 2011-11-11 11:41:15 UTC
It should work by autodetection of available modules.
For instance, the kate plugin (in -bad) has an overlay element that gets autoplugged by playbin2 for Kate streams in Ogg. This works fine, without having to tell playbin2 that it exists.

One thing that playsink does specific to textoverlay is that is gets considered even if it does not have "Overlay/Subtitle" or "Overlay/SubPicture" in its "type" string. Maybe this is why your own element does not get selected if it does not have one of these ?
Comment 3 bcxa.sz 2011-11-28 07:10:30 UTC
Hi Vincent,

I will look at but I still think the suboverlay hardcode to use textoverlay even though i have own developed textoverlay element (rank is higher, also have "Overlay/Subtitle").
Comment 4 Sebastian Dröge (slomo) 2011-11-28 16:46:41 UTC
subtitleoverlay has textoverlay hardcoded as the default renderer that is used when a subtitle parser is used (i.e. subparse) and no renderer is available.

It would make sense to make this configurable via properties instead of hardcoding textoverlay. A patch to do that would be acceptable, the property should work similar to the {text,video,audio}-sink properties of playbin2.
Comment 5 bcxa.sz 2011-12-12 12:01:39 UTC
welcome the patch :)

Or I will try to work out when I have time.
Comment 6 Sebastian Dröge (slomo) 2011-12-12 14:38:53 UTC
I guess the better solution would be to not add a property but do this with the element classes and priorities only. textoverlay should only be used if no compatible renderer with a rank >= marginal is available.


To make this feasable the code paths in _pad_blocked_cb() that currently differentiate between renderers and parsers need to be refactored and the renderer path has to be used after the parsers too. I'll try to get this done today/tomorrow.
Comment 7 Tim-Philipp Müller 2011-12-12 15:29:59 UTC
> The textoverlay is assumed that the upstream video element will provide decoded
> video buffer then blit the subtile buffer with the decoded video buffer.
> 
> This is not the case for embedded system. Some embedded system in order to use
> hardware accelation, the decoded video data can be in kernel space and send to
> display component from kernel to kernel. So in the user space, it can be the
> fake buffer from upsream video decoder element then it is not needed to blit
> the subtitle buffer. And the subtitle buffer can push to downstrean element
> (sink element) to display.
> 
> So I think subtitleovelay bin is hard coded to use textoverlay is not flexible.

Note that textoverlay is now capable of attaching subtitle overlays to such video buffers without blitting on the buffer directly, see the new GstVideoOverlayComposition API. The sink (for example) can then blit the subtitles on top of the video using a suitable hardware-specific API.
Comment 8 bcxa.sz 2011-12-13 09:34:10 UTC
(In reply to comment #6)
> I guess the better solution would be to not add a property but do this with the
> element classes and priorities only. textoverlay should only be used if no
> compatible renderer with a rank >= marginal is available.
> To make this feasable the code paths in _pad_blocked_cb() that currently
> differentiate between renderers and parsers need to be refactored and the
> renderer path has to be used after the parsers too. I'll try to get this done
> today/tomorrow.

Nice to see this.
Comment 9 Sebastian Dröge (slomo) 2011-12-13 12:35:53 UTC
commit cd11d6871603d030c7aace54ee6887c479632ec9
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Dec 13 13:28:47 2011 +0100

    subtitleoverlay: Refactor code to check if a property exists on an element

commit 87a4cbd0e3ec663a4ddcd3dfbe9fef872559bbc1
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Dec 13 13:20:24 2011 +0100

    subtitleoverlay: Refactor autoplugging code and select overlay element by rank too
    
    Previously we always used textoverlay for rendering the output of
    a parser, now the same code as for the renderers is used and the
    element with the highest rank is used.
    
    Fixes bug #663822.
Comment 10 bcxa.sz 2011-12-14 03:10:35 UTC
Thanks please let me which official release will include this fix. And I would like to try.
Comment 11 Sebastian Dröge (slomo) 2011-12-14 09:05:09 UTC
0.10.37 will contain this change but it will be released in a few months. You might want to try latest GIT instead, which is more or less what 0.10.36 will become plus some changes like this one.