[vlc-devel] [PATCH 01/18] picture: add the video context in the picture context

Steve Lhomme robux4 at ycbcr.xyz
Tue Nov 19 10:27:00 CET 2019


👇

On 2019-11-19 10:17, Thomas Guillem wrote:
> 
> 
> On Mon, Nov 18, 2019, at 08:43, Steve Lhomme wrote:
>> The creator of the picture_context_t must set the video context if needed.
>> If set, the picture context must hold a reference to the video context.
>>
>> When "copying" a picture (using the same surface in another picture context) a
>> new reference has to be acquired.
>>
>> Add an inline funtion to get the video context from a picture.
>> ---
>>   include/vlc_picture.h                |  6 ++++++
>>   modules/codec/avcodec/d3d11va.c      |  1 +
>>   modules/codec/avcodec/dxva2.c        |  1 +
>>   modules/codec/avcodec/vaapi.c        |  1 +
>>   modules/hw/d3d11/d3d11_deinterlace.c |  1 +
>>   modules/hw/d3d11/d3d11_surface.c     |  1 +
>>   modules/hw/d3d9/dxa9.c               |  1 +
>>   modules/hw/d3d9/dxva2_deinterlace.c  |  1 +
>>   src/misc/picture.c                   | 11 +++++++++++
>>   9 files changed, 24 insertions(+)
>>
>> diff --git a/include/vlc_picture.h b/include/vlc_picture.h
>> index 3298630e02a..75a87c8a81a 100644
>> --- a/include/vlc_picture.h
>> +++ b/include/vlc_picture.h
>> @@ -69,6 +69,7 @@ typedef struct picture_context_t
>>   {
>>       void (*destroy)(struct picture_context_t *);
>>       struct picture_context_t *(*copy)(struct picture_context_t *);
>> +    struct vlc_video_context *vctx;
>>   } picture_context_t;
>>   
>>   typedef struct picture_buffer_t
>> @@ -161,6 +162,11 @@ struct picture_t
>>       atomic_uintptr_t refs;
>>   };
>>   
>> +static inline vlc_video_context* picture_GetVideoContext(picture_t *pic)
>> +{
>> +    return pic->context ? pic->context->vctx : NULL;
>> +}
>> +
>>   /**
>>    * This function will create a new picture.
>>    * The picture created will implement a default release management compatible
>> diff --git a/modules/codec/avcodec/d3d11va.c b/modules/codec/avcodec/d3d11va.c
>> index a15830c7f29..174e2d0d40c 100644
>> --- a/modules/codec/avcodec/d3d11va.c
>> +++ b/modules/codec/avcodec/d3d11va.c
>> @@ -177,6 +177,7 @@ static struct d3d11va_pic_context *CreatePicContext(
>>           return NULL;
>>       pic_ctx->ctx.s = (picture_context_t) {
>>           d3d11va_pic_context_destroy, d3d11va_pic_context_copy,
>> +        NULL /*TODO*/
>>       };
>>   
>>       D3D11_TEXTURE2D_DESC txDesc;
>> diff --git a/modules/codec/avcodec/dxva2.c
>> b/modules/codec/avcodec/dxva2.c
>> index 3caa648a315..62b14476e43 100644
>> --- a/modules/codec/avcodec/dxva2.c
>> +++ b/modules/codec/avcodec/dxva2.c
>> @@ -190,6 +190,7 @@ static struct dxva2_pic_context
>> *CreatePicContext(IDirect3DSurface9 *surface)
>>           return NULL;
>>       pic_ctx->ctx.s = (picture_context_t) {
>>           dxva2_pic_context_destroy, dxva2_pic_context_copy,
>> +        NULL /*TODO*/
>>       };
>>       pic_ctx->ctx.picsys.surface = surface;
>>       AcquireD3D9PictureSys(&pic_ctx->ctx.picsys);
>> diff --git a/modules/codec/avcodec/vaapi.c
>> b/modules/codec/avcodec/vaapi.c
>> index daec9b4b4e7..110a6492d2d 100644
>> --- a/modules/codec/avcodec/vaapi.c
>> +++ b/modules/codec/avcodec/vaapi.c
>> @@ -163,6 +163,7 @@ static int Get(vlc_va_t *va, picture_t *pic,
>> uint8_t **data)
>>       }
>>       vaapi_ctx->ctx.s = (picture_context_t) {
>>           vaapi_dec_pic_context_destroy, vaapi_dec_pic_context_copy,
>> +        NULL /*TODO*/
>>       };
>>       vaapi_ctx->ctx.surface =
>> sys->render_targets[va_surface_GetIndex(va_surface)];
>>       vaapi_ctx->ctx.va_dpy = sys->hw_ctx.display;
>> diff --git a/modules/hw/d3d11/d3d11_deinterlace.c
>> b/modules/hw/d3d11/d3d11_deinterlace.c
>> index 90215efbd70..5c58632e59e 100644
>> --- a/modules/hw/d3d11/d3d11_deinterlace.c
>> +++ b/modules/hw/d3d11/d3d11_deinterlace.c
>> @@ -266,6 +266,7 @@ picture_t *AllocPicture( filter_t *p_filter )
>>           {
>>               pic_ctx->s = (picture_context_t) {
>>                   d3d11_pic_context_destroy, d3d11_pic_context_copy,
>> +                NULL /*TODO*/
>>               };
>>               pic_ctx->picsys = *pic_sys;
>>               AcquireD3D11PictureSys( &pic_ctx->picsys );
>> diff --git a/modules/hw/d3d11/d3d11_surface.c
>> b/modules/hw/d3d11/d3d11_surface.c
>> index 3989a0a37bf..93ce6377a27 100644
>> --- a/modules/hw/d3d11/d3d11_surface.c
>> +++ b/modules/hw/d3d11/d3d11_surface.c
>> @@ -578,6 +578,7 @@ static void NV12_D3D11(filter_t *p_filter,
>> picture_t *src, picture_t *dst)
>>           {
>>               pic_ctx->s = (picture_context_t) {
>>                   d3d11_pic_context_destroy, d3d11_pic_context_copy,
>> +                NULL /*TODO*/
>>               };
>>               pic_ctx->picsys = *p_sys;
>>               AcquireD3D11PictureSys(&pic_ctx->picsys);
>> diff --git a/modules/hw/d3d9/dxa9.c b/modules/hw/d3d9/dxa9.c
>> index 0f5700b9e87..4f1ea9708bb 100644
>> --- a/modules/hw/d3d9/dxa9.c
>> +++ b/modules/hw/d3d9/dxa9.c
>> @@ -265,6 +265,7 @@ static void YV12_D3D9(filter_t *p_filter, picture_t
>> *src, picture_t *dst)
>>           {
>>               pic_ctx->s = (picture_context_t) {
>>                   d3d9_pic_context_destroy, d3d9_pic_context_copy,
>> +                NULL /*TODO*/
>>               };
>>               pic_ctx->picsys = *p_sys;
>>               AcquireD3D9PictureSys(&pic_ctx->picsys);
>> diff --git a/modules/hw/d3d9/dxva2_deinterlace.c
>> b/modules/hw/d3d9/dxva2_deinterlace.c
>> index 2564752818e..48b59a23369 100644
>> --- a/modules/hw/d3d9/dxva2_deinterlace.c
>> +++ b/modules/hw/d3d9/dxva2_deinterlace.c
>> @@ -326,6 +326,7 @@ picture_t *AllocPicture( filter_t *p_filter )
>>           {
>>               pic_ctx->s = (picture_context_t) {
>>                   d3d9_pic_context_destroy, d3d9_pic_context_copy,
>> +                NULL /*TODO*/
>>               };
>>               pic_ctx->picsys = *pic_sys;
>>               AcquireD3D9PictureSys( &pic_ctx->picsys );
>> diff --git a/src/misc/picture.c b/src/misc/picture.c
>> index cdde6f9951a..c128c804055 100644
>> --- a/src/misc/picture.c
>> +++ b/src/misc/picture.c
>> @@ -43,7 +43,10 @@ static void PictureDestroyContext( picture_t
>> *p_picture )
>>       picture_context_t *ctx = p_picture->context;
>>       if (ctx != NULL)
>>       {
>> +        vlc_video_context *vctx = ctx->vctx;
>>           ctx->destroy(ctx);
>> +        if (vctx)
>> +            vlc_video_context_Release(vctx);
>>           p_picture->context = NULL;
>>       }
>>   }
>> @@ -389,7 +392,11 @@ void picture_CopyPixels( picture_t *p_dst, const
>> picture_t *p_src )
>>       assert( p_dst->context == NULL );
>>   
>>       if( p_src->context != NULL )
>> +    {
>>           p_dst->context = p_src->context->copy( p_src->context );
>> +        if ( p_dst->context != NULL && p_dst->context->vctx != NULL )
>> +            vlc_video_context_Hold( p_dst->context->vctx );
> 
> I  think the copy implementation should take care of holding the video context.
> It feels weird that the copy implementation copy the vctx pointer without holding it.

This is a safety. It should actually assert that if a source has a video 
context, the output should also have one. In many cases the whole 
picture context structure is copied, including the video context 
pointer. Then the caller can just add the reference if the copy/clone 
worked.

It might be an issue if for some reason the copy decides to create a new 
video context instance. It could be used in the future even if the 
underlying data are similar. (that's also a reason why we would need 
video_context_IsSimilar)

So it might be better to move this in each copy/clone implementation.

>> +    }
>>   }
>>   
>>   void picture_Copy( picture_t *p_dst, const picture_t *p_src )
>> @@ -425,7 +432,11 @@ picture_t *picture_Clone(picture_t *picture)
>>           picture_Hold(picture);
>>   
>>           if (picture->context != NULL)
>> +        {
>>               clone->context = picture->context->copy(picture->context);
>> +            if ( clone->context != NULL && clone->context->vctx != NULL )
>> +                vlc_video_context_Hold( clone->context->vctx );
> 
> dito.
> 
>> +        }
>>       }
>>       return clone;
>>   }
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> 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
> 


More information about the vlc-devel mailing list