[vlc-devel] [PATCH 04/20] display: keep the placed picture on open and after changing display settings
Alexandre Janniaux
ajanni at videolabs.io
Tue Aug 25 11:49:36 CEST 2020
Hi,
On Tue, Aug 25, 2020 at 11:21:05AM +0200, Steve Lhomme wrote:
> On 2020-08-25 10:57, Alexandre Janniaux wrote:
> > Hi,
> >
> > This seems incompatible with OpenGL modules, since they
> > need the alignment swapped within picture_place to adapt
> > the coordinate system.
>
> It seems that the value processed in the core and what OpenGL renders match.
> (I can't tell because OpenGL seems broken on Windows)
What do you mean by match? It's not matching, it's the
opposite vertical direction.
> It's just that OpenGL has the vertical axis in the other way. So if you
> align the picture on top (in the core) in the OpenGL axis it's aligned at
> the bottom, hence the local processing.
Exactly.
> I guess there's no way around the local hack in OpenGL modules. It's
> probably not possible to tell OpenGL to reverse its vertical axis. And you
> can't just use the negative/opposite values or the top/bottom rectangle.
picture_place is supposed to be an helper to refactor code in
the different vout. Some vout might even not using it at all
because it would not support moving it, though I didn't check
if it were the case.
OpenGL is copying the configuration an never modify the core
one, so it's only translating this.
If you want to refactor this out of the vout modules, the
vout modules needs to be able to express their coordinate
systems, but I'm not confident it would be enough to not leak
the module abstraction in place into the core.
Regards,
--
Alexandre Janniaux
Videolabs
> > Regards,
> > --
> > Alexandre Janniaux
> > Videolabs
> >
> > On Tue, Aug 25, 2020 at 09:29:52AM +0200, Steve Lhomme wrote:
> > > ---
> > > include/vlc_vout_display.h | 31 ++++++++++++--------
> > > src/video_output/display.c | 52 +++++++++++++++++++++++----------
> > > src/video_output/video_output.c | 9 +++---
> > > 3 files changed, 60 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/include/vlc_vout_display.h b/include/vlc_vout_display.h
> > > index 485d5634c2a..cdc06fb9f55 100644
> > > --- a/include/vlc_vout_display.h
> > > +++ b/include/vlc_vout_display.h
> > > @@ -268,6 +268,18 @@ typedef int (*vout_display_open_cb)(vout_display_t *vd,
> > > set_capability( "vout display", priority )
> > >
> > >
> > > +/**
> > > + * Video placement.
> > > + *
> > > + * This structure stores the result of a vout_display_PlacePicture() call.
> > > + */
> > > +typedef struct {
> > > + int x; /*< Relative pixel offset from the display left edge */
> > > + int y; /*< Relative pixel offset from the display top edge */
> > > + unsigned width; /*< Picture pixel width */
> > > + unsigned height; /*< Picture pixel height */
> > > +} vout_display_place_t;
> > > +
> > > struct vout_display_t {
> > > struct vlc_object_t obj;
> > >
> > > @@ -278,6 +290,13 @@ struct vout_display_t {
> > > */
> > > const vout_display_cfg_t *cfg;
> > >
> > > + /**
> > > + * Placement of the video in the display area.
> > > + *
> > > + * This cannot be modified directly. It reflects the current values.
> > > + */
> > > + const vout_display_place_t *place;
> > > +
> > > /**
> > > * Source video format.
> > > *
> > > @@ -478,18 +497,6 @@ static inline bool vout_display_cfg_IsWindowed(const vout_display_cfg_t *cfg)
> > > VLC_API void vout_display_GetDefaultDisplaySize(unsigned *width, unsigned *height, const video_format_t *source, const vout_display_cfg_t *);
> > >
> > >
> > > -/**
> > > - * Video placement.
> > > - *
> > > - * This structure stores the result of a vout_display_PlacePicture() call.
> > > - */
> > > -typedef struct {
> > > - int x; /*< Relative pixel offset from the display left edge */
> > > - int y; /*< Relative pixel offset from the display top edge */
> > > - unsigned width; /*< Picture pixel width */
> > > - unsigned height; /*< Picture pixel height */
> > > -} vout_display_place_t;
> > > -
> > > /**
> > > * Compares two \ref vout_display_place_t.
> > > */
> > > diff --git a/src/video_output/display.c b/src/video_output/display.c
> > > index ebc3a94b615..5ff0400dad5 100644
> > > --- a/src/video_output/display.c
> > > +++ b/src/video_output/display.c
> > > @@ -226,12 +226,9 @@ void vout_display_PlacePicture(vout_display_place_t *place,
> > > void vout_display_TranslateMouseState(vout_display_t *vd, vlc_mouse_t *video,
> > > const vlc_mouse_t *window)
> > > {
> > > - vout_display_place_t place;
> > > -
> > > - /* Translate window coordinates to video coordinates */
> > > - vout_display_PlacePicture(&place, &vd->source, vd->cfg);
> > > + const vout_display_place_t *place = vd->place;
> > >
> > > - if (place.width <= 0 || place.height <= 0) {
> > > + if (place->width <= 0 || place->height <= 0) {
> > > memset(video, 0, sizeof (*video));
> > > return;
> > > }
> > > @@ -245,16 +242,16 @@ void vout_display_TranslateMouseState(vout_display_t *vd, vlc_mouse_t *video,
> > > y = wy;
> > > break;
> > > case ORIENT_TOP_RIGHT:
> > > - x = place.width - wx;
> > > + x = place->width - wx;
> > > y = wy;
> > > break;
> > > case ORIENT_BOTTOM_LEFT:
> > > x = wx;
> > > - y = place.height - wy;
> > > + y = place->height - wy;
> > > break;
> > > case ORIENT_BOTTOM_RIGHT:
> > > - x = place.width - wx;
> > > - y = place.height - wy;
> > > + x = place->width - wx;
> > > + y = place->height - wy;
> > > break;
> > > case ORIENT_LEFT_TOP:
> > > x = wy;
> > > @@ -262,24 +259,24 @@ void vout_display_TranslateMouseState(vout_display_t *vd, vlc_mouse_t *video,
> > > break;
> > > case ORIENT_LEFT_BOTTOM:
> > > x = wy;
> > > - y = place.width - wx;
> > > + y = place->width - wx;
> > > break;
> > > case ORIENT_RIGHT_TOP:
> > > - x = place.height - wy;
> > > + x = place->height - wy;
> > > y = wx;
> > > break;
> > > case ORIENT_RIGHT_BOTTOM:
> > > - x = place.height - wy;
> > > - y = place.width - wx;
> > > + x = place->height - wy;
> > > + y = place->width - wx;
> > > break;
> > > default:
> > > vlc_assert_unreachable();
> > > }
> > >
> > > video->i_x = vd->source.i_x_offset
> > > - + (int64_t)(x - place.x) * vd->source.i_visible_width / place.width;
> > > + + (int64_t)(x - place->x) * vd->source.i_visible_width / place->width;
> > > video->i_y = vd->source.i_y_offset
> > > - + (int64_t)(y - place.y) * vd->source.i_visible_height / place.height;
> > > + + (int64_t)(y - place->y) * vd->source.i_visible_height / place->height;
> > > video->i_pressed = window->i_pressed;
> > > video->b_double_click = window->b_double_click;
> > > }
> > > @@ -289,6 +286,7 @@ typedef struct {
> > >
> > > /* */
> > > vout_display_cfg_t cfg;
> > > + vout_display_place_t place;
> > >
> > > struct {
> > > int left;
> > > @@ -468,6 +466,12 @@ void vout_FilterFlush(vout_display_t *vd)
> > > filter_chain_VideoFlush(osys->converters);
> > > }
> > >
> > > +static void UpdatePlacedPicture(vout_display_priv_t *osys)
> > > +{
> > > + vout_display_t *vd = &osys->display;
> > > + vout_display_PlacePicture(&osys->place, &vd->source, &osys->cfg);
> > > +}
> > > +
> > > static void vout_display_Reset(vout_display_t *vd)
> > > {
> > > vout_display_priv_t *osys = container_of(vd, vout_display_priv_t, display);
> > > @@ -486,6 +490,8 @@ static void vout_display_Reset(vout_display_t *vd)
> > > osys->pool = NULL;
> > > }
> > >
> > > + UpdatePlacedPicture(osys);
> > > +
> > > if (vout_display_Control(vd, VOUT_DISPLAY_RESET_PICTURES, &vd->fmt)
> > > || VoutDisplayCreateRender(vd))
> > > msg_Err(vd, "Failed to adjust render format");
> > > @@ -547,6 +553,8 @@ static int vout_UpdateSourceCrop(vout_display_t *vd)
> > > video_format_Print(VLC_OBJECT(vd), "SOURCE ", &osys->source);
> > > video_format_Print(VLC_OBJECT(vd), "CROPPED", &vd->source);
> > >
> > > + UpdatePlacedPicture(osys);
> > > +
> > > int ret = vout_display_Control(vd, VOUT_DISPLAY_CHANGE_SOURCE_CROP);
> > > osys->crop.left = left - osys->source.i_x_offset;
> > > osys->crop.top = top - osys->source.i_y_offset;
> > > @@ -574,6 +582,8 @@ static int vout_SetSourceAspect(vout_display_t *vd,
> > > vd->source.i_sar_den = osys->source.i_sar_den;
> > > }
> > >
> > > + UpdatePlacedPicture(osys);
> > > +
> > > if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_SOURCE_ASPECT))
> > > ret = -1;
> > >
> > > @@ -634,6 +644,9 @@ void vout_display_SetSize(vout_display_t *vd, unsigned width, unsigned height)
> > >
> > > osys->cfg.display.width = width;
> > > osys->cfg.display.height = height;
> > > +
> > > + UpdatePlacedPicture(osys);
> > > +
> > > if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_DISPLAY_SIZE)
> > > || vout_display_CheckReset(vd))
> > > vout_display_Reset(vd);
> > > @@ -647,6 +660,9 @@ void vout_SetDisplayFilled(vout_display_t *vd, bool is_filled)
> > > return; /* nothing to do */
> > >
> > > osys->cfg.is_display_filled = is_filled;
> > > +
> > > + UpdatePlacedPicture(osys);
> > > +
> > > if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_DISPLAY_FILLED)
> > > || vout_display_CheckReset(vd))
> > > vout_display_Reset(vd);
> > > @@ -662,6 +678,9 @@ void vout_SetDisplayZoom(vout_display_t *vd, unsigned num, unsigned den)
> > >
> > > osys->cfg.zoom.num = num;
> > > osys->cfg.zoom.den = den;
> > > +
> > > + UpdatePlacedPicture(osys);
> > > +
> > > if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_ZOOM) ||
> > > vout_display_CheckReset(vd))
> > > vout_display_Reset(vd);
> > > @@ -773,6 +792,7 @@ vout_display_t *vout_display_New(vlc_object_t *parent,
> > > video_format_Copy(&vd->source, source);
> > > vd->info = (vout_display_info_t){ };
> > > vd->cfg = &osys->cfg;
> > > + vd->place = &osys->place;
> > > vd->prepare = NULL;
> > > vd->display = NULL;
> > > vd->control = NULL;
> > > @@ -781,6 +801,8 @@ vout_display_t *vout_display_New(vlc_object_t *parent,
> > > if (owner)
> > > vd->owner = *owner;
> > >
> > > + vout_display_PlacePicture(&osys->place, &vd->source, &osys->cfg);
> > > +
> > > if (vlc_module_load(vd, "vout display", module, module && *module != '\0',
> > > vout_display_start, vd, &osys->cfg,
> > > osys->src_vctx) == NULL)
> > > diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
> > > index 4f48754cf2e..a4e31081ca4 100644
> > > --- a/src/video_output/video_output.c
> > > +++ b/src/video_output/video_output.c
> > > @@ -1256,17 +1256,16 @@ static int ThreadDisplayRenderPicture(vout_thread_sys_t *vout, bool is_forced)
> > > const vlc_fourcc_t *subpicture_chromas;
> > > video_format_t fmt_spu;
> > > if (do_dr_spu) {
> > > - vout_display_place_t place;
> > > - vout_display_PlacePicture(&place, &vd->source, vd->cfg);
> > > + const vout_display_place_t *place = vd->place;
> > >
> > > fmt_spu = vd->source;
> > > - if (fmt_spu.i_width * fmt_spu.i_height < place.width * place.height) {
> > > + if (fmt_spu.i_width * fmt_spu.i_height < place->width * place->height) {
> > > fmt_spu.i_sar_num = vd->cfg->display.sar.num;
> > > fmt_spu.i_sar_den = vd->cfg->display.sar.den;
> > > fmt_spu.i_width =
> > > - fmt_spu.i_visible_width = place.width;
> > > + fmt_spu.i_visible_width = place->width;
> > > fmt_spu.i_height =
> > > - fmt_spu.i_visible_height = place.height;
> > > + fmt_spu.i_visible_height = place->height;
> > > }
> > > subpicture_chromas = vd->info.subpicture_chromas;
> > > } else {
> > > --
> > > 2.26.2
> > >
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> >
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list