* [FFmpeg-devel] [PATCH] cbs_av1: Make fake OBU size length field a write option
@ 2023-09-25 13:53 Mark Thompson
2023-09-26 2:34 ` Wang, Fei W
2023-09-26 9:19 ` [FFmpeg-devel] [PATCH] " Andreas Rheinhardt
0 siblings, 2 replies; 7+ messages in thread
From: Mark Thompson @ 2023-09-25 13:53 UTC (permalink / raw)
To: FFmpeg development discussions and patches
This is an option to modify the behaviour of the writer, not a syntax
field.
---
Tested by hacking av1_metadata. For example, adding:
av_opt_set_int(ctx->common.output->priv_data, "fixed_obu_size_length", 7, 0);
gets you OBU headers that look like:
[trace_headers @ 0x55706fcb2880] OBU header
[trace_headers @ 0x55706fcb2880] 0 obu_forbidden_bit 0 = 0
[trace_headers @ 0x55706fcb2880] 1 obu_type 0100 = 4
[trace_headers @ 0x55706fcb2880] 5 obu_extension_flag 0 = 0
[trace_headers @ 0x55706fcb2880] 6 obu_has_size_field 1 = 1
[trace_headers @ 0x55706fcb2880] 7 obu_reserved_1bit 0 = 0
[trace_headers @ 0x55706fcb2880] 8 obu_size 10101110100010101000000010000000100000001000000000000000 = 1326
It's not obvious that there is any value in exposing this option more generally, though? It could made a visible option of av1_metadata or others if there is any use-case for it.
Thanks,
- Mark
libavcodec/cbs_av1.c | 31 +++++++++++++++++++++----------
libavcodec/cbs_av1.h | 5 ++++-
libavcodec/vaapi_encode_av1.c | 4 +++-
3 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index 4e687ace79..ed9a7b80d4 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -138,19 +138,25 @@ static int cbs_av1_read_leb128(CodedBitstreamContext *ctx, GetBitContext *gbc,
return 0;
}
-/** Minimum byte length will be used to indicate the len128 of value if byte_len is 0. */
static int cbs_av1_write_leb128(CodedBitstreamContext *ctx, PutBitContext *pbc,
- const char *name, uint64_t value, uint8_t byte_len)
+ const char *name, uint64_t value, int fixed_length)
{
int len, i;
uint8_t byte;
CBS_TRACE_WRITE_START();
- if (byte_len)
- av_assert0(byte_len >= (av_log2(value) + 7) / 7);
+ len = (av_log2(value) + 7) / 7;
- len = byte_len ? byte_len : (av_log2(value) + 7) / 7;
+ if (fixed_length) {
+ if (fixed_length < len) {
+ av_log(ctx->log_ctx, AV_LOG_ERROR, "OBU is too large for "
+ "fixed length size field (%d > %d).\n",
+ len, fixed_length);
+ return AVERROR(EINVAL);
+ }
+ len = fixed_length;
+ }
for (i = 0; i < len; i++) {
if (put_bits_left(pbc) < 8)
@@ -1006,8 +1012,8 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
if (obu->header.obu_has_size_field) {
pbc_tmp = *pbc;
- if (obu->obu_size_byte_len) {
- for (int i = 0; i < obu->obu_size_byte_len; i++)
+ if (priv->fixed_obu_size_length) {
+ for (int i = 0; i < priv->fixed_obu_size_length; i++)
put_bits(pbc, 8, 0);
} else {
// Add space for the size field to fill later.
@@ -1133,7 +1139,8 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
end_pos /= 8;
*pbc = pbc_tmp;
- err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size, obu->obu_size_byte_len);
+ err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size,
+ priv->fixed_obu_size_length);
if (err < 0)
goto error;
@@ -1150,10 +1157,12 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
}
if (obu->obu_size > 0) {
- if (!obu->obu_size_byte_len) {
- obu->obu_size_byte_len = start_pos - data_pos;
+ if (!priv->fixed_obu_size_length) {
memmove(pbc->buf + data_pos,
pbc->buf + start_pos, header_size);
+ } else {
+ // The size was fixed so the following data was
+ // already written in the correct place.
}
skip_put_bytes(pbc, header_size);
@@ -1273,6 +1282,8 @@ static const CodedBitstreamUnitTypeDescriptor cbs_av1_unit_types[] = {
static const AVOption cbs_av1_options[] = {
{ "operating_point", "Set operating point to select layers to parse from a scalable bitstream",
OFFSET(operating_point), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, AV1_MAX_OPERATING_POINTS - 1, 0 },
+ { "fixed_obu_size_length", "Set fixed length of the obu_size field",
+ OFFSET(fixed_obu_size_length), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 8, 0 },
{ NULL }
};
diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index a9e2d2284f..7924257164 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -401,7 +401,6 @@ typedef struct AV1RawOBU {
AV1RawOBUHeader header;
size_t obu_size;
- uint8_t obu_size_byte_len;
union {
AV1RawSequenceHeader sequence_header;
@@ -468,6 +467,10 @@ typedef struct CodedBitstreamAV1Context {
// AVOptions
int operating_point;
+ // When writing, fix the length in bytes of the obu_size field.
+ // Writing will fail with an error if an OBU larger than can be
+ // represented by the fixed size is encountered.
+ int fixed_obu_size_length;
} CodedBitstreamAV1Context;
diff --git a/libavcodec/vaapi_encode_av1.c b/libavcodec/vaapi_encode_av1.c
index 3ff1c47b53..861bf4a13b 100644
--- a/libavcodec/vaapi_encode_av1.c
+++ b/libavcodec/vaapi_encode_av1.c
@@ -133,6 +133,9 @@ static av_cold int vaapi_encode_av1_configure(AVCodecContext *avctx)
priv->cbc->trace_context = ctx;
priv->cbc->trace_write_callback = vaapi_encode_av1_trace_write_log;
+ av_opt_set_int(priv->cbc->priv_data, "fixed_obu_size_length",
+ priv->attr_ext2.bits.obu_size_bytes_minus1 + 1, 0);
+
if (ctx->rc_mode->quality) {
priv->q_idx_p = av_clip(ctx->rc_quality, 0, AV1_MAX_QUANT);
if (fabs(avctx->i_quant_factor) > 0.0)
@@ -634,7 +637,6 @@ static int vaapi_encode_av1_init_picture_params(AVCodecContext *avctx,
}
}
- fh_obu->obu_size_byte_len = priv->attr_ext2.bits.obu_size_bytes_minus1 + 1;
ret = vaapi_encode_av1_add_obu(avctx, obu, AV1_OBU_FRAME_HEADER, &priv->fh);
if (ret < 0)
goto end;
--
2.39.2
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] cbs_av1: Make fake OBU size length field a write option
2023-09-25 13:53 [FFmpeg-devel] [PATCH] cbs_av1: Make fake OBU size length field a write option Mark Thompson
@ 2023-09-26 2:34 ` Wang, Fei W
2023-09-26 20:30 ` [FFmpeg-devel] [PATCH v2] " Mark Thompson
2023-09-26 9:19 ` [FFmpeg-devel] [PATCH] " Andreas Rheinhardt
1 sibling, 1 reply; 7+ messages in thread
From: Wang, Fei W @ 2023-09-26 2:34 UTC (permalink / raw)
To: ffmpeg-devel
On Mon, 2023-09-25 at 14:53 +0100, Mark Thompson wrote:
> This is an option to modify the behaviour of the writer, not a syntax
> field.
> ---
> Tested by hacking av1_metadata. For example, adding:
>
> av_opt_set_int(ctx->common.output->priv_data,
> "fixed_obu_size_length", 7, 0);
>
> gets you OBU headers that look like:
>
> [trace_headers @ 0x55706fcb2880] OBU header
> [trace_headers @ 0x55706fcb2880]
> 0 obu_forbidden_bit
> 0 = 0
> [trace_headers @ 0x55706fcb2880]
> 1 obu_type
> 0100 = 4
> [trace_headers @ 0x55706fcb2880]
> 5 obu_extension_flag
> 0 = 0
> [trace_headers @ 0x55706fcb2880]
> 6 obu_has_size_field
> 1 = 1
> [trace_headers @ 0x55706fcb2880]
> 7 obu_reserved_1bit
> 0 = 0
> [trace_headers @ 0x55706fcb2880]
> 8 obu_size 10101110100010101000000010000000100000001000000
> 000000000 = 1326
>
> It's not obvious that there is any value in exposing this option more
> generally, though? It could made a visible option of av1_metadata or
> others if there is any use-case for it.
>
> Thanks,
>
> - Mark
>
>
> libavcodec/cbs_av1.c | 31 +++++++++++++++++++++----------
> libavcodec/cbs_av1.h | 5 ++++-
> libavcodec/vaapi_encode_av1.c | 4 +++-
> 3 files changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> index 4e687ace79..ed9a7b80d4 100644
> --- a/libavcodec/cbs_av1.c
> +++ b/libavcodec/cbs_av1.c
> @@ -138,19 +138,25 @@ static int
> cbs_av1_read_leb128(CodedBitstreamContext *ctx, GetBitContext *gbc,
> return 0;
> }
>
> -/** Minimum byte length will be used to indicate the len128 of value
> if byte_len is 0. */
> static int cbs_av1_write_leb128(CodedBitstreamContext *ctx,
> PutBitContext *pbc,
> - const char *name, uint64_t value,
> uint8_t byte_len)
> + const char *name, uint64_t value,
> int fixed_length)
> {
> int len, i;
> uint8_t byte;
>
> CBS_TRACE_WRITE_START();
>
> - if (byte_len)
> - av_assert0(byte_len >= (av_log2(value) + 7) / 7);
> + len = (av_log2(value) + 7) / 7;
>
> - len = byte_len ? byte_len : (av_log2(value) + 7) / 7;
> + if (fixed_length) {
> + if (fixed_length < len) {
> + av_log(ctx->log_ctx, AV_LOG_ERROR, "OBU is too large for
> "
> + "fixed length size field (%d > %d).\n",
> + len, fixed_length);
> + return AVERROR(EINVAL);
> + }
> + len = fixed_length;
> + }
>
> for (i = 0; i < len; i++) {
> if (put_bits_left(pbc) < 8)
> @@ -1006,8 +1012,8 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
>
> if (obu->header.obu_has_size_field) {
> pbc_tmp = *pbc;
> - if (obu->obu_size_byte_len) {
> - for (int i = 0; i < obu->obu_size_byte_len; i++)
> + if (priv->fixed_obu_size_length) {
> + for (int i = 0; i < priv->fixed_obu_size_length; i++)
> put_bits(pbc, 8, 0);
> } else {
> // Add space for the size field to fill later.
> @@ -1133,7 +1139,8 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> end_pos /= 8;
>
> *pbc = pbc_tmp;
> - err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size,
> obu->obu_size_byte_len);
> + err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size,
> + priv->fixed_obu_size_length);
> if (err < 0)
> goto error;
>
> @@ -1150,10 +1157,12 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> }
>
> if (obu->obu_size > 0) {
> - if (!obu->obu_size_byte_len) {
> - obu->obu_size_byte_len = start_pos - data_pos;
> + if (!priv->fixed_obu_size_length) {
> memmove(pbc->buf + data_pos,
> pbc->buf + start_pos, header_size);
> + } else {
> + // The size was fixed so the following data was
> + // already written in the correct place.
> }
> skip_put_bytes(pbc, header_size);
>
> @@ -1273,6 +1282,8 @@ static const CodedBitstreamUnitTypeDescriptor
> cbs_av1_unit_types[] = {
> static const AVOption cbs_av1_options[] = {
> { "operating_point", "Set operating point to select layers to
> parse from a scalable bitstream",
> OFFSET(operating_point), AV_OPT_TYPE_INT,
> { .i64 = -1 }, -1, AV1_MAX_OPERATING_POINTS - 1, 0 },
> + { "fixed_obu_size_length", "Set fixed length of the obu_size
> field",
> + OFFSET(fixed_obu_size_length), AV_OPT_TYPE_INT, { .i64 = 0 },
> 0, 8, 0 },
> { NULL }
> };
>
> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
> index a9e2d2284f..7924257164 100644
> --- a/libavcodec/cbs_av1.h
> +++ b/libavcodec/cbs_av1.h
> @@ -401,7 +401,6 @@ typedef struct AV1RawOBU {
> AV1RawOBUHeader header;
>
> size_t obu_size;
> - uint8_t obu_size_byte_len;
>
> union {
> AV1RawSequenceHeader sequence_header;
> @@ -468,6 +467,10 @@ typedef struct CodedBitstreamAV1Context {
>
> // AVOptions
> int operating_point;
> + // When writing, fix the length in bytes of the obu_size field.
> + // Writing will fail with an error if an OBU larger than can be
> + // represented by the fixed size is encountered.
> + int fixed_obu_size_length;
> } CodedBitstreamAV1Context;
>
>
> diff --git a/libavcodec/vaapi_encode_av1.c
> b/libavcodec/vaapi_encode_av1.c
> index 3ff1c47b53..861bf4a13b 100644
> --- a/libavcodec/vaapi_encode_av1.c
> +++ b/libavcodec/vaapi_encode_av1.c
> @@ -133,6 +133,9 @@ static av_cold int
> vaapi_encode_av1_configure(AVCodecContext *avctx)
> priv->cbc->trace_context = ctx;
> priv->cbc->trace_write_callback =
> vaapi_encode_av1_trace_write_log;
>
> + av_opt_set_int(priv->cbc->priv_data, "fixed_obu_size_length",
> + priv->attr_ext2.bits.obu_size_bytes_minus1 + 1,
> 0);
> +
This should be put after querying priv.attr_ext2 in
vaapi_encode_av1_init().
Thanks
Fei
> if (ctx->rc_mode->quality) {
> priv->q_idx_p = av_clip(ctx->rc_quality, 0, AV1_MAX_QUANT);
> if (fabs(avctx->i_quant_factor) > 0.0)
> @@ -634,7 +637,6 @@ static int
> vaapi_encode_av1_init_picture_params(AVCodecContext *avctx,
> }
> }
>
> - fh_obu->obu_size_byte_len = priv-
> >attr_ext2.bits.obu_size_bytes_minus1 + 1;
> ret = vaapi_encode_av1_add_obu(avctx, obu,
> AV1_OBU_FRAME_HEADER, &priv->fh);
> if (ret < 0)
> goto end;
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] cbs_av1: Make fake OBU size length field a write option
2023-09-25 13:53 [FFmpeg-devel] [PATCH] cbs_av1: Make fake OBU size length field a write option Mark Thompson
2023-09-26 2:34 ` Wang, Fei W
@ 2023-09-26 9:19 ` Andreas Rheinhardt
2023-09-26 20:05 ` Mark Thompson
1 sibling, 1 reply; 7+ messages in thread
From: Andreas Rheinhardt @ 2023-09-26 9:19 UTC (permalink / raw)
To: ffmpeg-devel
Mark Thompson:
> This is an option to modify the behaviour of the writer, not a syntax
> field.
> ---
> Tested by hacking av1_metadata. For example, adding:
>
> av_opt_set_int(ctx->common.output->priv_data, "fixed_obu_size_length",
> 7, 0);
>
> gets you OBU headers that look like:
>
> [trace_headers @ 0x55706fcb2880] OBU header
> [trace_headers @ 0x55706fcb2880] 0
> obu_forbidden_bit 0 = 0
> [trace_headers @ 0x55706fcb2880] 1
> obu_type 0100 = 4
> [trace_headers @ 0x55706fcb2880] 5
> obu_extension_flag 0 = 0
> [trace_headers @ 0x55706fcb2880] 6
> obu_has_size_field 1 = 1
> [trace_headers @ 0x55706fcb2880] 7
> obu_reserved_1bit 0 = 0
> [trace_headers @ 0x55706fcb2880] 8 obu_size
> 10101110100010101000000010000000100000001000000000000000 = 1326
>
> It's not obvious that there is any value in exposing this option more
> generally, though? It could made a visible option of av1_metadata or
> others if there is any use-case for it.
>
> Thanks,
>
> - Mark
>
>
> libavcodec/cbs_av1.c | 31 +++++++++++++++++++++----------
> libavcodec/cbs_av1.h | 5 ++++-
> libavcodec/vaapi_encode_av1.c | 4 +++-
> 3 files changed, 28 insertions(+), 12 deletions(-)
What is the advantage of this? Why would people want to waste space?
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] cbs_av1: Make fake OBU size length field a write option
2023-09-26 9:19 ` [FFmpeg-devel] [PATCH] " Andreas Rheinhardt
@ 2023-09-26 20:05 ` Mark Thompson
0 siblings, 0 replies; 7+ messages in thread
From: Mark Thompson @ 2023-09-26 20:05 UTC (permalink / raw)
To: ffmpeg-devel
On 26/09/2023 10:19, Andreas Rheinhardt wrote:
> Mark Thompson:
>> This is an option to modify the behaviour of the writer, not a syntax
>> field.
>> ---
>> Tested by hacking av1_metadata. For example, adding:
>>
>> av_opt_set_int(ctx->common.output->priv_data, "fixed_obu_size_length",
>> 7, 0);
>>
>> gets you OBU headers that look like:
>>
>> [trace_headers @ 0x55706fcb2880] OBU header
>> [trace_headers @ 0x55706fcb2880] 0
>> obu_forbidden_bit 0 = 0
>> [trace_headers @ 0x55706fcb2880] 1
>> obu_type 0100 = 4
>> [trace_headers @ 0x55706fcb2880] 5
>> obu_extension_flag 0 = 0
>> [trace_headers @ 0x55706fcb2880] 6
>> obu_has_size_field 1 = 1
>> [trace_headers @ 0x55706fcb2880] 7
>> obu_reserved_1bit 0 = 0
>> [trace_headers @ 0x55706fcb2880] 8 obu_size
>> 10101110100010101000000010000000100000001000000000000000 = 1326
>>
>> It's not obvious that there is any value in exposing this option more
>> generally, though? It could made a visible option of av1_metadata or
>> others if there is any use-case for it.
>>
>> Thanks,
>>
>> - Mark
>>
>>
>> libavcodec/cbs_av1.c | 31 +++++++++++++++++++++----------
>> libavcodec/cbs_av1.h | 5 ++++-
>> libavcodec/vaapi_encode_av1.c | 4 +++-
>> 3 files changed, 28 insertions(+), 12 deletions(-)
>
> What is the advantage of this? Why would people want to waste space?
Because they want to be able to write a different size into the field later without realigning all the following bytes.
VAAPI seems to require this, hence the previous code which added a fake syntax element field to the OBU structure containing the size to be written with, which this patch is replacing with an external option.
Thanks,
- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* [FFmpeg-devel] [PATCH v2] cbs_av1: Make fake OBU size length field a write option
2023-09-26 2:34 ` Wang, Fei W
@ 2023-09-26 20:30 ` Mark Thompson
2023-09-27 2:16 ` Wang, Fei W
0 siblings, 1 reply; 7+ messages in thread
From: Mark Thompson @ 2023-09-26 20:30 UTC (permalink / raw)
To: ffmpeg-devel
This is an option to modify the behaviour of the writer, not a syntax
field.
---
On 26/09/2023 03:34, Wang, Fei W wrote:
> On Mon, 2023-09-25 at 14:53 +0100, Mark Thompson wrote:
>> ...
>> diff --git a/libavcodec/vaapi_encode_av1.c
>> b/libavcodec/vaapi_encode_av1.c
>> index 3ff1c47b53..861bf4a13b 100644
>> --- a/libavcodec/vaapi_encode_av1.c
>> +++ b/libavcodec/vaapi_encode_av1.c
>> @@ -133,6 +133,9 @@ static av_cold int
>> vaapi_encode_av1_configure(AVCodecContext *avctx)
>> priv->cbc->trace_context = ctx;
>> priv->cbc->trace_write_callback =
>> vaapi_encode_av1_trace_write_log;
>>
>> + av_opt_set_int(priv->cbc->priv_data, "fixed_obu_size_length",
>> + priv->attr_ext2.bits.obu_size_bytes_minus1 + 1,
>> 0);
>> +
>
> This should be put after querying priv.attr_ext2 in
> vaapi_encode_av1_init().
Ah, true - I got my init and configure mixed up, but don't have convenient hardware to test. Fixed in this version.
Thanks,
- Mark
libavcodec/cbs_av1.c | 31 +++++++++++++++++++++----------
libavcodec/cbs_av1.h | 5 ++++-
libavcodec/vaapi_encode_av1.c | 4 +++-
3 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index 4e687ace79..ed9a7b80d4 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -138,19 +138,25 @@ static int cbs_av1_read_leb128(CodedBitstreamContext *ctx, GetBitContext *gbc,
return 0;
}
-/** Minimum byte length will be used to indicate the len128 of value if byte_len is 0. */
static int cbs_av1_write_leb128(CodedBitstreamContext *ctx, PutBitContext *pbc,
- const char *name, uint64_t value, uint8_t byte_len)
+ const char *name, uint64_t value, int fixed_length)
{
int len, i;
uint8_t byte;
CBS_TRACE_WRITE_START();
- if (byte_len)
- av_assert0(byte_len >= (av_log2(value) + 7) / 7);
+ len = (av_log2(value) + 7) / 7;
- len = byte_len ? byte_len : (av_log2(value) + 7) / 7;
+ if (fixed_length) {
+ if (fixed_length < len) {
+ av_log(ctx->log_ctx, AV_LOG_ERROR, "OBU is too large for "
+ "fixed length size field (%d > %d).\n",
+ len, fixed_length);
+ return AVERROR(EINVAL);
+ }
+ len = fixed_length;
+ }
for (i = 0; i < len; i++) {
if (put_bits_left(pbc) < 8)
@@ -1006,8 +1012,8 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
if (obu->header.obu_has_size_field) {
pbc_tmp = *pbc;
- if (obu->obu_size_byte_len) {
- for (int i = 0; i < obu->obu_size_byte_len; i++)
+ if (priv->fixed_obu_size_length) {
+ for (int i = 0; i < priv->fixed_obu_size_length; i++)
put_bits(pbc, 8, 0);
} else {
// Add space for the size field to fill later.
@@ -1133,7 +1139,8 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
end_pos /= 8;
*pbc = pbc_tmp;
- err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size, obu->obu_size_byte_len);
+ err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size,
+ priv->fixed_obu_size_length);
if (err < 0)
goto error;
@@ -1150,10 +1157,12 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
}
if (obu->obu_size > 0) {
- if (!obu->obu_size_byte_len) {
- obu->obu_size_byte_len = start_pos - data_pos;
+ if (!priv->fixed_obu_size_length) {
memmove(pbc->buf + data_pos,
pbc->buf + start_pos, header_size);
+ } else {
+ // The size was fixed so the following data was
+ // already written in the correct place.
}
skip_put_bytes(pbc, header_size);
@@ -1273,6 +1282,8 @@ static const CodedBitstreamUnitTypeDescriptor cbs_av1_unit_types[] = {
static const AVOption cbs_av1_options[] = {
{ "operating_point", "Set operating point to select layers to parse from a scalable bitstream",
OFFSET(operating_point), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, AV1_MAX_OPERATING_POINTS - 1, 0 },
+ { "fixed_obu_size_length", "Set fixed length of the obu_size field",
+ OFFSET(fixed_obu_size_length), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 8, 0 },
{ NULL }
};
diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index a9e2d2284f..7924257164 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -401,7 +401,6 @@ typedef struct AV1RawOBU {
AV1RawOBUHeader header;
size_t obu_size;
- uint8_t obu_size_byte_len;
union {
AV1RawSequenceHeader sequence_header;
@@ -468,6 +467,10 @@ typedef struct CodedBitstreamAV1Context {
// AVOptions
int operating_point;
+ // When writing, fix the length in bytes of the obu_size field.
+ // Writing will fail with an error if an OBU larger than can be
+ // represented by the fixed size is encountered.
+ int fixed_obu_size_length;
} CodedBitstreamAV1Context;
diff --git a/libavcodec/vaapi_encode_av1.c b/libavcodec/vaapi_encode_av1.c
index 3ff1c47b53..5a9ff0f798 100644
--- a/libavcodec/vaapi_encode_av1.c
+++ b/libavcodec/vaapi_encode_av1.c
@@ -634,7 +634,6 @@ static int vaapi_encode_av1_init_picture_params(AVCodecContext *avctx,
}
}
- fh_obu->obu_size_byte_len = priv->attr_ext2.bits.obu_size_bytes_minus1 + 1;
ret = vaapi_encode_av1_add_obu(avctx, obu, AV1_OBU_FRAME_HEADER, &priv->fh);
if (ret < 0)
goto end;
@@ -839,6 +838,9 @@ static av_cold int vaapi_encode_av1_init(AVCodecContext *avctx)
priv->attr_ext2.value = attr.value;
}
+ av_opt_set_int(priv->cbc->priv_data, "fixed_obu_size_length",
+ priv->attr_ext2.bits.obu_size_bytes_minus1 + 1, 0);
+
ret = vaapi_encode_av1_set_tile(avctx);
if (ret < 0)
return ret;
--
2.39.2
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] cbs_av1: Make fake OBU size length field a write option
2023-09-26 20:30 ` [FFmpeg-devel] [PATCH v2] " Mark Thompson
@ 2023-09-27 2:16 ` Wang, Fei W
2023-10-02 19:42 ` Mark Thompson
0 siblings, 1 reply; 7+ messages in thread
From: Wang, Fei W @ 2023-09-27 2:16 UTC (permalink / raw)
To: ffmpeg-devel
On Tue, 2023-09-26 at 21:30 +0100, Mark Thompson wrote:
> This is an option to modify the behaviour of the writer, not a syntax
> field.
> ---
> On 26/09/2023 03:34, Wang, Fei W wrote:
> > On Mon, 2023-09-25 at 14:53 +0100, Mark Thompson wrote:
> > > ...
> > > diff --git a/libavcodec/vaapi_encode_av1.c
> > > b/libavcodec/vaapi_encode_av1.c
> > > index 3ff1c47b53..861bf4a13b 100644
> > > --- a/libavcodec/vaapi_encode_av1.c
> > > +++ b/libavcodec/vaapi_encode_av1.c
> > > @@ -133,6 +133,9 @@ static av_cold int
> > > vaapi_encode_av1_configure(AVCodecContext *avctx)
> > > priv->cbc->trace_context = ctx;
> > > priv->cbc->trace_write_callback =
> > > vaapi_encode_av1_trace_write_log;
> > >
> > > + av_opt_set_int(priv->cbc->priv_data,
> > > "fixed_obu_size_length",
> > > + priv->attr_ext2.bits.obu_size_bytes_minus1 +
> > > 1,
> > > 0);
> > > +
> >
> > This should be put after querying priv.attr_ext2 in
> > vaapi_encode_av1_init().
>
> Ah, true - I got my init and configure mixed up, but don't have
> convenient hardware to test. Fixed in this version.
LGTM for this version.
Thanks
Fei
>
> Thanks,
>
> - Mark
>
> libavcodec/cbs_av1.c | 31 +++++++++++++++++++++----------
> libavcodec/cbs_av1.h | 5 ++++-
> libavcodec/vaapi_encode_av1.c | 4 +++-
> 3 files changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> index 4e687ace79..ed9a7b80d4 100644
> --- a/libavcodec/cbs_av1.c
> +++ b/libavcodec/cbs_av1.c
> @@ -138,19 +138,25 @@ static int
> cbs_av1_read_leb128(CodedBitstreamContext *ctx, GetBitContext *gbc,
> return 0;
> }
>
> -/** Minimum byte length will be used to indicate the len128 of value
> if byte_len is 0. */
> static int cbs_av1_write_leb128(CodedBitstreamContext *ctx,
> PutBitContext *pbc,
> - const char *name, uint64_t value,
> uint8_t byte_len)
> + const char *name, uint64_t value,
> int fixed_length)
> {
> int len, i;
> uint8_t byte;
>
> CBS_TRACE_WRITE_START();
>
> - if (byte_len)
> - av_assert0(byte_len >= (av_log2(value) + 7) / 7);
> + len = (av_log2(value) + 7) / 7;
>
> - len = byte_len ? byte_len : (av_log2(value) + 7) / 7;
> + if (fixed_length) {
> + if (fixed_length < len) {
> + av_log(ctx->log_ctx, AV_LOG_ERROR, "OBU is too large for
> "
> + "fixed length size field (%d > %d).\n",
> + len, fixed_length);
> + return AVERROR(EINVAL);
> + }
> + len = fixed_length;
> + }
>
> for (i = 0; i < len; i++) {
> if (put_bits_left(pbc) < 8)
> @@ -1006,8 +1012,8 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
>
> if (obu->header.obu_has_size_field) {
> pbc_tmp = *pbc;
> - if (obu->obu_size_byte_len) {
> - for (int i = 0; i < obu->obu_size_byte_len; i++)
> + if (priv->fixed_obu_size_length) {
> + for (int i = 0; i < priv->fixed_obu_size_length; i++)
> put_bits(pbc, 8, 0);
> } else {
> // Add space for the size field to fill later.
> @@ -1133,7 +1139,8 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> end_pos /= 8;
>
> *pbc = pbc_tmp;
> - err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size,
> obu->obu_size_byte_len);
> + err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size,
> + priv->fixed_obu_size_length);
> if (err < 0)
> goto error;
>
> @@ -1150,10 +1157,12 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> }
>
> if (obu->obu_size > 0) {
> - if (!obu->obu_size_byte_len) {
> - obu->obu_size_byte_len = start_pos - data_pos;
> + if (!priv->fixed_obu_size_length) {
> memmove(pbc->buf + data_pos,
> pbc->buf + start_pos, header_size);
> + } else {
> + // The size was fixed so the following data was
> + // already written in the correct place.
> }
> skip_put_bytes(pbc, header_size);
>
> @@ -1273,6 +1282,8 @@ static const CodedBitstreamUnitTypeDescriptor
> cbs_av1_unit_types[] = {
> static const AVOption cbs_av1_options[] = {
> { "operating_point", "Set operating point to select layers to
> parse from a scalable bitstream",
> OFFSET(operating_point), AV_OPT_TYPE_INT,
> { .i64 = -1 }, -1, AV1_MAX_OPERATING_POINTS - 1, 0 },
> + { "fixed_obu_size_length", "Set fixed length of the obu_size
> field",
> + OFFSET(fixed_obu_size_length), AV_OPT_TYPE_INT, { .i64 = 0 },
> 0, 8, 0 },
> { NULL }
> };
>
> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
> index a9e2d2284f..7924257164 100644
> --- a/libavcodec/cbs_av1.h
> +++ b/libavcodec/cbs_av1.h
> @@ -401,7 +401,6 @@ typedef struct AV1RawOBU {
> AV1RawOBUHeader header;
>
> size_t obu_size;
> - uint8_t obu_size_byte_len;
>
> union {
> AV1RawSequenceHeader sequence_header;
> @@ -468,6 +467,10 @@ typedef struct CodedBitstreamAV1Context {
>
> // AVOptions
> int operating_point;
> + // When writing, fix the length in bytes of the obu_size field.
> + // Writing will fail with an error if an OBU larger than can be
> + // represented by the fixed size is encountered.
> + int fixed_obu_size_length;
> } CodedBitstreamAV1Context;
>
>
> diff --git a/libavcodec/vaapi_encode_av1.c
> b/libavcodec/vaapi_encode_av1.c
> index 3ff1c47b53..5a9ff0f798 100644
> --- a/libavcodec/vaapi_encode_av1.c
> +++ b/libavcodec/vaapi_encode_av1.c
> @@ -634,7 +634,6 @@ static int
> vaapi_encode_av1_init_picture_params(AVCodecContext *avctx,
> }
> }
>
> - fh_obu->obu_size_byte_len = priv-
> >attr_ext2.bits.obu_size_bytes_minus1 + 1;
> ret = vaapi_encode_av1_add_obu(avctx, obu,
> AV1_OBU_FRAME_HEADER, &priv->fh);
> if (ret < 0)
> goto end;
> @@ -839,6 +838,9 @@ static av_cold int
> vaapi_encode_av1_init(AVCodecContext *avctx)
> priv->attr_ext2.value = attr.value;
> }
>
> + av_opt_set_int(priv->cbc->priv_data, "fixed_obu_size_length",
> + priv->attr_ext2.bits.obu_size_bytes_minus1 + 1,
> 0);
> +
> ret = vaapi_encode_av1_set_tile(avctx);
> if (ret < 0)
> return ret;
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] cbs_av1: Make fake OBU size length field a write option
2023-09-27 2:16 ` Wang, Fei W
@ 2023-10-02 19:42 ` Mark Thompson
0 siblings, 0 replies; 7+ messages in thread
From: Mark Thompson @ 2023-10-02 19:42 UTC (permalink / raw)
To: ffmpeg-devel
On 27/09/2023 03:16, Wang, Fei W wrote:
> On Tue, 2023-09-26 at 21:30 +0100, Mark Thompson wrote:
>> This is an option to modify the behaviour of the writer, not a syntax
>> field.
>> ---
>> On 26/09/2023 03:34, Wang, Fei W wrote:
>>> On Mon, 2023-09-25 at 14:53 +0100, Mark Thompson wrote:
>>>> ...
>>>> diff --git a/libavcodec/vaapi_encode_av1.c
>>>> b/libavcodec/vaapi_encode_av1.c
>>>> index 3ff1c47b53..861bf4a13b 100644
>>>> --- a/libavcodec/vaapi_encode_av1.c
>>>> +++ b/libavcodec/vaapi_encode_av1.c
>>>> @@ -133,6 +133,9 @@ static av_cold int
>>>> vaapi_encode_av1_configure(AVCodecContext *avctx)
>>>> priv->cbc->trace_context = ctx;
>>>> priv->cbc->trace_write_callback =
>>>> vaapi_encode_av1_trace_write_log;
>>>>
>>>> + av_opt_set_int(priv->cbc->priv_data,
>>>> "fixed_obu_size_length",
>>>> + priv->attr_ext2.bits.obu_size_bytes_minus1 +
>>>> 1,
>>>> 0);
>>>> +
>>>
>>> This should be put after querying priv.attr_ext2 in
>>> vaapi_encode_av1_init().
>>
>> Ah, true - I got my init and configure mixed up, but don't have
>> convenient hardware to test. Fixed in this version.
>
> LGTM for this version.
Applied. Thank you for the review!
- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-10-02 19:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-25 13:53 [FFmpeg-devel] [PATCH] cbs_av1: Make fake OBU size length field a write option Mark Thompson
2023-09-26 2:34 ` Wang, Fei W
2023-09-26 20:30 ` [FFmpeg-devel] [PATCH v2] " Mark Thompson
2023-09-27 2:16 ` Wang, Fei W
2023-10-02 19:42 ` Mark Thompson
2023-09-26 9:19 ` [FFmpeg-devel] [PATCH] " Andreas Rheinhardt
2023-09-26 20:05 ` Mark Thompson
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git