[vlc-devel] [PATCH 1/2] vout: add window properties in vout_display_cfg_t

[Thomas Guillem]

On Fri, Mar 27, 2020, at 10:39, Thomas Guillem wrote:
> 
> 
> On Fri, Mar 27, 2020, at 10:18, Rémi Denis-Courmont wrote:
> > Le perjantaina 27. maaliskuuta 2020, 10.57.35 EET Thomas Guillem a écrit :
> > > It should be used before the initialisation of the vd plugin. During the
> > > lifetime of the vd plugin, only cfg->display size should be used.
> > > 
> > > Refs #22674
> > > ---
> > >  include/vlc_vout_display.h |  9 +++++++++
> > >  src/video_output/display.c | 29 +++++++++++++++++++++++++----
> > >  2 files changed, 34 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/include/vlc_vout_display.h b/include/vlc_vout_display.h
> > > index cd183fb56c..f9fbc4e0e1 100644
> > > --- a/include/vlc_vout_display.h
> > > +++ b/include/vlc_vout_display.h
> > > @@ -86,6 +86,7 @@ typedef struct vlc_video_align {
> > >   * window, as follows:
> > >   * - If \ref is_display_filled is set,
> > >   *   the video size is fitted to the display size.
> > > + * - If \ref window size is valid, the video size is set to the window
> > > size, * - Otherwise, the video size is determined from the original video
> > > format, *   multiplied by the zoom factor.
> > >   */
> > > @@ -102,6 +103,14 @@ typedef struct vout_display_cfg {
> > >          vlc_rational_t sar; /**< Requested sample aspect ratio */
> > >      } display;
> > > 
> > > +    /** Window properties (should be ignored from display modules) */
> > > +    struct {
> > > +        /** Current window width, if valid (>0), height need to be valid.
> > > */
> > 
> > Nit: Doxygen wants empty line to separate the short and long description.
> > 
> > I am not sure if windowing systems universally treat 0 as an invalid dimension 
> > though.
> > 
> > > +        unsigned width;
> > > +        /** Current window height, if valid (>0), width need to be valid.
> > > */ +        unsigned height;
> > > +    } window_props;
> > 
> > I would have first moved window as window.handle or something, and then added 
> > window.width and window.height, but whatever.
> 
> OK.

Finally, I really prefer my version. Indeed I don't like having "cfg->window.handle->handle.xid".

Or maybe, you have a better name for handle ?

> 
> > 
> > > +
> > >      /** Alignment of the video within the window */
> > >      vlc_video_align_t align;
> > > 
> > > diff --git a/src/video_output/display.c b/src/video_output/display.c
> > > index 76d49ee240..de8f39a836 100644
> > > --- a/src/video_output/display.c
> > > +++ b/src/video_output/display.c
> > > @@ -102,6 +102,9 @@ void vout_display_GetDefaultDisplaySize(unsigned *width,
> > > unsigned *height, const video_format_t *source, const vout_display_cfg_t
> > > *cfg) {
> > > +    assert((cfg->window_props.width == 0) == (cfg->window_props.height ==
> > > 0)); +
> > > +    /* Requested by the user */
> > >      if (cfg->display.width != 0 && cfg->display.height != 0) {
> > >          *width  = cfg->display.width;
> > >          *height = cfg->display.height;
> > > @@ -113,7 +116,14 @@ void vout_display_GetDefaultDisplaySize(unsigned
> > > *width, unsigned *height, *width  = (int64_t)source->i_visible_width *
> > > source->i_sar_num * cfg->display.height * cfg->display.sar.den /
> > > source->i_visible_height / source->i_sar_den / cfg->display.sar.num;
> > > *height = cfg->display.height;
> > > -    } else if (source->i_sar_num >= source->i_sar_den) {
> > > +    }
> > > +    /* Size reported by the window module */
> > > +    else if (cfg->window_props.width != 0) {
> > > +        *width = cfg->window_props.width;
> > > +        *height = cfg->window_props.height;
> > > +    }
> > > +    /* Use the original video size */
> > > +    else if (source->i_sar_num >= source->i_sar_den) {
> > >          *width  = (int64_t)source->i_visible_width * source->i_sar_num *
> > > cfg->display.sar.den / source->i_sar_den / cfg->display.sar.num; *height =
> > > source->i_visible_height;
> > >      } else {
> > > @@ -137,6 +147,10 @@ void vout_display_PlacePicture(vout_display_place_t
> > > *place, const video_format_t *source,
> > >                                 const vout_display_cfg_t *cfg)
> > >  {
> > > +    /* vout_display_PlacePicture() is called from vd plugins. They should
> > > not +     * care about the initial window properties. */
> > > +    assert(cfg->window_props.width == 0 && cfg->window_props.height == 0);
> > > +
> > >      /* */
> > >      memset(place, 0, sizeof(*place));
> > >      if (cfg->display.width == 0 || cfg->display.height == 0)
> > > @@ -733,10 +747,17 @@ vout_display_t *vout_display_New(vlc_object_t *parent,
> > > if (unlikely(osys == NULL))
> > >          return NULL;
> > > 
> > > +    unsigned display_width, display_height;
> > > +    vout_display_GetDefaultDisplaySize(&display_width, &display_height,
> > > +                                       source, cfg);
> > > +
> > >      osys->cfg = *cfg;
> > > -    vout_display_GetDefaultDisplaySize(&osys->cfg.display.width,
> > > -                                       &osys->cfg.display.height,
> > > -                                       source, &osys->cfg);
> > > +    /* The window size was used for the initial setup. Now it can be
> > > dropped in +     * favor of the calculated display size. */
> > > +    osys->cfg.display.width = display_width;
> > > +    osys->cfg.display.height = display_height;
> > > +    osys->cfg.window_props.width = osys->cfg.window_props.height = 0;
> > > +
> > >  #ifdef _WIN32
> > >      osys->reset_pictures = false;
> > >  #endif
> > 
> > 
> > -- 
> > 雷米‧德尼-库尔蒙
> > http://www.remlab.net/
> > 
> > 
> > 
> > _______________________________________________
> > 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