[vlc-devel] [vlc-commits] picture: factor freeing picture_t

Steve Lhomme robux4 at ycbcr.xyz
Thu Dec 13 10:16:35 CET 2018


This crashes on Windows. The commits after that don't fix it.
It crashes with software and hardware chromas when freeing the decoded 
picture.

Can you send your patches on the ML when you have big changes like that 
that you are not sure about ? We all do. I assume this was tested before 
pushing.

On 12/12/2018 18:57, Rémi Denis-Courmont wrote:
> vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Wed Dec 12 19:56:02 2018 +0200| [e6f89b02c75600250cb2eba809caaff7cc4fe53e] | committer: Rémi Denis-Courmont
>
> picture: factor freeing picture_t
>
> Due to private members, manually allocating a picture has not been
> possible for a while. So we can assume that all picture_t are allocated
> with either picture_NewFromResource() or picture_NewFromFormat().
> And we can then just factor the final free() call.
>
>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=e6f89b02c75600250cb2eba809caaff7cc4fe53e
> ---
>
>   modules/hw/d3d11/d3d11_surface.c              | 1 -
>   modules/hw/d3d9/dxa9.c                        | 1 -
>   modules/hw/vaapi/vlc_vaapi.c                  | 1 -
>   modules/hw/vdpau/display.c                    | 1 -
>   modules/video_chroma/copy.c                   | 1 -
>   modules/video_output/android/display.h        | 1 -
>   modules/video_output/kms.c                    | 1 -
>   modules/video_output/opengl/converter_sw.c    | 1 -
>   modules/video_output/opengl/converter_vdpau.c | 1 -
>   modules/video_output/vulkan/display.c         | 1 -
>   modules/video_output/wayland/shm.c            | 1 -
>   modules/video_output/win32/direct3d11.c       | 1 -
>   modules/video_output/win32/direct3d9.c        | 3 +--
>   modules/video_output/xcb/pictures.c           | 2 --
>   src/misc/picture.c                            | 3 +--
>   src/misc/picture_pool.c                       | 2 --
>   16 files changed, 2 insertions(+), 20 deletions(-)
>
> diff --git a/modules/hw/d3d11/d3d11_surface.c b/modules/hw/d3d11/d3d11_surface.c
> index 14266ba458..9393cdb0dc 100644
> --- a/modules/hw/d3d11/d3d11_surface.c
> +++ b/modules/hw/d3d11/d3d11_surface.c
> @@ -499,7 +499,6 @@ static void DestroyPicture(picture_t *picture)
>       picture_sys_t *p_sys = picture->p_sys;
>       ReleasePictureSys( p_sys );
>       free(p_sys);
> -    free(picture);
>   }
>   
>   static void DeleteFilter( filter_t * p_filter )
> diff --git a/modules/hw/d3d9/dxa9.c b/modules/hw/d3d9/dxa9.c
> index 3ea297bcbd..ee40ad185f 100644
> --- a/modules/hw/d3d9/dxa9.c
> +++ b/modules/hw/d3d9/dxa9.c
> @@ -175,7 +175,6 @@ static void DestroyPicture(picture_t *picture)
>       picture_sys_t *p_sys = picture->p_sys;
>       ReleasePictureSys( p_sys );
>       free(p_sys);
> -    free(picture);
>   }
>   
>   static void DeleteFilter( filter_t * p_filter )
> diff --git a/modules/hw/vaapi/vlc_vaapi.c b/modules/hw/vaapi/vlc_vaapi.c
> index 3c69fea393..b6d6d4bd14 100644
> --- a/modules/hw/vaapi/vlc_vaapi.c
> +++ b/modules/hw/vaapi/vlc_vaapi.c
> @@ -564,7 +564,6 @@ pool_pic_destroy_cb(picture_t *pic)
>           free(instance);
>       }
>       free(pic->p_sys);
> -    free(pic);
>   }
>   
>   static void
> diff --git a/modules/hw/vdpau/display.c b/modules/hw/vdpau/display.c
> index 447b0499f1..63ae458a65 100644
> --- a/modules/hw/vdpau/display.c
> +++ b/modules/hw/vdpau/display.c
> @@ -80,7 +80,6 @@ static void pictureSys_DestroyVDPAU(picture_sys_t *psys)
>   static void PictureDestroyVDPAU(picture_t *pic)
>   {
>       pictureSys_DestroyVDPAU(pic->p_sys);
> -    free(pic);
>   }
>   
>   static VdpStatus picture_NewVDPAU(vdp_t *vdp, VdpRGBAFormat rgb_fmt,
> diff --git a/modules/video_chroma/copy.c b/modules/video_chroma/copy.c
> index b0cf4a1042..322bffcc1e 100644
> --- a/modules/video_chroma/copy.c
> +++ b/modules/video_chroma/copy.c
> @@ -1085,7 +1085,6 @@ static void pic_rsc_destroy(picture_t *pic)
>   {
>       for (unsigned i = 0; i < 3; i++)
>           free(pic->p[i].p_pixels);
> -    free(pic);
>   }
>   
>   static picture_t *pic_new_unaligned(const video_format_t *fmt)
> diff --git a/modules/video_output/android/display.h b/modules/video_output/android/display.h
> index 4e0afcbb12..673de00543 100644
> --- a/modules/video_output/android/display.h
> +++ b/modules/video_output/android/display.h
> @@ -101,7 +101,6 @@ static inline void AndroidOpaquePicture_DetachVout(picture_t *p_pic)
>       }
>       else
>           vlc_mutex_unlock(&p_picsys->hw.lock);
> -    free(p_pic);
>   }
>   
>   static inline void
> diff --git a/modules/video_output/kms.c b/modules/video_output/kms.c
> index 1cce937330..bd65ee823f 100644
> --- a/modules/video_output/kms.c
> +++ b/modules/video_output/kms.c
> @@ -605,7 +605,6 @@ static void CustomDestroyPicture(picture_t *p_picture)
>       vlc_close(sys->drm_fd);
>       sys->drm_fd = 0;
>       free(p_picture->p_sys);
> -    free(p_picture);
>   }
>   
>   
> diff --git a/modules/video_output/opengl/converter_sw.c b/modules/video_output/opengl/converter_sw.c
> index ae37d4929f..152bd58ac3 100644
> --- a/modules/video_output/opengl/converter_sw.c
> +++ b/modules/video_output/opengl/converter_sw.c
> @@ -114,7 +114,6 @@ pbo_picture_destroy(picture_t *pic)
>           picsys->DeleteBuffers(pic->i_planes, picsys->buffers);
>   
>       free(picsys);
> -    free(pic);
>   }
>   
>   static picture_t *
> diff --git a/modules/video_output/opengl/converter_vdpau.c b/modules/video_output/opengl/converter_vdpau.c
> index 0599796473..1bc0d3b8a7 100644
> --- a/modules/video_output/opengl/converter_vdpau.c
> +++ b/modules/video_output/opengl/converter_vdpau.c
> @@ -70,7 +70,6 @@ pool_pic_destroy_cb(picture_t *pic)
>       vdp_output_surface_destroy(p_sys->vdp, p_sys->surface);
>       vdp_release_x11(p_sys->vdp);
>       free(p_sys);
> -    free(pic);
>   }
>   
>   static picture_pool_t *
> diff --git a/modules/video_output/vulkan/display.c b/modules/video_output/vulkan/display.c
> index 6be194e59b..e836be005d 100644
> --- a/modules/video_output/vulkan/display.c
> +++ b/modules/video_output/vulkan/display.c
> @@ -202,7 +202,6 @@ static void DestroyPicture(picture_t *pic)
>       pl_buf_destroy(gpu, &picsys->buf);
>       vlc_vk_Release(picsys->vk);
>       free(picsys);
> -    free(pic);
>   }
>   
>   static picture_t *CreatePicture(vout_display_t *vd)
> diff --git a/modules/video_output/wayland/shm.c b/modules/video_output/wayland/shm.c
> index 1964d47c77..6773dc7db6 100644
> --- a/modules/video_output/wayland/shm.c
> +++ b/modules/video_output/wayland/shm.c
> @@ -67,7 +67,6 @@ static void PictureDestroy(picture_t *pic)
>       size_t picsize = pic->p[0].i_pitch * pic->p[0].i_lines;
>   
>       munmap(pic->p[0].p_pixels, (picsize + pagemask) & ~pagemask);
> -    free(pic);
>   }
>   
>   static void buffer_release_cb(void *data, struct wl_buffer *buffer)
> diff --git a/modules/video_output/win32/direct3d11.c b/modules/video_output/win32/direct3d11.c
> index e70180e54b..5fd66467e1 100644
> --- a/modules/video_output/win32/direct3d11.c
> +++ b/modules/video_output/win32/direct3d11.c
> @@ -567,7 +567,6 @@ static void DestroyDisplayPoolPicture(picture_t *picture)
>       picture_sys_t *p_sys = picture->p_sys;
>       ReleasePictureSys( p_sys );
>       free(p_sys);
> -    free(picture);
>   }
>   
>   #if !VLC_WINSTORE_APP
> diff --git a/modules/video_output/win32/direct3d9.c b/modules/video_output/win32/direct3d9.c
> index b6525d8a95..bd1c36b9df 100644
> --- a/modules/video_output/win32/direct3d9.c
> +++ b/modules/video_output/win32/direct3d9.c
> @@ -217,7 +217,6 @@ static void DestroyPicture(picture_t *picture)
>       ReleasePictureSys(picture->p_sys);
>   
>       free(picture->p_sys);
> -    free(picture);
>   }
>   
>   /**
> @@ -328,7 +327,7 @@ static picture_pool_t *Direct3D9CreatePicturePool(vlc_object_t *o,
>   error:
>       if (pool == NULL && pictures) {
>           for (unsigned i=0;i<picture_count; ++i)
> -            DestroyPicture(pictures[i]);
> +            picture_Release(pictures[i]);
>       }
>       free(pictures);
>       return pool;
> diff --git a/modules/video_output/xcb/pictures.c b/modules/video_output/xcb/pictures.c
> index 2d43eee689..bcace0f76c 100644
> --- a/modules/video_output/xcb/pictures.c
> +++ b/modules/video_output/xcb/pictures.c
> @@ -83,7 +83,6 @@ fail:
>   static void XCB_picture_SysV_Destroy (picture_t *pic)
>   {
>       shmdt (pic->p[0].p_pixels);
> -    free (pic);
>   }
>   
>   static int XCB_picture_SysV_Alloc(vout_display_t *vd, picture_resource_t *res,
> @@ -151,7 +150,6 @@ static int XCB_picture_SysV_Alloc(vout_display_t *vd, picture_resource_t *res,
>   static void XCB_picture_Destroy(picture_t *pic)
>   {
>       free(pic->p[0].p_pixels);
> -    free(pic);
>   }
>   
>   /**
> diff --git a/src/misc/picture.c b/src/misc/picture.c
> index be6de71ea7..58aa37a120 100644
> --- a/src/misc/picture.c
> +++ b/src/misc/picture.c
> @@ -56,7 +56,6 @@ static void PictureDestroyContext( picture_t *p_picture )
>   static void picture_DestroyFromResource( picture_t *p_picture )
>   {
>       free( p_picture->p_sys );
> -    free( p_picture );
>   }
>   
>   /**
> @@ -68,7 +67,6 @@ static void picture_DestroyFromFormat(picture_t *pic)
>   
>       if (res != NULL)
>           picture_Deallocate(res->fd, res->base, res->size);
> -    free(pic);
>   }
>   
>   VLC_WEAK void *picture_Allocate(int *restrict fdp, size_t size)
> @@ -331,6 +329,7 @@ void picture_Destroy(picture_t *picture)
>       picture_priv_t *priv = container_of(picture, picture_priv_t, picture);
>       assert(priv->gc.destroy != NULL);
>       priv->gc.destroy(picture);
> +    free(priv);
>   }
>   
>   /*****************************************************************************
> diff --git a/src/misc/picture_pool.c b/src/misc/picture_pool.c
> index 874a04e4d8..9c896f840a 100644
> --- a/src/misc/picture_pool.c
> +++ b/src/misc/picture_pool.c
> @@ -77,8 +77,6 @@ static void picture_pool_ReleasePicture(picture_t *clone)
>       unsigned offset = sys & (POOL_MAX - 1);
>       picture_t *picture = pool->picture[offset];
>   
> -    free(clone);
> -
>       if (pool->pic_unlock != NULL)
>           pool->pic_unlock(picture);
>       picture_Release(picture);
>
> _______________________________________________
> vlc-commits mailing list
> vlc-commits at videolan.org
> https://mailman.videolan.org/listinfo/vlc-commits



More information about the vlc-devel mailing list