[vlc-devel] [PATCH 01/19] vout: kms: don't use a custom picture destroy

[Alexandre Janniaux]

Hi,

On Thu, Jul 30, 2020 at 02:27:48PM +0200, Steve Lhomme wrote:
> On 2020-07-30 14:26, Alexandre Janniaux wrote:
> > Hi,
> >
> > Missing initialization for picture_resource_t.
>
> The planes are still initialized as before. Only the callback and p_sys are
> removed.

No, like I mentionned in previous posts:

> > > -    picture_resource_t rsc = {
> > > -        .p_sys = psys,
> > > -        .pf_destroy = CustomDestroyPicture,
> > > -    };

This is a designated initializer, it initializes omitted
fields to the initial value of the bss / like other objects
that have static storage duration, which is zero in all
reasonable crt0. But...

> > > +    picture_resource_t rsc = {};

...this initializer doesn't exist in C, it's a GNU extension,
and it generates warnings.

Regards,
--
Alexandre Janniaux
Videolabs


>
> > Regards,
> > --
> > Alexandre Janniaux
> > Videolabs
> >
> > On Thu, Jul 30, 2020 at 02:16:42PM +0200, Steve Lhomme wrote:
> > > The picture is not used outside of this file. We don't need to use the picture
> > > release refcount to release the resources used in the picture.
> > >
> > > That's how other display modules handle their dummy local picture. The picture
> > > is allocated locally and the resources are released locally at the end.
> > > ---
> > >   modules/video_output/kms.c | 25 +++----------------------
> > >   1 file changed, 3 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/modules/video_output/kms.c b/modules/video_output/kms.c
> > > index be4a6669788..8d2e3382c22 100644
> > > --- a/modules/video_output/kms.c
> > > +++ b/modules/video_output/kms.c
> > > @@ -101,11 +101,6 @@ struct vout_display_sys_t {
> > >       int             drm_fd;
> > >   };
> > >
> > > -typedef struct {
> > > -    vout_display_sys_t  *p_voutsys;
> > > -} picture_sys_t;
> > > -
> > > -
> > >   static void DestroyFB(vout_display_sys_t const *sys, uint32_t const buf)
> > >   {
> > >       struct drm_mode_destroy_dumb destroy_req = { .handle = sys->handle[buf] };
> > > @@ -491,10 +486,8 @@ static bool ChromaNegotiation(vout_display_t *vd)
> > >       return false;
> > >   }
> > >
> > > -static void CustomDestroyPicture(picture_t *p_picture)
> > > +static void CustomDestroyPicture(vout_display_sys_t *sys)
> > >   {
> > > -    picture_sys_t *psys = (picture_sys_t*)p_picture->p_sys;
> > > -    vout_display_sys_t *sys = (vout_display_sys_t *)psys->p_voutsys;
> > >       int c;
> > >
> > >       for (c = 0; c < MAXHWBUF; c++)
> > > @@ -504,7 +497,6 @@ static void CustomDestroyPicture(picture_t *p_picture)
> > >       drmDropMaster(sys->drm_fd);
> > >       vlc_close(sys->drm_fd);
> > >       sys->drm_fd = 0;
> > > -    free(p_picture->p_sys);
> > >   }
> > >
> > >   static int OpenDisplay(vout_display_t *vd)
> > > @@ -584,14 +576,7 @@ static int OpenDisplay(vout_display_t *vd)
> > >       if (!found_connector)
> > >           goto err_out;
> > >
> > > -    picture_sys_t *psys = calloc(1, sizeof(*psys));
> > > -    if (psys == NULL)
> > > -        goto err_out;
> > > -
> > > -    picture_resource_t rsc = {
> > > -        .p_sys = psys,
> > > -        .pf_destroy = CustomDestroyPicture,
> > > -    };
> > > +    picture_resource_t rsc = {};
> > >
> > >       for (size_t i = 0; i < PICTURE_PLANE_MAX; i++) {
> > >           rsc.p[i].p_pixels = sys->map[0] + sys->offsets[i];
> > > @@ -599,15 +584,10 @@ static int OpenDisplay(vout_display_t *vd)
> > >           rsc.p[i].i_pitch  = sys->stride;
> > >       }
> > >
> > > -    psys->p_voutsys = sys;
> > > -
> > >       sys->picture = picture_NewFromResource(&vd->fmt, &rsc);
> > >
> > >       if (!sys->picture)
> > > -    {
> > > -        free(psys);
> > >           goto err_out;
> > > -    }
> > >
> > >       return VLC_SUCCESS;
> > >   err_out:
> > > @@ -676,6 +656,7 @@ static void Close(vout_display_t *vd)
> > >
> > >       if (sys->picture)
> > >           picture_Release(sys->picture);
> > > +    CustomDestroyPicture(sys);
> > >
> > >       if (sys->drm_fd)
> > >           drmDropMaster(sys->drm_fd);
> > > --
> > > 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