[Mesa-dev] [PATCH] st/vdpau: add device reference counting
Ilia Mirkin
imirkin at alum.mit.edu
Wed Aug 13 08:13:03 PDT 2014
On Wed, Aug 13, 2014 at 11:07 AM, Christian König
<deathsimple at vodafone.de> wrote:
> From: Christian König <christian.koenig at amd.com>
>
> This fixes an issue with flash where it tries to destroy a decoder
> after already destroying the device associated with the decoder.
>
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=82517
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
Have you considered the opposite approach -- tearing everything down
when the device is torn down, instead of keeping the device alive
longer? Any idea what NVIDIA does? (Do the VDPAU docs say anything
about this? I don't see it anywhere.)
The reason I bring it up is that now an application that doesn't
destroy everything about a device will end up leaking it, which may be
a heavier object to leak than just surfaces or something.
> ---
> src/gallium/state_trackers/vdpau/bitmap.c | 4 +++-
> src/gallium/state_trackers/vdpau/decode.c | 4 +++-
> src/gallium/state_trackers/vdpau/device.c | 21 ++++++++++++++++-----
> src/gallium/state_trackers/vdpau/mixer.c | 4 +++-
> src/gallium/state_trackers/vdpau/output.c | 4 +++-
> src/gallium/state_trackers/vdpau/presentation.c | 4 +++-
> src/gallium/state_trackers/vdpau/surface.c | 4 +++-
> src/gallium/state_trackers/vdpau/vdpau_private.h | 12 ++++++++++++
> 8 files changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/src/gallium/state_trackers/vdpau/bitmap.c b/src/gallium/state_trackers/vdpau/bitmap.c
> index a068921..97a4287 100644
> --- a/src/gallium/state_trackers/vdpau/bitmap.c
> +++ b/src/gallium/state_trackers/vdpau/bitmap.c
> @@ -67,7 +67,7 @@ vlVdpBitmapSurfaceCreate(VdpDevice device,
> if (!vlsurface)
> return VDP_STATUS_RESOURCES;
>
> - vlsurface->device = dev;
> + DeviceReference(&vlsurface->device, dev);
>
> memset(&res_tmpl, 0, sizeof(res_tmpl));
> res_tmpl.target = PIPE_TEXTURE_2D;
> @@ -117,6 +117,7 @@ err_sampler:
> pipe_sampler_view_reference(&vlsurface->sampler_view, NULL);
> err_unlock:
> pipe_mutex_unlock(dev->mutex);
> + DeviceReference(&vlsurface->device, NULL);
> FREE(vlsurface);
> return ret;
> }
> @@ -138,6 +139,7 @@ vlVdpBitmapSurfaceDestroy(VdpBitmapSurface surface)
> pipe_mutex_unlock(vlsurface->device->mutex);
>
> vlRemoveDataHTAB(surface);
> + DeviceReference(&vlsurface->device, NULL);
> FREE(vlsurface);
>
> return VDP_STATUS_OK;
> diff --git a/src/gallium/state_trackers/vdpau/decode.c b/src/gallium/state_trackers/vdpau/decode.c
> index 1e5f81e..767d311 100644
> --- a/src/gallium/state_trackers/vdpau/decode.c
> +++ b/src/gallium/state_trackers/vdpau/decode.c
> @@ -110,7 +110,7 @@ vlVdpDecoderCreate(VdpDevice device,
> return VDP_STATUS_RESOURCES;
> }
>
> - vldecoder->device = dev;
> + DeviceReference(&vldecoder->device, dev);
>
> templat.entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;
> templat.chroma_format = PIPE_VIDEO_CHROMA_FORMAT_420;
> @@ -141,6 +141,7 @@ error_handle:
>
> error_decoder:
> pipe_mutex_unlock(dev->mutex);
> + DeviceReference(&vldecoder->device, NULL);
> FREE(vldecoder);
> return ret;
> }
> @@ -163,6 +164,7 @@ vlVdpDecoderDestroy(VdpDecoder decoder)
> pipe_mutex_destroy(vldecoder->mutex);
>
> vlRemoveDataHTAB(decoder);
> + DeviceReference(&vldecoder->device, NULL);
> FREE(vldecoder);
>
> return VDP_STATUS_OK;
> diff --git a/src/gallium/state_trackers/vdpau/device.c b/src/gallium/state_trackers/vdpau/device.c
> index 0cdda73..9c5ec60 100644
> --- a/src/gallium/state_trackers/vdpau/device.c
> +++ b/src/gallium/state_trackers/vdpau/device.c
> @@ -59,6 +59,8 @@ vdp_imp_device_create_x11(Display *display, int screen, VdpDevice *device,
> goto no_dev;
> }
>
> + pipe_reference_init(&dev->reference, 1);
> +
> dev->vscreen = vl_screen_create(display, screen);
> if (!dev->vscreen) {
> ret = VDP_STATUS_RESOURCES;
> @@ -124,7 +126,7 @@ vlVdpPresentationQueueTargetCreateX11(VdpDevice device, Drawable drawable,
> if (!pqt)
> return VDP_STATUS_RESOURCES;
>
> - pqt->device = dev;
> + DeviceReference(&pqt->device, dev);
> pqt->drawable = drawable;
>
> *target = vlAddDataHTAB(pqt);
> @@ -153,6 +155,7 @@ vlVdpPresentationQueueTargetDestroy(VdpPresentationQueueTarget presentation_queu
> return VDP_STATUS_INVALID_HANDLE;
>
> vlRemoveDataHTAB(presentation_queue_target);
> + DeviceReference(&pqt->device, NULL);
> FREE(pqt);
>
> return VDP_STATUS_OK;
> @@ -168,16 +171,24 @@ vlVdpDeviceDestroy(VdpDevice device)
> if (!dev)
> return VDP_STATUS_INVALID_HANDLE;
>
> + vlRemoveDataHTAB(device);
> + DeviceReference(&dev, NULL);
> +
> + return VDP_STATUS_OK;
> +}
> +
> +/**
> + * Free a VdpDevice.
> + */
> +void
> +vlVdpDeviceFree(vlVdpDevice *dev)
> +{
> pipe_mutex_destroy(dev->mutex);
> vl_compositor_cleanup(&dev->compositor);
> dev->context->destroy(dev->context);
> vl_screen_destroy(dev->vscreen);
> -
> - vlRemoveDataHTAB(device);
> FREE(dev);
> vlDestroyHTAB();
> -
> - return VDP_STATUS_OK;
> }
>
> /**
> diff --git a/src/gallium/state_trackers/vdpau/mixer.c b/src/gallium/state_trackers/vdpau/mixer.c
> index e6bfb8c..a724aa5 100644
> --- a/src/gallium/state_trackers/vdpau/mixer.c
> +++ b/src/gallium/state_trackers/vdpau/mixer.c
> @@ -60,7 +60,7 @@ vlVdpVideoMixerCreate(VdpDevice device,
> if (!vmixer)
> return VDP_STATUS_RESOURCES;
>
> - vmixer->device = dev;
> + DeviceReference(&vmixer->device, dev);
>
> pipe_mutex_lock(dev->mutex);
>
> @@ -160,6 +160,7 @@ no_params:
> no_handle:
> vl_compositor_cleanup_state(&vmixer->cstate);
> pipe_mutex_unlock(dev->mutex);
> + DeviceReference(&vmixer->device, NULL);
> FREE(vmixer);
> return ret;
> }
> @@ -199,6 +200,7 @@ vlVdpVideoMixerDestroy(VdpVideoMixer mixer)
> FREE(vmixer->sharpness.filter);
> }
> pipe_mutex_unlock(vmixer->device->mutex);
> + DeviceReference(&vmixer->device, NULL);
>
> FREE(vmixer);
>
> diff --git a/src/gallium/state_trackers/vdpau/output.c b/src/gallium/state_trackers/vdpau/output.c
> index 457f678..caae50f 100644
> --- a/src/gallium/state_trackers/vdpau/output.c
> +++ b/src/gallium/state_trackers/vdpau/output.c
> @@ -69,7 +69,7 @@ vlVdpOutputSurfaceCreate(VdpDevice device,
> if (!vlsurface)
> return VDP_STATUS_RESOURCES;
>
> - vlsurface->device = dev;
> + DeviceReference(&vlsurface->device, dev);
>
> memset(&res_tmpl, 0, sizeof(res_tmpl));
>
> @@ -120,6 +120,7 @@ err_resource:
> pipe_resource_reference(&res, NULL);
> err_unlock:
> pipe_mutex_unlock(dev->mutex);
> + DeviceReference(&vlsurface->device, NULL);
> FREE(vlsurface);
> return VDP_STATUS_ERROR;
> }
> @@ -149,6 +150,7 @@ vlVdpOutputSurfaceDestroy(VdpOutputSurface surface)
> pipe_mutex_unlock(vlsurface->device->mutex);
>
> vlRemoveDataHTAB(surface);
> + DeviceReference(&vlsurface->device, NULL);
> FREE(vlsurface);
>
> return VDP_STATUS_OK;
> diff --git a/src/gallium/state_trackers/vdpau/presentation.c b/src/gallium/state_trackers/vdpau/presentation.c
> index cb6cb38..7f8dbed 100644
> --- a/src/gallium/state_trackers/vdpau/presentation.c
> +++ b/src/gallium/state_trackers/vdpau/presentation.c
> @@ -62,7 +62,7 @@ vlVdpPresentationQueueCreate(VdpDevice device,
> if (!pq)
> return VDP_STATUS_RESOURCES;
>
> - pq->device = dev;
> + DeviceReference(&pq->device, dev);
> pq->drawable = pqt->drawable;
>
> pipe_mutex_lock(dev->mutex);
> @@ -83,6 +83,7 @@ vlVdpPresentationQueueCreate(VdpDevice device,
>
> no_handle:
> no_compositor:
> + DeviceReference(&pq->device, NULL);
> FREE(pq);
> return ret;
> }
> @@ -104,6 +105,7 @@ vlVdpPresentationQueueDestroy(VdpPresentationQueue presentation_queue)
> pipe_mutex_unlock(pq->device->mutex);
>
> vlRemoveDataHTAB(presentation_queue);
> + DeviceReference(&pq->device, NULL);
> FREE(pq);
>
> return VDP_STATUS_OK;
> diff --git a/src/gallium/state_trackers/vdpau/surface.c b/src/gallium/state_trackers/vdpau/surface.c
> index 0d9f2b0..1932cdd 100644
> --- a/src/gallium/state_trackers/vdpau/surface.c
> +++ b/src/gallium/state_trackers/vdpau/surface.c
> @@ -74,7 +74,7 @@ vlVdpVideoSurfaceCreate(VdpDevice device, VdpChromaType chroma_type,
> goto inv_device;
> }
>
> - p_surf->device = dev;
> + DeviceReference(&p_surf->device, dev);
> pipe = dev->context;
>
> pipe_mutex_lock(dev->mutex);
> @@ -115,6 +115,7 @@ no_handle:
> p_surf->video_buffer->destroy(p_surf->video_buffer);
>
> inv_device:
> + DeviceReference(&p_surf->device, NULL);
> FREE(p_surf);
>
> no_res:
> @@ -140,6 +141,7 @@ vlVdpVideoSurfaceDestroy(VdpVideoSurface surface)
> pipe_mutex_unlock(p_surf->device->mutex);
>
> vlRemoveDataHTAB(surface);
> + DeviceReference(&p_surf->device, NULL);
> FREE(p_surf);
>
> return VDP_STATUS_OK;
> diff --git a/src/gallium/state_trackers/vdpau/vdpau_private.h b/src/gallium/state_trackers/vdpau/vdpau_private.h
> index ce6852b..65f8e47 100644
> --- a/src/gallium/state_trackers/vdpau/vdpau_private.h
> +++ b/src/gallium/state_trackers/vdpau/vdpau_private.h
> @@ -344,6 +344,7 @@ CheckSurfaceParams(struct pipe_screen *screen,
>
> typedef struct
> {
> + struct pipe_reference reference;
> struct vl_screen *vscreen;
> struct pipe_context *context;
> struct vl_compositor compositor;
> @@ -453,6 +454,7 @@ void vlVdpSave4DelayedRendering(vlVdpDevice *dev, VdpOutputSurface surface, stru
> /* Internal function pointers */
> VdpGetErrorString vlVdpGetErrorString;
> VdpDeviceDestroy vlVdpDeviceDestroy;
> +void vlVdpDeviceFree(vlVdpDevice *dev);
> VdpGetProcAddress vlVdpGetProcAddress;
> VdpGetApiVersion vlVdpGetApiVersion;
> VdpGetInformationString vlVdpGetInformationString;
> @@ -542,4 +544,14 @@ static inline void VDPAU_MSG(unsigned int level, const char *fmt, ...)
> }
> }
>
> +static inline void
> +DeviceReference(vlVdpDevice **ptr, vlVdpDevice *dev)
> +{
> + vlVdpDevice *old_dev = *ptr;
> +
> + if (pipe_reference(&(*ptr)->reference, &dev->reference))
> + vlVdpDeviceFree(old_dev);
> + *ptr = dev;
> +}
> +
> #endif /* VDPAU_PRIVATE_H */
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list