diff mbox

[FFmpeg-devel,v2,09/36] vaapi_encode: Allocate picture-private data in generic code

Message ID 20180607234331.32139-10-sw@jkqxz.net
State Accepted
Headers show

Commit Message

Mark Thompson June 7, 2018, 11:43 p.m. UTC 
---
 libavcodec/vaapi_encode.c | 15 ++++++++++++---
 libavcodec/vaapi_encode.h |  4 ++++
 2 files changed, 16 insertions(+), 3 deletions(-)

Comments

Xiang, Haihao June 12, 2018, 4:32 a.m. UTC | #1 
On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
> ---

>  libavcodec/vaapi_encode.c | 15 ++++++++++++---

>  libavcodec/vaapi_encode.h |  4 ++++

>  2 files changed, 16 insertions(+), 3 deletions(-)

> 

> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c

> index cedf3d3549..ed6d289c6b 100644

> --- a/libavcodec/vaapi_encode.c

> +++ b/libavcodec/vaapi_encode.c

> @@ -528,14 +528,23 @@ static int vaapi_encode_discard(AVCodecContext *avctx,

>      return 0;

>  }

>  

> -static VAAPIEncodePicture *vaapi_encode_alloc(void)

> +static VAAPIEncodePicture *vaapi_encode_alloc(AVCodecContext *avctx)

>  {

> +    VAAPIEncodeContext *ctx = avctx->priv_data;

>      VAAPIEncodePicture *pic;

>  

>      pic = av_mallocz(sizeof(*pic));

>      if (!pic)

>          return NULL;

>  

> +    if (ctx->codec->picture_priv_data_size > 0) {

> +        pic->priv_data = av_mallocz(ctx->codec->picture_priv_data_size);

> +        if (!pic->priv_data) {

> +            av_freep(&pic);

> +            return NULL;

> +        }

> +    }

> +

>      pic->input_surface = VA_INVALID_ID;

>      pic->recon_surface = VA_INVALID_ID;

>      pic->output_buffer = VA_INVALID_ID;

> @@ -668,7 +677,7 @@ static int vaapi_encode_get_next(AVCodecContext *avctx,

>          }

>      }

>  

> -    pic = vaapi_encode_alloc();

> +    pic = vaapi_encode_alloc(avctx);

>      if (!pic)

>          return AVERROR(ENOMEM);

>  

