[vlc-devel] [PATCH 1/2] wingdi: remove pool

[Steve Lhomme]

On 20/12/2018 23:06, Rémi Denis-Courmont wrote:
> With only one picture buffer, copying is necessary anyhow.

Right now it works because converters/filters (unless you have an RGB 
source) end up writing in the one picture that's provided by the vout 
pool. So it's effectively a direct rendering even though it's not meant 
to be.

This patch adds an extra copy. In the rare cases where wingdi is used, 
it's probably CPU/resource constrained machines that may not work well 
with this extra copy (late frames for example).

I don't think this change is necessary right now. For push we might 
still be able to make use of this direct rendering.

> Let the core manage the picture pool.
> ---
>   modules/video_output/win32/wingdi.c | 29 +++++++++++++++--------------
>   1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/modules/video_output/win32/wingdi.c b/modules/video_output/win32/wingdi.c
> index 4cc97eb5fc..0da62df9ee 100644
> --- a/modules/video_output/win32/wingdi.c
> +++ b/modules/video_output/win32/wingdi.c
> @@ -63,6 +63,7 @@ struct vout_display_sys_t
>   {
>       vout_display_sys_win32_t sys;
>   
> +    picture_t *picture;
>       int  i_depth;
>   
>       /* Our offscreen bitmap and its framebuffer */
> @@ -78,7 +79,8 @@ struct vout_display_sys_t
>       };
>   };
>   
> -static picture_pool_t *Pool  (vout_display_t *, unsigned);
> +static void           Prepare(vout_display_t *, picture_t *, subpicture_t *,
> +                              vlc_tick_t);
>   static void           Display(vout_display_t *, picture_t *);
>   static int            Control(vout_display_t *, int, va_list);
>   
> @@ -109,8 +111,7 @@ static int Open(vout_display_t *vd, const vout_display_cfg_t *cfg,
>       vd->info.has_double_click     = true;
>       vd->info.has_pictures_invalid = true;
>   
> -    vd->pool    = Pool;
> -    vd->prepare = NULL;
> +    vd->prepare = Prepare;
>       vd->display = Display;
>       vd->control = Control;
>       return VLC_SUCCESS;
> @@ -131,10 +132,15 @@ static void Close(vout_display_t *vd)
>   }
>   
>   /* */
> -static picture_pool_t *Pool(vout_display_t *vd, unsigned count)
> +static void Prepare(vout_display_t *vd, picture_t *picture,
> +                    subpicture_t *subpic, vlc_tick_t date)
>   {
> -    VLC_UNUSED(count);
> -    return vd->sys->sys.pool;
> +    vout_display_sys_t *sys = vd->sys;
> +
> +    if (likely(sys->picture != NULL))
> +        picture_CopyPixels(sys->picture, picture);
> +
> +    VLC_UNUSED(subpic); VLC_UNUSED(date);
>   }
>   
>   static void Display(vout_display_t *vd, picture_t *picture)
> @@ -293,11 +299,7 @@ static int Init(vout_display_t *vd, video_format_t *fmt)
>       rsc.p[0].i_lines  = fmt->i_height;
>       rsc.p[0].i_pitch  = i_pic_pitch;;
>   
> -    picture_t *picture = picture_NewFromResource(fmt, &rsc);
> -    if (picture != NULL)
> -        sys->sys.pool = picture_pool_New(1, &picture);
> -    else
> -        sys->sys.pool = NULL;
> +    sys->picture = picture_NewFromResource(fmt, &rsc);

If it fails we should not return an error and not use this vout display 
module. That will also save some checks on NULL elsewhere.

>   
>       UpdateRects(vd, true);
>   
> @@ -308,9 +310,8 @@ static void Clean(vout_display_t *vd)
>   {
>       vout_display_sys_t *sys = vd->sys;
>   
> -    if (sys->sys.pool)
> -        picture_pool_Release(sys->sys.pool);
> -    sys->sys.pool = NULL;
> +    if (sys->picture != NULL)
> +        picture_Release(sys->picture);
>   
>       if (sys->off_dc)
>           DeleteDC(sys->off_dc);
> -- 
> 2.20.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel