Re: [libav-devel] [PATCH] vaapi_encode: Write sequence header as extradata

[Mark Thompson]

On 12/10/16 10:38, Anton Khirnov wrote:
> Quoting Mark Thompson (2016-10-10 21:54:35)
>> Only works if packed headers are supported, where we can know the
>> output before generating the first frame.
>> ---
>> Added padding; fail harder; informative comment in header.
>>
>> Not sure how to do this in the non-packed-header case - we could just
>> invoke this anyway, but the result is unlikely to precisely match what
>> the encoder then produces.
> 
> I guess we shouldn't then -- creating corrupted files is evil. At least
> in avconv we can now use the extract_extradata bitstream filter (once it
> goes in) to get the extradata from the first packet.

Ok.  (Later, the bitstream filter could be applied automatically by the encoder
when it knows it doesn't have packed header support?)

>>
>>  libavcodec/vaapi_encode.c | 22 ++
>>  libavcodec/vaapi_encode.h |  2 ++
>>  2 files changed, 24 insertions(+)
>>
>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>> index b600a00..4dc1f50 100644
>> --- a/libavcodec/vaapi_encode.c
>> +++ b/libavcodec/vaapi_encode.c
>> @@ -1399,6 +1399,28 @@ 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, _len);
>> +if (err < 0) {
>> +av_log(avctx, AV_LOG_ERROR, "Failed to write sequence header "
>> +   "for extradata: %d.\n", err);
>> +goto fail;
>> +} else {
>> +avctx->extradata_size = bit_len / 8;
> 
> Round up? Or is this guaranteed to be byte-aligned (in which case why is
> it not in bytes in the first place)?

I'll add the round up.

It is byte-aligned in all current cases; the length is in bits to be consistent
with the other header-writing functions, which can produce non-byte-aligned 
output.

Thanks,

- Mark

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] vaapi_encode: Write sequence header as extradata

[Anton Khirnov]

Quoting Mark Thompson (2016-10-10 21:54:35)
> Only works if packed headers are supported, where we can know the
> output before generating the first frame.
> ---
> Added padding; fail harder; informative comment in header.
> 
> Not sure how to do this in the non-packed-header case - we could just
> invoke this anyway, but the result is unlikely to precisely match what
> the encoder then produces.

I guess we shouldn't then -- creating corrupted files is evil. At least
in avconv we can now use the extract_extradata bitstream filter (once it
goes in) to get the extradata from the first packet.

> 
>  libavcodec/vaapi_encode.c | 22 ++
>  libavcodec/vaapi_encode.h |  2 ++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index b600a00..4dc1f50 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -1399,6 +1399,28 @@ 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, _len);
> +if (err < 0) {
> +av_log(avctx, AV_LOG_ERROR, "Failed to write sequence header "
> +   "for extradata: %d.\n", err);
> +goto fail;
> +} else {
> +avctx->extradata_size = bit_len / 8;

Round up? Or is this guaranteed to be byte-aligned (in which case why is
it not in bytes in the first place)?

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] vaapi_encode: Write sequence header as extradata

[Mark Thompson]

Only works if packed headers are supported, where we can know the
output before generating the first frame.
---
Added padding; fail harder; informative comment in header.

Not sure how to do this in the non-packed-header case - we could just invoke 
this anyway, but the result is unlikely to precisely match what the encoder 
then produces.

 libavcodec/vaapi_encode.c | 22 ++
 libavcodec/vaapi_encode.h |  2 ++
 2 files changed, 24 insertions(+)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index b600a00..4dc1f50 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1399,6 +1399,28 @@ 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, _len);
+if (err < 0) {
+av_log(avctx, AV_LOG_ERROR, "Failed to write sequence header "
+   "for extradata: %d.\n", err);
+goto fail;
+} else {
+avctx->extradata_size = bit_len / 8;
+avctx->extradata = av_mallocz(avctx->extradata_size +
+  AV_INPUT_BUFFER_PADDING_SIZE);
+if (!avctx->extradata) {
+err = AVERROR(ENOMEM);
+goto fail;
+}
+memcpy(avctx->extradata, data, avctx->extradata_size);
+}
+}
+
 return 0;

 fail:
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index c47d979..a9ab527 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -235,6 +235,8 @@ typedef struct VAAPIEncodeType {
 int slice_header_type;

 // Write the packed header data to the provided buffer.
+// The sequence header is also used to fill the codec extradata
+// when the encoder is starting.
 int (*write_sequence_header)(AVCodecContext *avctx,
  char *data, size_t *data_len);
 int  (*write_picture_header)(AVCodecContext *avctx,
-- 
2.9.3

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] vaapi_encode: Write sequence header as extradata

[Mark Thompson]

On 02/10/16 17:05, Vittorio Giovara wrote:
> On Sun, Oct 2, 2016 at 4:19 AM, Mark Thompson  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, _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

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] vaapi_encode: Write sequence header as extradata

[Vittorio Giovara]

On Sun, Oct 2, 2016 at 4:19 AM, Mark Thompson  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, _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.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] vaapi_encode: Write sequence header as extradata

[Mark Thompson]

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, _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:
-- 
2.9.3

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel