[vlc-devel] [PATCH 04/15] display: make vd->fmt a const pointer

Steve Lhomme robux4 at ycbcr.xyz
Thu Sep 3 08:10:35 CEST 2020


On 2020-09-03 7:52, Steve Lhomme wrote:
> This will prevent display modules from modifying it anytime they want.
> They can modify the format on open and maybe later when a new format is pushed.

And on RESET_PICTURES for the few display modules that support it.

> The storage of the display video_format_t is in vout_display_priv_t.display_fmt.
> ---
>   include/vlc_vout_display.h              |  2 +-
>   modules/hw/mmal/vout.c                  | 36 ++++++++++++-------------
>   modules/hw/vdpau/display.c              |  4 +--
>   modules/video_output/decklink.cpp       |  4 +--
>   modules/video_output/fb.c               |  2 +-
>   modules/video_output/kms.c              |  2 +-
>   modules/video_output/kva.c              |  2 +-
>   modules/video_output/vmem.c             |  2 +-
>   modules/video_output/wayland/shm.c      |  8 +++---
>   modules/video_output/win32/direct3d11.c |  2 +-
>   modules/video_output/win32/direct3d9.c  |  4 +--
>   modules/video_output/yuv.c              |  6 ++---
>   src/video_output/display.c              | 28 ++++++++++---------
>   src/video_output/video_output.c         |  2 +-
>   src/video_output/vout_wrapper.c         |  2 +-
>   15 files changed, 54 insertions(+), 52 deletions(-)
> 
> diff --git a/include/vlc_vout_display.h b/include/vlc_vout_display.h
> index d5fc96a9193..830275b1e30 100644
> --- a/include/vlc_vout_display.h
> +++ b/include/vlc_vout_display.h
> @@ -302,7 +302,7 @@ struct vout_display_t {
>        * By default, it is equal to ::source except for the aspect ratio
>        * which is undefined(0) and is ignored.
>        */
> -    video_format_t fmt;
> +    const video_format_t *fmt;
>   
>       /* Information
>        *
> diff --git a/modules/hw/mmal/vout.c b/modules/hw/mmal/vout.c
> index d471971461f..d96328fc63d 100644
> --- a/modules/hw/mmal/vout.c
> +++ b/modules/hw/mmal/vout.c
> @@ -177,12 +177,12 @@ static MMAL_FOURCC_T vout_vlc_to_mmal_pic_fourcc(const unsigned int fcc)
>   
>   static void display_set_format(const vout_display_t * const vd, MMAL_ES_FORMAT_T *const es_fmt, const bool is_intermediate)
>   {
> -    const unsigned int w = is_intermediate ? vd->fmt.i_visible_width  : vd->fmt.i_width ;
> -    const unsigned int h = is_intermediate ? vd->fmt.i_visible_height : vd->fmt.i_height;
> +    const unsigned int w = is_intermediate ? vd->fmt->i_visible_width  : vd->fmt->i_width ;
> +    const unsigned int h = is_intermediate ? vd->fmt->i_visible_height : vd->fmt->i_height;
>       MMAL_VIDEO_FORMAT_T * const v_fmt = &es_fmt->es->video;
>   
>       es_fmt->type = MMAL_ES_TYPE_VIDEO;
> -    es_fmt->encoding = is_intermediate ? MMAL_ENCODING_I420 : vout_vlc_to_mmal_pic_fourcc(vd->fmt.i_chroma);
> +    es_fmt->encoding = is_intermediate ? MMAL_ENCODING_I420 : vout_vlc_to_mmal_pic_fourcc(vd->fmt->i_chroma);
>       es_fmt->encoding_variant = 0;
>   
>       v_fmt->width  = (w + 31) & ~31;
> @@ -191,25 +191,25 @@ static void display_set_format(const vout_display_t * const vd, MMAL_ES_FORMAT_T
>       v_fmt->crop.y = 0;
>       v_fmt->crop.width = w;
>       v_fmt->crop.height = h;
> -    if (vd->fmt.i_sar_num == 0 || vd->fmt.i_sar_den == 0) {
> +    if (vd->fmt->i_sar_num == 0 || vd->fmt->i_sar_den == 0) {
>           v_fmt->par.num        = 1;
>           v_fmt->par.den        = 1;
>       } else {
> -        v_fmt->par.num        = vd->fmt.i_sar_num;
> -        v_fmt->par.den        = vd->fmt.i_sar_den;
> +        v_fmt->par.num        = vd->fmt->i_sar_num;
> +        v_fmt->par.den        = vd->fmt->i_sar_den;
>       }
> -    v_fmt->frame_rate.num = vd->fmt.i_frame_rate;
> -    v_fmt->frame_rate.den = vd->fmt.i_frame_rate_base;
> -    v_fmt->color_space    = vlc_to_mmal_color_space(vd->fmt.space);
> +    v_fmt->frame_rate.num = vd->fmt->i_frame_rate;
> +    v_fmt->frame_rate.den = vd->fmt->i_frame_rate_base;
> +    v_fmt->color_space    = vlc_to_mmal_color_space(vd->fmt->space);
>   }
>   
>   static void display_src_rect(const vout_display_t * const vd, MMAL_RECT_T *const rect)
>   {
>       const bool wants_isp = false;
> -    rect->x = wants_isp ? 0 : vd->fmt.i_x_offset;
> -    rect->y = wants_isp ? 0 : vd->fmt.i_y_offset;
> -    rect->width = vd->fmt.i_visible_width;
> -    rect->height = vd->fmt.i_visible_height;
> +    rect->x = wants_isp ? 0 : vd->fmt->i_x_offset;
> +    rect->y = wants_isp ? 0 : vd->fmt->i_y_offset;
> +    rect->width = vd->fmt->i_visible_width;
> +    rect->height = vd->fmt->i_visible_height;
>   }
>   
>   static void isp_input_cb(MMAL_PORT_T *port, MMAL_BUFFER_HEADER_T *buf)
> @@ -830,7 +830,7 @@ static void vd_prepare(vout_display_t *vd, picture_t *p_pic,
>       }
>   
>       // *****
> -    if (want_copy(&vd->fmt)) {
> +    if (want_copy(vd->fmt)) {
>           if (sys->copy_buf != NULL) {
>               msg_Err(vd, "Copy buf not NULL");
>               mmal_buffer_header_release(sys->copy_buf);
> @@ -931,8 +931,8 @@ static void adjust_refresh_rate(vout_display_t *vd, const video_format_t *fmt)
>                                   mode->scan_mode == HDMI_INTERLACED)
>                       continue;
>               } else {
> -                if (mode->width != vd->fmt.i_visible_width ||
> -                        mode->height != vd->fmt.i_visible_height)
> +                if (mode->width != vd->fmt->i_visible_width ||
> +                        mode->height != vd->fmt->i_visible_height)
>                       continue;
>                   if (mode->scan_mode != sys->b_progressive ? HDMI_NONINTERLACED : HDMI_INTERLACED)
>                       continue;
> @@ -1105,9 +1105,9 @@ static int OpenMmalVout(vout_display_t *vd, const vout_display_cfg_t *cfg,
>       MMAL_STATUS_T status;
>       int ret = VLC_EGENERIC;
>       // At the moment all copy is via I420
> -    const bool needs_copy = !hw_mmal_chroma_is_mmal(vd->fmt.i_chroma);
> +    const bool needs_copy = !hw_mmal_chroma_is_mmal(vd->fmt->i_chroma);
>       const MMAL_FOURCC_T enc_in = needs_copy ? MMAL_ENCODING_I420 :
> -        vout_vlc_to_mmal_pic_fourcc(vd->fmt.i_chroma);
> +        vout_vlc_to_mmal_pic_fourcc(vd->fmt->i_chroma);
>   
>       sys = calloc(1, sizeof(struct vout_display_sys_t));
>       if (!sys)
> diff --git a/modules/hw/vdpau/display.c b/modules/hw/vdpau/display.c
> index 31e1164b4a7..e3484a72988 100644
> --- a/modules/hw/vdpau/display.c
> +++ b/modules/hw/vdpau/display.c
> @@ -260,8 +260,8 @@ static int Control(vout_display_t *vd, int query, va_list ap)
>           vout_display_place_t place;
>   
>           vout_display_PlacePicture(&place, vd->source, cfg);
> -        if (place.width  != vd->fmt.i_visible_width
> -         || place.height != vd->fmt.i_visible_height)
> +        if (place.width  != vd->fmt->i_visible_width
> +         || place.height != vd->fmt->i_visible_height)
>               return VLC_EGENERIC;
>   
>           const uint32_t values[] = { place.x, place.y,
> diff --git a/modules/video_output/decklink.cpp b/modules/video_output/decklink.cpp
> index cfe069c34ff..5007d5be7fe 100644
> --- a/modules/video_output/decklink.cpp
> +++ b/modules/video_output/decklink.cpp
> @@ -667,8 +667,8 @@ static void PrepareVideo(vout_display_t *vd, picture_t *picture, subpicture_t *,
>   
>       HRESULT result;
>       int w, h, stride, length;
> -    w = vd->fmt.i_width;
> -    h = vd->fmt.i_height;
> +    w = vd->fmt->i_width;
> +    h = vd->fmt->i_height;
>   
>       IDeckLinkMutableVideoFrame *pDLVideoFrame;
>       result = sys->p_output->CreateVideoFrame(w, h, w*3,
> diff --git a/modules/video_output/fb.c b/modules/video_output/fb.c
> index d8dcf8646a6..c06700377f4 100644
> --- a/modules/video_output/fb.c
> +++ b/modules/video_output/fb.c
> @@ -598,7 +598,7 @@ static int OpenDisplay(vout_display_t *vd, bool force_resolution)
>   
>       picture_resource_t rsc = { 0 };
>   
> -    sys->picture = picture_NewFromResource(&vd->fmt, &rsc);
> +    sys->picture = picture_NewFromResource(vd->fmt, &rsc);
>       if (unlikely(sys->picture == NULL)) {
>           munmap(sys->video_ptr, sys->video_size);
>           ioctl(sys->fd, FBIOPUT_VSCREENINFO, &sys->old_info);
> diff --git a/modules/video_output/kms.c b/modules/video_output/kms.c
> index 3e686e1414a..f0b5d32ebcc 100644
> --- a/modules/video_output/kms.c
> +++ b/modules/video_output/kms.c
> @@ -578,7 +578,7 @@ static int OpenDisplay(vout_display_t *vd)
>   
>       picture_resource_t rsc = { 0 };
>   
> -    sys->picture = picture_NewFromResource(&vd->fmt, &rsc);
> +    sys->picture = picture_NewFromResource(vd->fmt, &rsc);
>   
>       if (!sys->picture)
>           goto err_out;
> diff --git a/modules/video_output/kva.c b/modules/video_output/kva.c
> index 821dcc46b3d..cda74bddabe 100644
> --- a/modules/video_output/kva.c
> +++ b/modules/video_output/kva.c
> @@ -592,7 +592,7 @@ static int OpenDisplay( vout_display_t *vd, video_format_t *fmt )
>       char *title = var_InheritString( vd, "video-title" );
>       if (title != NULL
>        || asprintf( &title, VOUT_TITLE " (%4.4s to %4.4s - %s mode KVA output)",
> -                  (char *)&vd->fmt.i_chroma, (char *)&sys->kvas.fccSrcColor,
> +                  (char *)&vd->fmt->i_chroma, (char *)&sys->kvas.fccSrcColor,
>                     psz_video_mode[sys->kvac.ulMode - 1] ) >= 0)
>       {
>           WinSetWindowText( sys->frame, title );
> diff --git a/modules/video_output/vmem.c b/modules/video_output/vmem.c
> index 7463fb1a7b9..2de886284ee 100644
> --- a/modules/video_output/vmem.c
> +++ b/modules/video_output/vmem.c
> @@ -244,7 +244,7 @@ static void Prepare(vout_display_t *vd, picture_t *pic, subpicture_t *subpic,
>   
>       sys->pic_opaque = sys->lock(sys->opaque, planes);
>   
> -    picture_t *locked = picture_NewFromResource(&vd->fmt, &rsc);
> +    picture_t *locked = picture_NewFromResource(vd->fmt, &rsc);
>       if (likely(locked != NULL)) {
>           for (unsigned i = 0; i < PICTURE_PLANE_MAX; i++) {
>               locked->p[i].p_pixels = planes[i];
> diff --git a/modules/video_output/wayland/shm.c b/modules/video_output/wayland/shm.c
> index 1d84ec27fe0..10a9cf3d022 100644
> --- a/modules/video_output/wayland/shm.c
> +++ b/modules/video_output/wayland/shm.c
> @@ -115,11 +115,11 @@ static void Prepare(vout_display_t *vd, picture_t *pic, subpicture_t *subpic,
>       }
>   
>       if (sys->viewport == NULL) /* Poor man's crop */
> -        offset += 4 * vd->fmt.i_x_offset
> -                  + pic->p->i_pitch * vd->fmt.i_y_offset;
> +        offset += 4 * vd->fmt->i_x_offset
> +                  + pic->p->i_pitch * vd->fmt->i_y_offset;
>   
> -    buf = wl_shm_pool_create_buffer(pool, offset, vd->fmt.i_visible_width,
> -                                    vd->fmt.i_visible_height, stride,
> +    buf = wl_shm_pool_create_buffer(pool, offset, vd->fmt->i_visible_width,
> +                                    vd->fmt->i_visible_height, stride,
>                                       WL_SHM_FORMAT_XRGB8888);
>       wl_shm_pool_destroy(pool);
>       if (buf == NULL)
> diff --git a/modules/video_output/win32/direct3d11.c b/modules/video_output/win32/direct3d11.c
> index f67b677d201..e116fff9c76 100644
> --- a/modules/video_output/win32/direct3d11.c
> +++ b/modules/video_output/win32/direct3d11.c
> @@ -257,7 +257,7 @@ static void UpdateSize(vout_display_t *vd)
>       msg_Dbg(vd, "Detected size change %dx%d", sys->area.place.width,
>               sys->area.place.height);
>   
> -    UpdateDisplayFormat(vd, &vd->fmt);
> +    UpdateDisplayFormat(vd, vd->fmt);
>   
>       RECT rect_dst = {
>           .left   = sys->area.place.x,
> diff --git a/modules/video_output/win32/direct3d9.c b/modules/video_output/win32/direct3d9.c
> index 01581ed0f7f..60217fe3e59 100644
> --- a/modules/video_output/win32/direct3d9.c
> +++ b/modules/video_output/win32/direct3d9.c
> @@ -1162,7 +1162,7 @@ static void Prepare(vout_display_t *vd, picture_t *picture,
>                   return VLC_EGENERIC;
>           }
>   #endif
> -        UpdateOutput(vd, &vd->fmt, NULL);
> +        UpdateOutput(vd, vd->fmt, NULL);
>   
>           sys->clear_scene = true;
>           sys->area.place_changed = false;
> @@ -1176,7 +1176,7 @@ static void Prepare(vout_display_t *vd, picture_t *picture,
>           if (hr == D3DERR_DEVICENOTRESET && !sys->reset_device) {
>               sys->reset_device = true;
>               /* FIXME what to do here in case of failure */
> -            if (Direct3D9Reset(vd, &vd->fmt)) {
> +            if (Direct3D9Reset(vd, vd->fmt)) {
>                   msg_Err(vd, "Failed to reset device");
>                   return;
>               }
> diff --git a/modules/video_output/yuv.c b/modules/video_output/yuv.c
> index 6667a63fe05..6d7003e863a 100644
> --- a/modules/video_output/yuv.c
> +++ b/modules/video_output/yuv.c
> @@ -175,7 +175,7 @@ static void Display(vout_display_t *vd, picture_t *picture)
>       vout_display_sys_t *sys = vd->sys;
>   
>       /* */
> -    video_format_t fmt = vd->fmt;
> +    video_format_t fmt = *vd->fmt;
>   
>       if (ORIENT_IS_SWAP(vd->source->orientation))
>       {
> @@ -232,8 +232,8 @@ static void Display(vout_display_t *vd, picture_t *picture)
>           const plane_t *plane = &picture->p[i];
>           const uint8_t *pixels = plane->p_pixels;
>   
> -        pixels += (vd->fmt.i_x_offset * plane->i_visible_pitch)
> -                  / vd->fmt.i_visible_height;
> +        pixels += (vd->fmt->i_x_offset * plane->i_visible_pitch)
> +                  / vd->fmt->i_visible_height;
>   
>           for( int y = 0; y < plane->i_visible_lines; y++) {
>               const size_t written = fwrite(pixels, 1, plane->i_visible_pitch,
> diff --git a/src/video_output/display.c b/src/video_output/display.c
> index a394ad30e27..ea7fd7ae5aa 100644
> --- a/src/video_output/display.c
> +++ b/src/video_output/display.c
> @@ -51,9 +51,9 @@ static picture_t *VideoBufferNew(filter_t *filter)
>       vout_display_t *vd = filter->owner.sys;
>       const video_format_t *fmt = &filter->fmt_out.video;
>   
> -    assert(vd->fmt.i_chroma == fmt->i_chroma &&
> -           vd->fmt.i_width  == fmt->i_width  &&
> -           vd->fmt.i_height == fmt->i_height);
> +    assert(vd->fmt->i_chroma == fmt->i_chroma &&
> +           vd->fmt->i_width  == fmt->i_width  &&
> +           vd->fmt->i_height == fmt->i_height);
>   
>       picture_pool_t *pool = vout_GetPool(vd, 3);
>       if (!pool)
> @@ -281,6 +281,7 @@ typedef struct {
>   
>       /* */
>       video_format_t source;
> +    video_format_t display_fmt;
>       vlc_video_context *src_vctx;
>        /* filters to convert the vout source to fmt, NULL means no conversion
>         * can be done and nothing will be displayed */
> @@ -297,14 +298,14 @@ static int vout_display_start(void *func, bool forced, va_list ap)
>       vlc_video_context *context = osys->src_vctx;
>   
>       /* Picture buffer does not have the concept of aspect ratio */
> -    video_format_Copy(&vd->fmt, vd->source);
> -    vd->fmt.i_sar_num = 0;
> -    vd->fmt.i_sar_den = 0;
> +    video_format_Copy(&osys->display_fmt, vd->source);
> +    osys->display_fmt.i_sar_num = 0;
> +    osys->display_fmt.i_sar_den = 0;
>       vd->obj.force = forced; /* TODO: pass to activate() instead? */
>   
> -    int ret = activate(vd, cfg, &vd->fmt, context);
> +    int ret = activate(vd, cfg, &osys->display_fmt, context);
>       if (ret != VLC_SUCCESS) {
> -        video_format_Clean(&vd->fmt);
> +        video_format_Clean(&osys->display_fmt);
>           vlc_objres_clear(VLC_OBJECT(vd));
>       }
>       return ret;
> @@ -338,7 +339,7 @@ static int VoutDisplayCreateRender(vout_display_t *vd)
>       v_src.i_sar_num = 0;
>       v_src.i_sar_den = 0;
>   
> -    video_format_t v_dst = vd->fmt;
> +    video_format_t v_dst = *vd->fmt;
>       v_dst.i_sar_num = 0;
>       v_dst.i_sar_den = 0;
>   
> @@ -412,7 +413,7 @@ picture_pool_t *vout_GetPool(vout_display_t *vd, unsigned count)
>       vout_display_priv_t *osys = container_of(vd, vout_display_priv_t, display);
>   
>       if (osys->pool == NULL)
> -        osys->pool = picture_pool_NewFromFormat(&vd->fmt, count);
> +        osys->pool = picture_pool_NewFromFormat(vd->fmt, count);
>       return osys->pool;
>   }
>   
> @@ -469,7 +470,7 @@ static void vout_display_Reset(vout_display_t *vd)
>       }
>   
>       if (vout_display_Control(vd, VOUT_DISPLAY_RESET_PICTURES, &osys->cfg,
> -                             &vd->fmt)
> +                             &osys->display_fmt)
>        || VoutDisplayCreateRender(vd))
>           msg_Err(vd, "Failed to adjust render format");
>   }
> @@ -737,6 +738,7 @@ vout_display_t *vout_display_New(vlc_object_t *parent,
>       /* */
>       vout_display_t *vd = &osys->display;
>       vd->source = &osys->source;
> +    vd->fmt = &osys->display_fmt;
>       vd->owner = &osys->display_owner;
>       vd->info = (vout_display_info_t){ };
>       vd->cfg = &osys->cfg;
> @@ -766,7 +768,7 @@ vout_display_t *vout_display_New(vlc_object_t *parent,
>           if (vd->close != NULL)
>               vd->close(vd);
>           vlc_objres_clear(VLC_OBJECT(vd));
> -        video_format_Clean(&vd->fmt);
> +        video_format_Clean(&osys->display_fmt);
>           goto error;
>       }
>       return vd;
> @@ -797,6 +799,6 @@ void vout_display_Delete(vout_display_t *vd)
>       vlc_objres_clear(VLC_OBJECT(vd));
>   
>       video_format_Clean(&osys->source);
> -    video_format_Clean(&vd->fmt);
> +    video_format_Clean(&osys->display_fmt);
>       vlc_object_delete(vd);
>   }
> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
> index 1f2e3243887..769fed2a442 100644
> --- a/src/video_output/video_output.c
> +++ b/src/video_output/video_output.c
> @@ -1297,7 +1297,7 @@ static int ThreadDisplayRenderPicture(vout_thread_sys_t *vout, bool is_forced)
>           if (do_early_spu) {
>               fmt_spu = *vd->source;
>           } else {
> -            fmt_spu = vd->fmt;
> +            fmt_spu = *vd->fmt;
>               fmt_spu.i_sar_num = vd->cfg->display.sar.num;
>               fmt_spu.i_sar_den = vd->cfg->display.sar.den;
>           }
> diff --git a/src/video_output/vout_wrapper.c b/src/video_output/vout_wrapper.c
> index 78c6ec8f27a..9d819f8a2df 100644
> --- a/src/video_output/vout_wrapper.c
> +++ b/src/video_output/vout_wrapper.c
> @@ -117,7 +117,7 @@ vout_display_t *vout_OpenWrapper(vout_thread_t *vout, vout_thread_private_t *sys
>       var_AddCallback(vout, "video-wallpaper", Forward, vd);
>   #endif
>       var_SetBool(VLC_OBJECT(vout), "viewpoint-changeable",
> -                vd->fmt.projection_mode != PROJECTION_MODE_RECTANGULAR);
> +                vd->fmt->projection_mode != PROJECTION_MODE_RECTANGULAR);
>       return vd;
>   
>   error:
> -- 
> 2.26.2
> 
> _______________________________________________
> 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