vaapi_encode: Write sequence header as extradata

Message ID 3bd8264c-5e1b-81a5-8eac-ef00a338d389@jkqxz.net
State Superseded
Headers show

Commit Message

Mark Thompson Oct. 2, 2016, 8:19 a.m.
Only works if packed headers are supported, where we can know the
output before generating the first frame.
---
If this is wanted when packed headers aren't supported, we will need a different approach because we can't know what the output will be until the first frame is written.

 libavcodec/vaapi_encode.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Vittorio Giovara Oct. 2, 2016, 4:05 p.m. | #1
On Sun, Oct 2, 2016 at 4:19 AM, Mark Thompson <sw@jkqxz.net> wrote:
> Only works if packed headers are supported, where we can know the
> output before generating the first frame.
> ---
> If this is wanted when packed headers aren't supported, we will need a different approach because we can't know what the output will be until the first frame is written.
>
>  libavcodec/vaapi_encode.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index b600a00..d0afb10 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -1399,6 +1399,27 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
>      // where it actually overlaps properly, though.)
>      ctx->issue_mode = ISSUE_MODE_MAXIMISE_THROUGHPUT;
>
> +    if (ctx->va_packed_headers & VA_ENC_PACKED_HEADER_SEQUENCE &&
> +        ctx->codec->write_sequence_header) {
> +        char data[MAX_PARAM_BUFFER_SIZE];
> +        size_t bit_len = 8 * sizeof(data);
> +
> +        err = ctx->codec->write_sequence_header(avctx, data, &bit_len);
> +        if (err < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Failed to write sequence header "
> +                   "for extradata: %d.\n", err);
> +            // Don't write extradata.
> +        } else {
> +            avctx->extradata_size = bit_len / 8;
> +            avctx->extradata = av_mallocz(avctx->extradata_size);

Don't you need the extra padding here?

> +            if (avctx->extradata) {
> +                memcpy(avctx->extradata, data, avctx->extradata_size);
> +            } else {
> +                avctx->extradata_size = 0;
> +            }

Might just want to fail and return an error here. If malloc fails the
system is KO anyway.
Mark Thompson Oct. 2, 2016, 4:16 p.m. | #2
On 02/10/16 17:05, Vittorio Giovara wrote:
> On Sun, Oct 2, 2016 at 4:19 AM, Mark Thompson <sw@jkqxz.net> wrote:
>> Only works if packed headers are supported, where we can know the
>> output before generating the first frame.
>> ---
>> If this is wanted when packed headers aren't supported, we will need a different approach because we can't know what the output will be until the first frame is written.
>>
>>  libavcodec/vaapi_encode.c | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>> index b600a00..d0afb10 100644
>> --- a/libavcodec/vaapi_encode.c
>> +++ b/libavcodec/vaapi_encode.c
>> @@ -1399,6 +1399,27 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
>>      // where it actually overlaps properly, though.)
>>      ctx->issue_mode = ISSUE_MODE_MAXIMISE_THROUGHPUT;
>>
>> +    if (ctx->va_packed_headers & VA_ENC_PACKED_HEADER_SEQUENCE &&
>> +        ctx->codec->write_sequence_header) {
>> +        char data[MAX_PARAM_BUFFER_SIZE];
>> +        size_t bit_len = 8 * sizeof(data);
>> +
>> +        err = ctx->codec->write_sequence_header(avctx, data, &bit_len);
>> +        if (err < 0) {
>> +            av_log(avctx, AV_LOG_ERROR, "Failed to write sequence header "
>> +                   "for extradata: %d.\n", err);
>> +            // Don't write extradata.
>> +        } else {
>> +            avctx->extradata_size = bit_len / 8;
>> +            avctx->extradata = av_mallocz(avctx->extradata_size);
> 
> Don't you need the extra padding here?

Apparently yes.  (Having been informed about its existence.)

>> +            if (avctx->extradata) {
>> +                memcpy(avctx->extradata, data, avctx->extradata_size);
>> +            } else {
>> +                avctx->extradata_size = 0;
>> +            }
> 
> Might just want to fail and return an error here. If malloc fails the
> system is KO anyway.

Yeah, probably fair.

Thanks,

- Mark

Patch

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index b600a00..d0afb10 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1399,6 +1399,27 @@  av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
     // where it actually overlaps properly, though.)
     ctx->issue_mode = ISSUE_MODE_MAXIMISE_THROUGHPUT;

+    if (ctx->va_packed_headers & VA_ENC_PACKED_HEADER_SEQUENCE &&
+        ctx->codec->write_sequence_header) {
+        char data[MAX_PARAM_BUFFER_SIZE];
+        size_t bit_len = 8 * sizeof(data);
+
+        err = ctx->codec->write_sequence_header(avctx, data, &bit_len);
+        if (err < 0) {
+            av_log(avctx, AV_LOG_ERROR, "Failed to write sequence header "
+                   "for extradata: %d.\n", err);
+            // Don't write extradata.
+        } else {
+            avctx->extradata_size = bit_len / 8;
+            avctx->extradata = av_mallocz(avctx->extradata_size);
+            if (avctx->extradata) {
+                memcpy(avctx->extradata, data, avctx->extradata_size);
+            } else {
+                avctx->extradata_size = 0;
+            }
+        }
+    }
+
     return 0;

 fail: