[FFmpeg-devel] [PATCH] hwcontext_d3d11va: properly reset values after release/close

2017-11-23 Thread Jan Ekström 
Makes the uninit function re-entrable, which can be a common case
when an API user first tries to initialize its context, fails, and
then finally unrefs the AVHWDevice.

Fixes a crash reported by sm2345 on IRC.
---
 libavutil/hwcontext_d3d11va.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/libavutil/hwcontext_d3d11va.c b/libavutil/hwcontext_d3d11va.c
index 65dd6651fc..845a4a45fe 100644
--- a/libavutil/hwcontext_d3d11va.c
+++ b/libavutil/hwcontext_d3d11va.c
@@ -458,20 +458,31 @@ static void d3d11va_device_uninit(AVHWDeviceContext 
*hwdev)
 {
 AVD3D11VADeviceContext *device_hwctx = hwdev->hwctx;
 
-if (device_hwctx->device)
+if (device_hwctx->device) {
 ID3D11Device_Release(device_hwctx->device);
+device_hwctx->device = NULL;
+}
 
-if (device_hwctx->device_context)
+if (device_hwctx->device_context) {
 ID3D11DeviceContext_Release(device_hwctx->device_context);
+device_hwctx->device_context = NULL;
+}
 
-if (device_hwctx->video_device)
+if (device_hwctx->video_device) {
 ID3D11VideoDevice_Release(device_hwctx->video_device);
+device_hwctx->video_device = NULL;
+}
 
-if (device_hwctx->video_context)
+if (device_hwctx->video_context) {
 ID3D11VideoContext_Release(device_hwctx->video_context);
+device_hwctx->video_context = NULL;
+}
 
-if (device_hwctx->lock == d3d11va_default_lock)
+if (device_hwctx->lock == d3d11va_default_lock) {
 CloseHandle(device_hwctx->lock_ctx);
+device_hwctx->lock_ctx = INVALID_HANDLE_VALUE;
+device_hwctx->lock = NULL;
+}
 }
 
 static int d3d11va_device_create(AVHWDeviceContext *ctx, const char *device,
-- 
2.14.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] hwcontext_d3d11va: properly reset values after release/close

2017-11-24 Thread Jan Ekstrom 
On Nov 24, 2017 03:01, "Jan Ekström"  wrote:

Makes the uninit function re-entrable, which can be a common case
when an API user first tries to initialize its context, fails, and
then finally unrefs the AVHWDevice.

Fixes a crash reported by sm2345 on IRC


Relevant backtrace I received from the user:

https://pastebin.com/MtWTEgmc

Regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] hwcontext_d3d11va: properly reset values after release/close

2017-11-24 Thread Hendrik Leppkes 
On Fri, Nov 24, 2017 at 2:01 AM, Jan Ekström  wrote:
> Makes the uninit function re-entrable, which can be a common case
> when an API user first tries to initialize its context, fails, and
> then finally unrefs the AVHWDevice.
>
> Fixes a crash reported by sm2345 on IRC.
> ---
>  libavutil/hwcontext_d3d11va.c | 21 -
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/libavutil/hwcontext_d3d11va.c b/libavutil/hwcontext_d3d11va.c
> index 65dd6651fc..845a4a45fe 100644
> --- a/libavutil/hwcontext_d3d11va.c
> +++ b/libavutil/hwcontext_d3d11va.c
> @@ -458,20 +458,31 @@ static void d3d11va_device_uninit(AVHWDeviceContext 
> *hwdev)
>  {
>  AVD3D11VADeviceContext *device_hwctx = hwdev->hwctx;
>
> -if (device_hwctx->device)
> +if (device_hwctx->device) {
>  ID3D11Device_Release(device_hwctx->device);
> +device_hwctx->device = NULL;
> +}
>
> -if (device_hwctx->device_context)
> +if (device_hwctx->device_context) {
>  ID3D11DeviceContext_Release(device_hwctx->device_context);
> +device_hwctx->device_context = NULL;
> +}
>
> -if (device_hwctx->video_device)
> +if (device_hwctx->video_device) {
>  ID3D11VideoDevice_Release(device_hwctx->video_device);
> +device_hwctx->video_device = NULL;
> +}
>
> -if (device_hwctx->video_context)
> +if (device_hwctx->video_context) {
>  ID3D11VideoContext_Release(device_hwctx->video_context);
> +device_hwctx->video_context = NULL;
> +}
>
> -if (device_hwctx->lock == d3d11va_default_lock)
> +if (device_hwctx->lock == d3d11va_default_lock) {
>  CloseHandle(device_hwctx->lock_ctx);
> +device_hwctx->lock_ctx = INVALID_HANDLE_VALUE;
> +device_hwctx->lock = NULL;
> +}
>  }
>

