Re: [FFmpeg-devel] [PATCH 4/4] vaapi: add interfaces to properly initialize context.

2015-08-18 Thread wm4
On Tue, 18 Aug 2015 10:33:20 +0200
Gwenole Beauchesne  wrote:

> Hi,
> 
> 2015-08-17 22:26 GMT+02:00 wm4 :
> > On Mon, 17 Aug 2015 19:17:53 +0200
> > Gwenole Beauchesne  wrote:
> >
> >> Add av_vaapi_context_init() and av_vaapi_context_alloc() helper functions
> >> so that to properly initialize the vaapi_context structure, and allow for
> >> future extensions without breaking the API/ABI.
> >>
> >> The new version and flags fields are inserted after the last known user
> >> initialized field, and actually useful field at all, i.e. VA context_id.
> >> This is safe because the vaapi_context structure was required to be zero
> >> initialized in the past. And, since those new helper functions and changes
> >> cause the AV_VAAPI_CONTEXT_VERSION to be bumped, we can track older struct
> >> requirements from within libavcodec.
> >>
> >> Besides, it is now required by design, and actual usage, that the vaapi
> >> context structure be completely initialized once and for all before the
> >> AVCodecContext.get_format() function ever completes.
> >>
> >> Signed-off-by: Gwenole Beauchesne 
> >> ---
> >>  libavcodec/vaapi.c  | 30 +++
> >>  libavcodec/vaapi.h  | 59 
> >> +
> >>  libavcodec/vaapi_internal.h |  1 +
> >>  3 files changed, 85 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/libavcodec/vaapi.c b/libavcodec/vaapi.c
> >> index 1ae71a5..4d3e2f3 100644
> >> --- a/libavcodec/vaapi.c
> >> +++ b/libavcodec/vaapi.c
> >> @@ -41,11 +41,41 @@ static void destroy_buffers(VADisplay display, 
> >> VABufferID *buffers, unsigned int
> >>  }
> >>  }
> >>
> >> +/* Allocates a vaapi_context structure */
> >> +struct vaapi_context *av_vaapi_context_alloc(unsigned int flags)
> >> +{
> >> +struct vaapi_context *vactx;
> >> +
> >> +vactx = av_malloc(sizeof(*vactx));
> >> +if (!vactx)
> >> +return NULL;
> >> +
> >> +av_vaapi_context_init(vactx, AV_VAAPI_CONTEXT_VERSION, flags);
> >> +return vactx;
> >> +}
> >> +
> >> +/* Initializes a vaapi_context structure with safe defaults */
> >> +void av_vaapi_context_init(struct vaapi_context *vactx, unsigned int 
> >> version,
> >> +   unsigned int flags)
> >> +{
> >> +vactx->display  = NULL;
> >> +vactx->config_id= VA_INVALID_ID;
> >> +vactx->context_id   = VA_INVALID_ID;
> >> +
> >> +if (version > 0) {
> >> +vactx->version  = version;
> >> +vactx->flags= flags;
> >> +}
> >> +}
> >> +
> >>  int ff_vaapi_context_init(AVCodecContext *avctx)
> >>  {
> >>  FFVAContext * const vactx = ff_vaapi_get_context(avctx);
> >>  const struct vaapi_context * const user_vactx = 
> >> avctx->hwaccel_context;
> >>
> >> +if (user_vactx->version > 0)
> >> +vactx->flags= user_vactx->flags;
> >> +
> >>  vactx->display  = user_vactx->display;
> >>  vactx->config_id= user_vactx->config_id;
> >>  vactx->context_id   = user_vactx->context_id;
> >> diff --git a/libavcodec/vaapi.h b/libavcodec/vaapi.h
> >> index 4448a2e..1f032a0 100644
> >> --- a/libavcodec/vaapi.h
> >> +++ b/libavcodec/vaapi.h
> >> @@ -40,14 +40,16 @@
> >>   * @{
> >>   */
> >>
> >> +#define AV_VAAPI_CONTEXT_VERSION 1
> >> +
> >>  /**
> >>   * This structure is used to share data between the FFmpeg library and
> >>   * the client video application.
> >> - * This shall be zero-allocated and available as
> >> - * AVCodecContext.hwaccel_context. All user members can be set once
> >> - * during initialization or through each AVCodecContext.get_buffer()
> >> - * function call. In any case, they must be valid prior to calling
> >> - * decoding functions.
> >> + *
> >> + * This shall be initialized with av_vaapi_context_init(), or
> >> + * indirectly through av_vaapi_context_alloc(), and made available as
> >> + * AVCodecContext.hwaccel_context. All user members must be properly
> >> + * initialized before AVCodecContext.get_format() completes.
> >>   */
> >>  struct vaapi_context {
> >>  /**
> >> @@ -74,6 +76,29 @@ struct vaapi_context {
> >>   */
> >>  uint32_t context_id;
> >>
> >> +/**
> >> + * This field must be set to AV_VAAPI_CONTEXT_VERSION
> >> + *
> >> + * @since Version 1.
> >> + *
> >> + * - encoding: unused
> >> + * - decoding: Set by user, through av_vaapi_context_init()
> >> + */
> >> +unsigned int version;
> >
> > Not sure if I see any point in this. Normally, backwards-compatible ABI
> > additions can add fields in FFmpeg, but the API user doesn't get a way
> > to check whether a field is really there.
> >
> > Or in other words, the level of ABI compatibility we target is that
> > newer libav* libs work with old application binaries, but not the other
> > way around.
> 
> Yes, and I have reasons to see why this mechanism is needed.
> 
> Imagine some old application binaries doesn't allocate the context
> through the supplied helper fun

Re: [FFmpeg-devel] [PATCH 4/4] vaapi: add interfaces to properly initialize context.

2015-08-18 Thread Hendrik Leppkes
On Tue, Aug 18, 2015 at 10:33 AM, Gwenole Beauchesne  wrote:
> Hi,
>
> 2015-08-17 22:26 GMT+02:00 wm4 :
>> On Mon, 17 Aug 2015 19:17:53 +0200
>> Gwenole Beauchesne  wrote:
>>
>>> Add av_vaapi_context_init() and av_vaapi_context_alloc() helper functions
>>> so that to properly initialize the vaapi_context structure, and allow for
>>> future extensions without breaking the API/ABI.
>>>
>>> The new version and flags fields are inserted after the last known user
>>> initialized field, and actually useful field at all, i.e. VA context_id.
>>> This is safe because the vaapi_context structure was required to be zero
>>> initialized in the past. And, since those new helper functions and changes
>>> cause the AV_VAAPI_CONTEXT_VERSION to be bumped, we can track older struct
>>> requirements from within libavcodec.
>>>
>>> Besides, it is now required by design, and actual usage, that the vaapi
>>> context structure be completely initialized once and for all before the
>>> AVCodecContext.get_format() function ever completes.
>>>
>>> Signed-off-by: Gwenole Beauchesne 
>>> ---
>>>  libavcodec/vaapi.c  | 30 +++
>>>  libavcodec/vaapi.h  | 59 
>>> +
>>>  libavcodec/vaapi_internal.h |  1 +
>>>  3 files changed, 85 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libavcodec/vaapi.c b/libavcodec/vaapi.c
>>> index 1ae71a5..4d3e2f3 100644
>>> --- a/libavcodec/vaapi.c
>>> +++ b/libavcodec/vaapi.c
>>> @@ -41,11 +41,41 @@ static void destroy_buffers(VADisplay display, 
>>> VABufferID *buffers, unsigned int
>>>  }
>>>  }
>>>
>>> +/* Allocates a vaapi_context structure */
>>> +struct vaapi_context *av_vaapi_context_alloc(unsigned int flags)
>>> +{
>>> +struct vaapi_context *vactx;
>>> +
>>> +vactx = av_malloc(sizeof(*vactx));
>>> +if (!vactx)
>>> +return NULL;
>>> +
>>> +av_vaapi_context_init(vactx, AV_VAAPI_CONTEXT_VERSION, flags);
>>> +return vactx;
>>> +}
>>> +
>>> +/* Initializes a vaapi_context structure with safe defaults */
>>> +void av_vaapi_context_init(struct vaapi_context *vactx, unsigned int 
>>> version,
>>> +   unsigned int flags)
>>> +{
>>> +vactx->display  = NULL;
>>> +vactx->config_id= VA_INVALID_ID;
>>> +vactx->context_id   = VA_INVALID_ID;
>>> +
>>> +if (version > 0) {
>>> +vactx->version  = version;
>>> +vactx->flags= flags;
>>> +}
>>> +}
>>> +
>>>  int ff_vaapi_context_init(AVCodecContext *avctx)
>>>  {
>>>  FFVAContext * const vactx = ff_vaapi_get_context(avctx);
>>>  const struct vaapi_context * const user_vactx = avctx->hwaccel_context;
>>>
>>> +if (user_vactx->version > 0)
>>> +vactx->flags= user_vactx->flags;
>>> +
>>>  vactx->display  = user_vactx->display;
>>>  vactx->config_id= user_vactx->config_id;
>>>  vactx->context_id   = user_vactx->context_id;
>>> diff --git a/libavcodec/vaapi.h b/libavcodec/vaapi.h
>>> index 4448a2e..1f032a0 100644
>>> --- a/libavcodec/vaapi.h
>>> +++ b/libavcodec/vaapi.h
>>> @@ -40,14 +40,16 @@
>>>   * @{
>>>   */
>>>
>>> +#define AV_VAAPI_CONTEXT_VERSION 1
>>> +
>>>  /**
>>>   * This structure is used to share data between the FFmpeg library and
>>>   * the client video application.
>>> - * This shall be zero-allocated and available as
>>> - * AVCodecContext.hwaccel_context. All user members can be set once
>>> - * during initialization or through each AVCodecContext.get_buffer()
>>> - * function call. In any case, they must be valid prior to calling
>>> - * decoding functions.
>>> + *
>>> + * This shall be initialized with av_vaapi_context_init(), or
>>> + * indirectly through av_vaapi_context_alloc(), and made available as
>>> + * AVCodecContext.hwaccel_context. All user members must be properly
>>> + * initialized before AVCodecContext.get_format() completes.
>>>   */
>>>  struct vaapi_context {
>>>  /**
>>> @@ -74,6 +76,29 @@ struct vaapi_context {
>>>   */
>>>  uint32_t context_id;
>>>
>>> +/**
>>> + * This field must be set to AV_VAAPI_CONTEXT_VERSION
>>> + *
>>> + * @since Version 1.
>>> + *
>>> + * - encoding: unused
>>> + * - decoding: Set by user, through av_vaapi_context_init()
>>> + */
>>> +unsigned int version;
>>
>> Not sure if I see any point in this. Normally, backwards-compatible ABI
>> additions can add fields in FFmpeg, but the API user doesn't get a way
>> to check whether a field is really there.
>>
>> Or in other words, the level of ABI compatibility we target is that
>> newer libav* libs work with old application binaries, but not the other
>> way around.
>
> Yes, and I have reasons to see why this mechanism is needed.
>
> Imagine some old application binaries doesn't allocate the context
> through the supplied helper functions. Without knowing the original
> layout of the user supplied vaapi_context, we could be in a situation
> where we read past th

Re: [FFmpeg-devel] [PATCH 4/4] vaapi: add interfaces to properly initialize context.

2015-08-18 Thread Gwenole Beauchesne
Hi,

2015-08-17 22:26 GMT+02:00 wm4 :
> On Mon, 17 Aug 2015 19:17:53 +0200
> Gwenole Beauchesne  wrote:
>
>> Add av_vaapi_context_init() and av_vaapi_context_alloc() helper functions
>> so that to properly initialize the vaapi_context structure, and allow for
>> future extensions without breaking the API/ABI.
>>
>> The new version and flags fields are inserted after the last known user
>> initialized field, and actually useful field at all, i.e. VA context_id.
>> This is safe because the vaapi_context structure was required to be zero
>> initialized in the past. And, since those new helper functions and changes
>> cause the AV_VAAPI_CONTEXT_VERSION to be bumped, we can track older struct
>> requirements from within libavcodec.
>>
>> Besides, it is now required by design, and actual usage, that the vaapi
>> context structure be completely initialized once and for all before the
>> AVCodecContext.get_format() function ever completes.
>>
>> Signed-off-by: Gwenole Beauchesne 
>> ---
>>  libavcodec/vaapi.c  | 30 +++
>>  libavcodec/vaapi.h  | 59 
>> +
>>  libavcodec/vaapi_internal.h |  1 +
>>  3 files changed, 85 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavcodec/vaapi.c b/libavcodec/vaapi.c
>> index 1ae71a5..4d3e2f3 100644
>> --- a/libavcodec/vaapi.c
>> +++ b/libavcodec/vaapi.c
>> @@ -41,11 +41,41 @@ static void destroy_buffers(VADisplay display, 
>> VABufferID *buffers, unsigned int
>>  }
>>  }
>>
>> +/* Allocates a vaapi_context structure */
>> +struct vaapi_context *av_vaapi_context_alloc(unsigned int flags)
>> +{
>> +struct vaapi_context *vactx;
>> +
>> +vactx = av_malloc(sizeof(*vactx));
>> +if (!vactx)
>> +return NULL;
>> +
>> +av_vaapi_context_init(vactx, AV_VAAPI_CONTEXT_VERSION, flags);
>> +return vactx;
>> +}
>> +
>> +/* Initializes a vaapi_context structure with safe defaults */
>> +void av_vaapi_context_init(struct vaapi_context *vactx, unsigned int 
>> version,
>> +   unsigned int flags)
>> +{
>> +vactx->display  = NULL;
>> +vactx->config_id= VA_INVALID_ID;
>> +vactx->context_id   = VA_INVALID_ID;
>> +
>> +if (version > 0) {
>> +vactx->version  = version;
>> +vactx->flags= flags;
>> +}
>> +}
>> +
>>  int ff_vaapi_context_init(AVCodecContext *avctx)
>>  {
>>  FFVAContext * const vactx = ff_vaapi_get_context(avctx);
>>  const struct vaapi_context * const user_vactx = avctx->hwaccel_context;
>>
>> +if (user_vactx->version > 0)
>> +vactx->flags= user_vactx->flags;
>> +
>>  vactx->display  = user_vactx->display;
>>  vactx->config_id= user_vactx->config_id;
>>  vactx->context_id   = user_vactx->context_id;
>> diff --git a/libavcodec/vaapi.h b/libavcodec/vaapi.h
>> index 4448a2e..1f032a0 100644
>> --- a/libavcodec/vaapi.h
>> +++ b/libavcodec/vaapi.h
>> @@ -40,14 +40,16 @@
>>   * @{
>>   */
>>
>> +#define AV_VAAPI_CONTEXT_VERSION 1
>> +
>>  /**
>>   * This structure is used to share data between the FFmpeg library and
>>   * the client video application.
>> - * This shall be zero-allocated and available as
>> - * AVCodecContext.hwaccel_context. All user members can be set once
>> - * during initialization or through each AVCodecContext.get_buffer()
>> - * function call. In any case, they must be valid prior to calling
>> - * decoding functions.
>> + *
>> + * This shall be initialized with av_vaapi_context_init(), or
>> + * indirectly through av_vaapi_context_alloc(), and made available as
>> + * AVCodecContext.hwaccel_context. All user members must be properly
>> + * initialized before AVCodecContext.get_format() completes.
>>   */
>>  struct vaapi_context {
>>  /**
>> @@ -74,6 +76,29 @@ struct vaapi_context {
>>   */
>>  uint32_t context_id;
>>
>> +/**
>> + * This field must be set to AV_VAAPI_CONTEXT_VERSION
>> + *
>> + * @since Version 1.
>> + *
>> + * - encoding: unused
>> + * - decoding: Set by user, through av_vaapi_context_init()
>> + */
>> +unsigned int version;
>
> Not sure if I see any point in this. Normally, backwards-compatible ABI
> additions can add fields in FFmpeg, but the API user doesn't get a way
> to check whether a field is really there.
>
> Or in other words, the level of ABI compatibility we target is that
> newer libav* libs work with old application binaries, but not the other
> way around.

Yes, and I have reasons to see why this mechanism is needed.

Imagine some old application binaries doesn't allocate the context
through the supplied helper functions. Without knowing the original
layout of the user supplied vaapi_context, we could be in a situation
where we read past the end of the struct. The current patch series
doesn't present it, but future ones can enlarge the public
vaapi_context with various other needed fields for configuration.
Aside of "hwaccel v2 plans",

Re: [FFmpeg-devel] [PATCH 4/4] vaapi: add interfaces to properly initialize context.

2015-08-17 Thread wm4
On Mon, 17 Aug 2015 19:17:53 +0200
Gwenole Beauchesne  wrote:

> Add av_vaapi_context_init() and av_vaapi_context_alloc() helper functions
> so that to properly initialize the vaapi_context structure, and allow for
> future extensions without breaking the API/ABI.
> 
> The new version and flags fields are inserted after the last known user
> initialized field, and actually useful field at all, i.e. VA context_id.
> This is safe because the vaapi_context structure was required to be zero
> initialized in the past. And, since those new helper functions and changes
> cause the AV_VAAPI_CONTEXT_VERSION to be bumped, we can track older struct
> requirements from within libavcodec.
> 
> Besides, it is now required by design, and actual usage, that the vaapi
> context structure be completely initialized once and for all before the
> AVCodecContext.get_format() function ever completes.
> 
> Signed-off-by: Gwenole Beauchesne 
> ---
>  libavcodec/vaapi.c  | 30 +++
>  libavcodec/vaapi.h  | 59 
> +
>  libavcodec/vaapi_internal.h |  1 +
>  3 files changed, 85 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/vaapi.c b/libavcodec/vaapi.c
> index 1ae71a5..4d3e2f3 100644
> --- a/libavcodec/vaapi.c
> +++ b/libavcodec/vaapi.c
> @@ -41,11 +41,41 @@ static void destroy_buffers(VADisplay display, VABufferID 
> *buffers, unsigned int
>  }
>  }
>  
> +/* Allocates a vaapi_context structure */
> +struct vaapi_context *av_vaapi_context_alloc(unsigned int flags)
> +{
> +struct vaapi_context *vactx;
> +
> +vactx = av_malloc(sizeof(*vactx));
> +if (!vactx)
> +return NULL;
> +
> +av_vaapi_context_init(vactx, AV_VAAPI_CONTEXT_VERSION, flags);
> +return vactx;
> +}
> +
> +/* Initializes a vaapi_context structure with safe defaults */
> +void av_vaapi_context_init(struct vaapi_context *vactx, unsigned int version,
> +   unsigned int flags)
> +{
> +vactx->display  = NULL;
> +vactx->config_id= VA_INVALID_ID;
> +vactx->context_id   = VA_INVALID_ID;
> +
> +if (version > 0) {
> +vactx->version  = version;
> +vactx->flags= flags;
> +}
> +}
> +
>  int ff_vaapi_context_init(AVCodecContext *avctx)
>  {
>  FFVAContext * const vactx = ff_vaapi_get_context(avctx);
>  const struct vaapi_context * const user_vactx = avctx->hwaccel_context;
>  
> +if (user_vactx->version > 0)
> +vactx->flags= user_vactx->flags;
> +
>  vactx->display  = user_vactx->display;
>  vactx->config_id= user_vactx->config_id;
>  vactx->context_id   = user_vactx->context_id;
> diff --git a/libavcodec/vaapi.h b/libavcodec/vaapi.h
> index 4448a2e..1f032a0 100644
> --- a/libavcodec/vaapi.h
> +++ b/libavcodec/vaapi.h
> @@ -40,14 +40,16 @@
>   * @{
>   */
>  
> +#define AV_VAAPI_CONTEXT_VERSION 1
> +
>  /**
>   * This structure is used to share data between the FFmpeg library and
>   * the client video application.
> - * This shall be zero-allocated and available as
> - * AVCodecContext.hwaccel_context. All user members can be set once
> - * during initialization or through each AVCodecContext.get_buffer()
> - * function call. In any case, they must be valid prior to calling
> - * decoding functions.
> + *
> + * This shall be initialized with av_vaapi_context_init(), or
> + * indirectly through av_vaapi_context_alloc(), and made available as
> + * AVCodecContext.hwaccel_context. All user members must be properly
> + * initialized before AVCodecContext.get_format() completes.
>   */
>  struct vaapi_context {
>  /**
> @@ -74,6 +76,29 @@ struct vaapi_context {
>   */
>  uint32_t context_id;
>  
> +/**
> + * This field must be set to AV_VAAPI_CONTEXT_VERSION
> + *
> + * @since Version 1.
> + *
> + * - encoding: unused
> + * - decoding: Set by user, through av_vaapi_context_init()
> + */
> +unsigned int version;

Not sure if I see any point in this. Normally, backwards-compatible ABI
additions can add fields in FFmpeg, but the API user doesn't get a way
to check whether a field is really there.

Or in other words, the level of ABI compatibility we target is that
newer libav* libs work with old application binaries, but not the other
way around.

> +
> +/**
> + * A bit field configuring the internal context used by libavcodec
> + *
> + * This is a combination of flags from common AV_HWACCEL_FLAG_xxx and
> + * from VA-API specific AV_VAAPI_FLAG_xxx.
> + *
> + * @since Version 1.
> + *
> + * - encoding: unused
> + * - decoding: Set by user, through av_vaapi_context_init()
> + */
> +unsigned int flags;

I'd say just leave it private, and the user can only initialize it with
av_vaapi_context_init().

> +
>  #if FF_API_VAAPI_CONTEXT
>  /**
>   * VAPictureParameterBuffer ID
> @@ -184,6 +209,30 @@ struct vaapi_context {
>  #endif
>

Re: [FFmpeg-devel] [PATCH 4/4] vaapi: add interfaces to properly initialize context.

2015-08-17 Thread Gwenole Beauchesne
Hi,

2015-08-17 19:17 GMT+02:00 Gwenole Beauchesne :
> Add av_vaapi_context_init() and av_vaapi_context_alloc() helper functions
> so that to properly initialize the vaapi_context structure, and allow for
> future extensions without breaking the API/ABI.
>
> The new version and flags fields are inserted after the last known user
> initialized field, and actually useful field at all, i.e. VA context_id.
> This is safe because the vaapi_context structure was required to be zero
> initialized in the past. And, since those new helper functions and changes
> cause the AV_VAAPI_CONTEXT_VERSION to be bumped, we can track older struct
> requirements from within libavcodec.
>
> Besides, it is now required by design, and actual usage, that the vaapi
> context structure be completely initialized once and for all before the
> AVCodecContext.get_format() function ever completes.
>
> Signed-off-by: Gwenole Beauchesne 
> ---
>  libavcodec/vaapi.c  | 30 +++
>  libavcodec/vaapi.h  | 59 
> +
>  libavcodec/vaapi_internal.h |  1 +
>  3 files changed, 85 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/vaapi.c b/libavcodec/vaapi.c
> index 1ae71a5..4d3e2f3 100644
> --- a/libavcodec/vaapi.c
> +++ b/libavcodec/vaapi.c
> @@ -41,11 +41,41 @@ static void destroy_buffers(VADisplay display, VABufferID 
> *buffers, unsigned int
>  }
>  }
>
> +/* Allocates a vaapi_context structure */
> +struct vaapi_context *av_vaapi_context_alloc(unsigned int flags)
> +{
> +struct vaapi_context *vactx;
> +
> +vactx = av_malloc(sizeof(*vactx));
> +if (!vactx)
> +return NULL;
> +
> +av_vaapi_context_init(vactx, AV_VAAPI_CONTEXT_VERSION, flags);
> +return vactx;
> +}
> +
> +/* Initializes a vaapi_context structure with safe defaults */
> +void av_vaapi_context_init(struct vaapi_context *vactx, unsigned int version,
> +   unsigned int flags)
> +{
> +vactx->display  = NULL;
> +vactx->config_id= VA_INVALID_ID;
> +vactx->context_id   = VA_INVALID_ID;
> +
> +if (version > 0) {
> +vactx->version  = version;
> +vactx->flags= flags;
> +}
> +}
> +
>  int ff_vaapi_context_init(AVCodecContext *avctx)
>  {
>  FFVAContext * const vactx = ff_vaapi_get_context(avctx);
>  const struct vaapi_context * const user_vactx = avctx->hwaccel_context;
>
> +if (user_vactx->version > 0)
> +vactx->flags= user_vactx->flags;
> +
>  vactx->display  = user_vactx->display;
>  vactx->config_id= user_vactx->config_id;
>  vactx->context_id   = user_vactx->context_id;
> diff --git a/libavcodec/vaapi.h b/libavcodec/vaapi.h
> index 4448a2e..1f032a0 100644
> --- a/libavcodec/vaapi.h
> +++ b/libavcodec/vaapi.h
> @@ -40,14 +40,16 @@
>   * @{
>   */
>
> +#define AV_VAAPI_CONTEXT_VERSION 1
> +
>  /**
>   * This structure is used to share data between the FFmpeg library and
>   * the client video application.
> - * This shall be zero-allocated and available as
> - * AVCodecContext.hwaccel_context. All user members can be set once
> - * during initialization or through each AVCodecContext.get_buffer()
> - * function call. In any case, they must be valid prior to calling
> - * decoding functions.
> + *
> + * This shall be initialized with av_vaapi_context_init(), or
> + * indirectly through av_vaapi_context_alloc(), and made available as
> + * AVCodecContext.hwaccel_context. All user members must be properly
> + * initialized before AVCodecContext.get_format() completes.
>   */
>  struct vaapi_context {
>  /**
> @@ -74,6 +76,29 @@ struct vaapi_context {
>   */
>  uint32_t context_id;
>
> +/**
> + * This field must be set to AV_VAAPI_CONTEXT_VERSION
> + *
> + * @since Version 1.
> + *
> + * - encoding: unused
> + * - decoding: Set by user, through av_vaapi_context_init()
> + */
> +unsigned int version;
> +
> +/**
> + * A bit field configuring the internal context used by libavcodec
> + *
> + * This is a combination of flags from common AV_HWACCEL_FLAG_xxx and
> + * from VA-API specific AV_VAAPI_FLAG_xxx.
> + *
> + * @since Version 1.
> + *
> + * - encoding: unused
> + * - decoding: Set by user, through av_vaapi_context_init()
> + */
> +unsigned int flags;
> +
>  #if FF_API_VAAPI_CONTEXT
>  /**
>   * VAPictureParameterBuffer ID
> @@ -184,6 +209,30 @@ struct vaapi_context {
>  #endif
>  };
>
> +/**
> + * Allocates a vaapi_context structure.
> + *
> + * This function allocates and initializes a vaapi_context with safe
> + * defaults, e.g. with proper invalid ids for VA config and context.
> + *
> + * The resulting structure can be deallocated with av_freep().
> + *
> + * @param[in]  flagszero, or a combination of AV_HWACCEL_FLAG_xxx or
> + * AV_VAAPI_FLAG_xxx flags OR'd altogether.
> + * @return Newly-allocated struct vaapi_contex

[FFmpeg-devel] [PATCH 4/4] vaapi: add interfaces to properly initialize context.

2015-08-17 Thread Gwenole Beauchesne
Add av_vaapi_context_init() and av_vaapi_context_alloc() helper functions
so that to properly initialize the vaapi_context structure, and allow for
future extensions without breaking the API/ABI.

The new version and flags fields are inserted after the last known user
initialized field, and actually useful field at all, i.e. VA context_id.
This is safe because the vaapi_context structure was required to be zero
initialized in the past. And, since those new helper functions and changes
cause the AV_VAAPI_CONTEXT_VERSION to be bumped, we can track older struct
requirements from within libavcodec.

Besides, it is now required by design, and actual usage, that the vaapi
context structure be completely initialized once and for all before the
AVCodecContext.get_format() function ever completes.

Signed-off-by: Gwenole Beauchesne 
---
 libavcodec/vaapi.c  | 30 +++
 libavcodec/vaapi.h  | 59 +
 libavcodec/vaapi_internal.h |  1 +
 3 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/libavcodec/vaapi.c b/libavcodec/vaapi.c
index 1ae71a5..4d3e2f3 100644
--- a/libavcodec/vaapi.c
+++ b/libavcodec/vaapi.c
@@ -41,11 +41,41 @@ static void destroy_buffers(VADisplay display, VABufferID 
*buffers, unsigned int
 }
 }
 
+/* Allocates a vaapi_context structure */
+struct vaapi_context *av_vaapi_context_alloc(unsigned int flags)
+{
+struct vaapi_context *vactx;
+
+vactx = av_malloc(sizeof(*vactx));
+if (!vactx)
+return NULL;
+
+av_vaapi_context_init(vactx, AV_VAAPI_CONTEXT_VERSION, flags);
+return vactx;
+}
+
+/* Initializes a vaapi_context structure with safe defaults */
+void av_vaapi_context_init(struct vaapi_context *vactx, unsigned int version,
+   unsigned int flags)
+{
+vactx->display  = NULL;
+vactx->config_id= VA_INVALID_ID;
+vactx->context_id   = VA_INVALID_ID;
+
+if (version > 0) {
+vactx->version  = version;
+vactx->flags= flags;
+}
+}
+
 int ff_vaapi_context_init(AVCodecContext *avctx)
 {
 FFVAContext * const vactx = ff_vaapi_get_context(avctx);
 const struct vaapi_context * const user_vactx = avctx->hwaccel_context;
 
+if (user_vactx->version > 0)
+vactx->flags= user_vactx->flags;
+
 vactx->display  = user_vactx->display;
 vactx->config_id= user_vactx->config_id;
 vactx->context_id   = user_vactx->context_id;
diff --git a/libavcodec/vaapi.h b/libavcodec/vaapi.h
index 4448a2e..1f032a0 100644
--- a/libavcodec/vaapi.h
+++ b/libavcodec/vaapi.h
@@ -40,14 +40,16 @@
  * @{
  */
 
+#define AV_VAAPI_CONTEXT_VERSION 1
+
 /**
  * This structure is used to share data between the FFmpeg library and
  * the client video application.
- * This shall be zero-allocated and available as
- * AVCodecContext.hwaccel_context. All user members can be set once
- * during initialization or through each AVCodecContext.get_buffer()
- * function call. In any case, they must be valid prior to calling
- * decoding functions.
+ *
+ * This shall be initialized with av_vaapi_context_init(), or
+ * indirectly through av_vaapi_context_alloc(), and made available as
+ * AVCodecContext.hwaccel_context. All user members must be properly
+ * initialized before AVCodecContext.get_format() completes.
  */
 struct vaapi_context {
 /**
@@ -74,6 +76,29 @@ struct vaapi_context {
  */
 uint32_t context_id;
 
+/**
+ * This field must be set to AV_VAAPI_CONTEXT_VERSION
+ *
+ * @since Version 1.
+ *
+ * - encoding: unused
+ * - decoding: Set by user, through av_vaapi_context_init()
+ */
+unsigned int version;
+
+/**
+ * A bit field configuring the internal context used by libavcodec
+ *
+ * This is a combination of flags from common AV_HWACCEL_FLAG_xxx and
+ * from VA-API specific AV_VAAPI_FLAG_xxx.
+ *
+ * @since Version 1.
+ *
+ * - encoding: unused
+ * - decoding: Set by user, through av_vaapi_context_init()
+ */
+unsigned int flags;
+
 #if FF_API_VAAPI_CONTEXT
 /**
  * VAPictureParameterBuffer ID
@@ -184,6 +209,30 @@ struct vaapi_context {
 #endif
 };
 
+/**
+ * Allocates a vaapi_context structure.
+ *
+ * This function allocates and initializes a vaapi_context with safe
+ * defaults, e.g. with proper invalid ids for VA config and context.
+ *
+ * The resulting structure can be deallocated with av_freep().
+ *
+ * @param[in]  flagszero, or a combination of AV_HWACCEL_FLAG_xxx or
+ * AV_VAAPI_FLAG_xxx flags OR'd altogether.
+ * @return Newly-allocated struct vaapi_context, or NULL on failure
+ */
+struct vaapi_context *av_vaapi_context_alloc(unsigned int flags);
+
+/**
+ * Initializes a vaapi_context structure with safe defaults.
+ *
+ * @param[in]  version  this must be set to AV_VAAPI_CONTEXT_VERSION
+ * @param[in]  flagszero, or a combination of AV_HWACCEL_FLAG_xxx or
+ * AV_