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 766018 - gl/dispmanx: Implements set_render_rectangle to adjust the position of window
gl/dispmanx: Implements set_render_rectangle to adjust the position of window
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.9.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-05-05 08:19 UTC by Gwang Yoon Hwang
Modified: 2016-10-09 17:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Initial patch (4.68 KB, patch)
2016-05-05 08:24 UTC, Gwang Yoon Hwang
committed Details | Review

Description Gwang Yoon Hwang 2016-05-05 08:19:18 UTC
We cannot set the x, y coordinate of the video frame at the dispmanx at this point. We need to teach dispmanx backend to understand about set_render_rectangle API to draw a video with other UI.

This patch keeps the current behavior which places video frame at the center of the display if there is no set_render_rectangle call to the dispmanx window.

I have some doubt about the API design. 
It is impossible to revert to the default center-align behavior if we call it once.
Maybe we can reset the behavior by calling set_render_rectangle again with weird range (such as 0x0 rect). But I'm not sure about whether is it right design or not.
Comment 1 Gwang Yoon Hwang 2016-05-05 08:24:13 UTC
Created attachment 327329 [details] [review]
Initial patch
Comment 2 Matthew Waters (ystreet00) 2016-05-05 08:31:02 UTC
Review of attachment 327329 [details] [review]:

Looks good.

For unsetting the render rectangle, "pass -1 for the width and height parameters." from the gst_video_rectangle_set_render_rectangle() documentation.
Comment 3 Gwang Yoon Hwang 2016-05-05 08:50:43 UTC
(In reply to Matthew Waters (ystreet00) from comment #2)
> Review of attachment 327329 [details] [review] [review]:
> 
> Looks good.
> 
> For unsetting the render rectangle, "pass -1 for the width and height
> parameters." from the gst_video_rectangle_set_render_rectangle()
> documentation.

Ah, thanks!
I didn't know it was already documented. :)
It looks like we need to implement "resetting region" behavior to the
GstGLWindowWayland and remove the early return from GstGLWindow, then.
Comment 4 Matthew Waters (ystreet00) 2016-09-28 07:10:28 UTC
Let's push this for now, the -1 width/height arguments can be fixed later.

commit b75ec0c43360c7d47a00d78a7d73b4d5430d8cf9
Author: Gwang Yoon Hwang <yoon@igalia.com>
Date:   Thu May 5 15:53:57 2016 +0900

    gl/dispmanx: Implements set_render_rectangle to adjust the position of window
    
    We cannot set the x, y coordinate of the video frame at the dispmanx at
    this point. We need to teach dispmanx backend to understand about
    set_render_rectangle API to draw a video with other UI.
    
    This patch keeps the current behavior which places video frame at the
    center of the display if there is no set_render_rectangle call to the
    dispmanx window.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=766018
Comment 5 Munez 2016-10-09 04:28:47 UTC
Comment on attachment 327329 [details] [review]
Initial patch

+  window_egl->render_rect.x = x;
+  window_egl->render_rect.y = x;   ===>  This should have been y ?

Even after applying this patch it is not behaving as expected.. Video window width/hight is fine but x,y cordinates are wrong it always displays at left bottom corner.. Is it expected ?
Comment 6 Matthew Waters (ystreet00) 2016-10-09 07:34:43 UTC
(In reply to Munez from comment #5)
> Comment on attachment 327329 [details] [review] [review]
> Initial patch
> 
> +  window_egl->render_rect.x = x;
> +  window_egl->render_rect.y = x;   ===>  This should have been y ?

Yes.

> Even after applying this patch it is not behaving as expected.. Video window
> width/hight is fine but x,y cordinates are wrong it always displays at left
> bottom corner.. Is it expected ?

No idea.
Comment 7 Munez 2016-10-09 09:11:24 UTC
What is the idea behind sink_center_rect() ?

 * Takes @src rectangle and position it at the center of @dst rectangle with or
 * without @scaling. It handles clipping if the @src rectangle is bigger than
 * the @dst one and @scaling is set to FALSE.

If I have set render rect as [0x0x1920x1080] and the video is [1280x720] what is the expected behavior ? 

and

If I have set render rect as [100x100x600x400] and the video is [1280x720] what is the expected behavior ?
Comment 8 Matthew Waters (ystreet00) 2016-10-09 09:24:26 UTC
Bugzilla is not a support forum.

Also, unless you are going to reopen this bug, replying to resolved bugs is not a good idea.

video_sink_center_rect() will scale a rectangle into another rectangle preserving the aspect ratio or not.
Comment 9 Munez 2016-10-09 17:52:31 UTC
I will to open an another bug and propose a patch for fixing
window_egl->render_rect.y = x;
and
gst_gl_window_dispmanx_egl_show() call to call resize only when render_rect is not set..