> @@ -697,7 +706,7 @@ static int vaapi_encode_get_next(AVCodecContext *avctx,

>  

>          for (i = 0; i < ctx->b_per_p &&

>               ctx->gop_counter < avctx->gop_size; i++) {

> -            pic = vaapi_encode_alloc();

> +            pic = vaapi_encode_alloc(avctx);

>              if (!pic)

>                  goto fail;

>  

> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h

> index c7370a17e2..54dc4a475e 100644

> --- a/libavcodec/vaapi_encode.h

> +++ b/libavcodec/vaapi_encode.h

> @@ -211,6 +211,10 @@ typedef struct VAAPIEncodeType {

>      // add any necessary global parameters).

>      int (*configure)(AVCodecContext *avctx);

>  

> +    // The size of any private data structure associated with each

> +    // picture (can be zero if not required).

> +    size_t picture_priv_data_size;

> +



I didn't see this field is set in the existent vaapi codecs, is the private data
structure really needed for a picture? If not, I'd like to remove the field of
priv_data from VAAPIEncodePicture instead. 


>      // The size of the parameter structures:

>      // sizeof(VAEnc{type}ParameterBuffer{codec}).

>      size_t sequence_params_size;
Mark Thompson June 13, 2018, 9:50 p.m. UTC | #2 
On 12/06/18 05:32, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> ---
>>  libavcodec/vaapi_encode.c | 15 ++++++++++++---
>>  libavcodec/vaapi_encode.h |  4 ++++
>>  2 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>> index cedf3d3549..ed6d289c6b 100644
>> --- a/libavcodec/vaapi_encode.c
>> +++ b/libavcodec/vaapi_encode.c
>> @@ -528,14 +528,23 @@ static int vaapi_encode_discard(AVCodecContext *avctx,
>>      return 0;
>>  }
>>  
>> -static VAAPIEncodePicture *vaapi_encode_alloc(void)
>> +static VAAPIEncodePicture *vaapi_encode_alloc(AVCodecContext *avctx)
>>  {
>> +    VAAPIEncodeContext *ctx = avctx->priv_data;
>>      VAAPIEncodePicture *pic;
>>  
>>      pic = av_mallocz(sizeof(*pic));
>>      if (!pic)
>>          return NULL;
>>  
>> +    if (ctx->codec->picture_priv_data_size > 0) {
>> +        pic->priv_data = av_mallocz(ctx->codec->picture_priv_data_size);
>> +        if (!pic->priv_data) {
>> +            av_freep(&pic);
>> +            return NULL;
>> +        }
>> +    }
>> +
>>      pic->input_surface = VA_INVALID_ID;
>>      pic->recon_surface = VA_INVALID_ID;
>>      pic->output_buffer = VA_INVALID_ID;
>> @@ -668,7 +677,7 @@ static int vaapi_encode_get_next(AVCodecContext *avctx,
>>          }
>>      }
>>  
>> -    pic = vaapi_encode_alloc();
>> +    pic = vaapi_encode_alloc(avctx);
>>      if (!pic)
>>          return AVERROR(ENOMEM);
>>  
>> @@ -697,7 +706,7 @@ static int vaapi_encode_get_next(AVCodecContext *avctx,
>>  
>>          for (i = 0; i < ctx->b_per_p &&
>>               ctx->gop_counter < avctx->gop_size; i++) {
>> -            pic = vaapi_encode_alloc();
>> +            pic = vaapi_encode_alloc(avctx);
>>              if (!pic)
>>                  goto fail;
>>  
>> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
>> index c7370a17e2..54dc4a475e 100644
>> --- a/libavcodec/vaapi_encode.h
>> +++ b/libavcodec/vaapi_encode.h
>> @@ -211,6 +211,10 @@ typedef struct VAAPIEncodeType {
>>      // add any necessary global parameters).
>>      int (*configure)(AVCodecContext *avctx);
>>  
>> +    // The size of any private data structure associated with each
>> +    // picture (can be zero if not required).
>> +    size_t picture_priv_data_size;
>> +
> 
> 
> I didn't see this field is set in the existent vaapi codecs, is the private data
> structure really needed for a picture? If not, I'd like to remove the field of
> priv_data from VAAPIEncodePicture instead. 

Right, it's not actually used yet in the current set.  I'll push this patch back so it goes with others actually using it.

Thanks,

- Mark
diff mbox

Patch

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index cedf3d3549..ed6d289c6b 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -528,14 +528,23 @@  static int vaapi_encode_discard(AVCodecContext *avctx,
     return 0;
 }
 
-static VAAPIEncodePicture *vaapi_encode_alloc(void)
+static VAAPIEncodePicture *vaapi_encode_alloc(AVCodecContext *avctx)
 {
+    VAAPIEncodeContext *ctx = avctx->priv_data;
     VAAPIEncodePicture *pic;
 
     pic = av_mallocz(sizeof(*pic));
     if (!pic)
         return NULL;
 
+    if (ctx->codec->picture_priv_data_size > 0) {
+        pic->priv_data = av_mallocz(ctx->codec->picture_priv_data_size);
+        if (!pic->priv_data) {
+            av_freep(&pic);
+            return NULL;
+        }
+    }
+
     pic->input_surface = VA_INVALID_ID;
     pic->recon_surface = VA_INVALID_ID;
     pic->output_buffer = VA_INVALID_ID;
@@ -668,7 +677,7 @@  static int vaapi_encode_get_next(AVCodecContext *avctx,
         }
     }
 
-    pic = vaapi_encode_alloc();
+    pic = vaapi_encode_alloc(avctx);
     if (!pic)
         return AVERROR(ENOMEM);
 
@@ -697,7 +706,7 @@  static int vaapi_encode_get_next(AVCodecContext *avctx,
 
         for (i = 0; i < ctx->b_per_p &&
              ctx->gop_counter < avctx->gop_size; i++) {
-            pic = vaapi_encode_alloc();
+            pic = vaapi_encode_alloc(avctx);
             if (!pic)
                 goto fail;
 
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index c7370a17e2..54dc4a475e 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -211,6 +211,10 @@  typedef struct VAAPIEncodeType {
     // add any necessary global parameters).
     int (*configure)(AVCodecContext *avctx);
 
+    // The size of any private data structure associated with each
+    // picture (can be zero if not required).
+    size_t picture_priv_data_size;
+
     // The size of the parameter structures:
     // sizeof(VAEnc{type}ParameterBuffer{codec}).
     size_t sequence_params_size;