LGTM. In some cases the uninit can be called twice, which seems like a
bit of an odd design, but we better don't leak any pointers out then.
Should probably go through the other hwcontext implementation to check
for similar problems.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] hwcontext_d3d11va: properly reset values after release/close

2017-11-25 Thread Mark Thompson 
On 24/11/17 01:01, Jan Ekström wrote:
> Makes the uninit function re-entrable, which can be a common case
> when an API user first tries to initialize its context, fails, and
> then finally unrefs the AVHWDevice.
> 
> Fixes a crash reported by sm2345 on IRC.
> ---
>  libavutil/hwcontext_d3d11va.c | 21 -
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/libavutil/hwcontext_d3d11va.c b/libavutil/hwcontext_d3d11va.c
> index 65dd6651fc..845a4a45fe 100644
> --- a/libavutil/hwcontext_d3d11va.c
> +++ b/libavutil/hwcontext_d3d11va.c
> @@ -458,20 +458,31 @@ static void d3d11va_device_uninit(AVHWDeviceContext 
> *hwdev)
>  {
>  AVD3D11VADeviceContext *device_hwctx = hwdev->hwctx;
>  
> -if (device_hwctx->device)
> +if (device_hwctx->device) {
>  ID3D11Device_Release(device_hwctx->device);
> +device_hwctx->device = NULL;
> +}
>  
> -if (device_hwctx->device_context)
> +if (device_hwctx->device_context) {
>  ID3D11DeviceContext_Release(device_hwctx->device_context);
> +device_hwctx->device_context = NULL;
> +}
>  
> -if (device_hwctx->video_device)
> +if (device_hwctx->video_device) {
>  ID3D11VideoDevice_Release(device_hwctx->video_device);
> +device_hwctx->video_device = NULL;
> +}
>  
> -if (device_hwctx->video_context)
> +if (device_hwctx->video_context) {
>  ID3D11VideoContext_Release(device_hwctx->video_context);
> +device_hwctx->video_context = NULL;
> +}
>  
> -if (device_hwctx->lock == d3d11va_default_lock)
> +if (device_hwctx->lock == d3d11va_default_lock) {
>  CloseHandle(device_hwctx->lock_ctx);
> +device_hwctx->lock_ctx = INVALID_HANDLE_VALUE;
> +device_hwctx->lock = NULL;
> +}
>  }
>  
>  static int d3d11va_device_create(AVHWDeviceContext *ctx, const char *device,
> 

LGTM, please apply.

I checked the other hwcontext implementations for the same problem, and found 
it only in OpenCL - it could also crash there, fixed in 
.

Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] hwcontext_d3d11va: properly reset values after release/close

2017-11-25 Thread Jan Ekström 
On Sat, Nov 25, 2017 at 6:25 PM, Mark Thompson  wrote:
> LGTM, please apply.
>
> I checked the other hwcontext implementations for the same problem, and found 
> it only in OpenCL - it could also crash there, fixed in 
> .
>
> Thanks,
>
> - Mark

Thanks for the reviews, applied.

Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel