* [FFmpeg-devel] [PATCH] lavc/cbs_av1: restore CodedBitstreamAV1Context when AVERROR(ENOSPC)
@ 2022-09-26 7:23 Xiang, Haihao
2022-09-27 2:44 ` Chen, Wenbin
2022-09-27 3:31 ` James Almer
0 siblings, 2 replies; 4+ messages in thread
From: Xiang, Haihao @ 2022-09-26 7:23 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Haihao Xiang
From: Haihao Xiang <haihao.xiang@intel.com>
The current pbc might be small for an obu frame, so a new pbc is
required then parse this obu frame again. Because
CodedBitstreamAV1Context has already been updated for this obu frame, we
need to restore CodedBitstreamAV1Context, otherwise
CodedBitstreamAV1Context doesn't match this obu frame when parsing obu
frame again, e.g. CodedBitstreamAV1Context.order_hint.
$ ffmpeg -i input.ivf -c:v copy -f null -
[...]
[av1_frame_merge @ 0x558bc3d6f880] ref_order_hint[i] does not match
inferred value: 20, but should be 22.
[av1_frame_merge @ 0x558bc3d6f880] Failed to write unit 1 (type 6).
[av1_frame_merge @ 0x558bc3d6f880] Failed to write packet.
[obu @ 0x558bc3d6e040] av1_frame_merge filter failed to send output
packet
---
libavcodec/cbs_av1.c | 85 ++++++++++++++++++++++++++++++++++----------
1 file changed, 67 insertions(+), 18 deletions(-)
diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index 154d9156cf..585bc72cdb 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -1058,15 +1058,33 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
AV1RawTileData *td;
size_t header_size;
int err, start_pos, end_pos, data_pos;
+ CodedBitstreamAV1Context av1ctx;
// OBUs in the normal bitstream format must contain a size field
// in every OBU (in annex B it is optional, but we don't support
// writing that).
obu->header.obu_has_size_field = 1;
+ av1ctx = *priv;
+
+ if (priv->sequence_header_ref) {
+ av1ctx.sequence_header_ref = av_buffer_ref(priv->sequence_header_ref);
+ if (!av1ctx.sequence_header_ref) {
+ err = AVERROR(ENOMEM);
+ goto error;
+ }
+ }
+
+ if (priv->frame_header_ref) {
+ av1ctx.frame_header_ref = av_buffer_ref(priv->frame_header_ref);
+ if (!av1ctx.frame_header_ref) {
+ err = AVERROR(ENOMEM);
+ goto error;
+ }
+ }
err = cbs_av1_write_obu_header(ctx, pbc, &obu->header);
if (err < 0)
- return err;
+ goto error;
if (obu->header.obu_has_size_field) {
pbc_tmp = *pbc;
@@ -1084,18 +1102,21 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
err = cbs_av1_write_sequence_header_obu(ctx, pbc,
&obu->obu.sequence_header);
if (err < 0)
- return err;
+ goto error;
av_buffer_unref(&priv->sequence_header_ref);
priv->sequence_header = NULL;
err = ff_cbs_make_unit_refcounted(ctx, unit);
if (err < 0)
- return err;
+ goto error;
priv->sequence_header_ref = av_buffer_ref(unit->content_ref);
- if (!priv->sequence_header_ref)
- return AVERROR(ENOMEM);
+ if (!priv->sequence_header_ref) {
+ err = AVERROR(ENOMEM);
+ goto error;
+ }
+
priv->sequence_header = &obu->obu.sequence_header;
}
break;
@@ -1103,7 +1124,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
{
err = cbs_av1_write_temporal_delimiter_obu(ctx, pbc);
if (err < 0)
- return err;
+ goto error;
}
break;
case AV1_OBU_FRAME_HEADER:
@@ -1115,7 +1136,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
AV1_OBU_REDUNDANT_FRAME_HEADER,
NULL);
if (err < 0)
- return err;
+ goto error;
}
break;
case AV1_OBU_TILE_GROUP:
@@ -1123,7 +1144,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
err = cbs_av1_write_tile_group_obu(ctx, pbc,
&obu->obu.tile_group);
if (err < 0)
- return err;
+ goto error;
td = &obu->obu.tile_group.tile_data;
}
@@ -1132,7 +1153,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
{
err = cbs_av1_write_frame_obu(ctx, pbc, &obu->obu.frame, NULL);
if (err < 0)
- return err;
+ goto error;
td = &obu->obu.frame.tile_group.tile_data;
}
@@ -1141,7 +1162,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
{
err = cbs_av1_write_tile_list_obu(ctx, pbc, &obu->obu.tile_list);
if (err < 0)
- return err;
+ goto error;
td = &obu->obu.tile_list.tile_data;
}
@@ -1150,18 +1171,19 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
{
err = cbs_av1_write_metadata_obu(ctx, pbc, &obu->obu.metadata);
if (err < 0)
- return err;
+ goto error;
}
break;
case AV1_OBU_PADDING:
{
err = cbs_av1_write_padding_obu(ctx, pbc, &obu->obu.padding);
if (err < 0)
- return err;
+ goto error;
}
break;
default:
- return AVERROR(ENOSYS);
+ err = AVERROR(ENOSYS);
+ goto error;
}
end_pos = put_bits_count(pbc);
@@ -1172,7 +1194,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
// Add trailing bits and recalculate.
err = cbs_av1_write_trailing_bits(ctx, pbc, 8 - end_pos % 8);
if (err < 0)
- return err;
+ goto error;
end_pos = put_bits_count(pbc);
obu->obu_size = header_size = (end_pos - start_pos + 7) / 8;
} else {
@@ -1190,14 +1212,36 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
*pbc = pbc_tmp;
err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size);
if (err < 0)
- return err;
+ goto error;
data_pos = put_bits_count(pbc) / 8;
flush_put_bits(pbc);
av_assert0(data_pos <= start_pos);
- if (8 * obu->obu_size > put_bits_left(pbc))
- return AVERROR(ENOSPC);
+ if (8 * obu->obu_size > put_bits_left(pbc)) {
+ av_buffer_unref(&priv->sequence_header_ref);
+ av_buffer_unref(&priv->frame_header_ref);
+ *priv = av1ctx;
+
+ if (av1ctx.sequence_header_ref) {
+ priv->sequence_header_ref = av_buffer_ref(av1ctx.sequence_header_ref);
+ if (!priv->sequence_header_ref) {
+ err = AVERROR(ENOMEM);
+ goto error;
+ }
+ }
+
+ if (av1ctx.frame_header_ref) {
+ priv->frame_header_ref = av_buffer_ref(av1ctx.frame_header_ref);
+ if (!priv->frame_header_ref) {
+ err = AVERROR(ENOMEM);
+ goto error;
+ }
+ }
+
+ err = AVERROR(ENOSPC);
+ goto error;
+ }
if (obu->obu_size > 0) {
memmove(pbc->buf + data_pos,
@@ -1213,8 +1257,13 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
// OBU data must be byte-aligned.
av_assert0(put_bits_count(pbc) % 8 == 0);
+ err = 0;
- return 0;
+error:
+ av_buffer_unref(&av1ctx.sequence_header_ref);
+ av_buffer_unref(&av1ctx.frame_header_ref);
+
+ return err;
}
static int cbs_av1_assemble_fragment(CodedBitstreamContext *ctx,
--
2.17.1
_______________________________________________
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] 4+ messages in thread* Re: [FFmpeg-devel] [PATCH] lavc/cbs_av1: restore CodedBitstreamAV1Context when AVERROR(ENOSPC)
2022-09-26 7:23 [FFmpeg-devel] [PATCH] lavc/cbs_av1: restore CodedBitstreamAV1Context when AVERROR(ENOSPC) Xiang, Haihao
@ 2022-09-27 2:44 ` Chen, Wenbin
2022-09-27 3:31 ` James Almer
1 sibling, 0 replies; 4+ messages in thread
From: Chen, Wenbin @ 2022-09-27 2:44 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> From: Haihao Xiang <haihao.xiang@intel.com>
>
> The current pbc might be small for an obu frame, so a new pbc is
> required then parse this obu frame again. Because
> CodedBitstreamAV1Context has already been updated for this obu frame,
> we
> need to restore CodedBitstreamAV1Context, otherwise
> CodedBitstreamAV1Context doesn't match this obu frame when parsing obu
> frame again, e.g. CodedBitstreamAV1Context.order_hint.
>
> $ ffmpeg -i input.ivf -c:v copy -f null -
> [...]
> [av1_frame_merge @ 0x558bc3d6f880] ref_order_hint[i] does not match
> inferred value: 20, but should be 22.
> [av1_frame_merge @ 0x558bc3d6f880] Failed to write unit 1 (type 6).
> [av1_frame_merge @ 0x558bc3d6f880] Failed to write packet.
> [obu @ 0x558bc3d6e040] av1_frame_merge filter failed to send output
> packet
> ---
> libavcodec/cbs_av1.c | 85 ++++++++++++++++++++++++++++++++++----------
> 1 file changed, 67 insertions(+), 18 deletions(-)
>
> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> index 154d9156cf..585bc72cdb 100644
> --- a/libavcodec/cbs_av1.c
> +++ b/libavcodec/cbs_av1.c
> @@ -1058,15 +1058,33 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> AV1RawTileData *td;
> size_t header_size;
> int err, start_pos, end_pos, data_pos;
> + CodedBitstreamAV1Context av1ctx;
>
> // OBUs in the normal bitstream format must contain a size field
> // in every OBU (in annex B it is optional, but we don't support
> // writing that).
> obu->header.obu_has_size_field = 1;
> + av1ctx = *priv;
> +
> + if (priv->sequence_header_ref) {
> + av1ctx.sequence_header_ref = av_buffer_ref(priv-
> >sequence_header_ref);
> + if (!av1ctx.sequence_header_ref) {
> + err = AVERROR(ENOMEM);
> + goto error;
> + }
> + }
> +
> + if (priv->frame_header_ref) {
> + av1ctx.frame_header_ref = av_buffer_ref(priv->frame_header_ref);
> + if (!av1ctx.frame_header_ref) {
> + err = AVERROR(ENOMEM);
> + goto error;
> + }
> + }
>
> err = cbs_av1_write_obu_header(ctx, pbc, &obu->header);
> if (err < 0)
> - return err;
> + goto error;
>
> if (obu->header.obu_has_size_field) {
> pbc_tmp = *pbc;
> @@ -1084,18 +1102,21 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> err = cbs_av1_write_sequence_header_obu(ctx, pbc,
> &obu->obu.sequence_header);
> if (err < 0)
> - return err;
> + goto error;
>
> av_buffer_unref(&priv->sequence_header_ref);
> priv->sequence_header = NULL;
>
> err = ff_cbs_make_unit_refcounted(ctx, unit);
> if (err < 0)
> - return err;
> + goto error;
>
> priv->sequence_header_ref = av_buffer_ref(unit->content_ref);
> - if (!priv->sequence_header_ref)
> - return AVERROR(ENOMEM);
> + if (!priv->sequence_header_ref) {
> + err = AVERROR(ENOMEM);
> + goto error;
> + }
> +
> priv->sequence_header = &obu->obu.sequence_header;
> }
> break;
> @@ -1103,7 +1124,7 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> {
> err = cbs_av1_write_temporal_delimiter_obu(ctx, pbc);
> if (err < 0)
> - return err;
> + goto error;
> }
> break;
> case AV1_OBU_FRAME_HEADER:
> @@ -1115,7 +1136,7 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> AV1_OBU_REDUNDANT_FRAME_HEADER,
> NULL);
> if (err < 0)
> - return err;
> + goto error;
> }
> break;
> case AV1_OBU_TILE_GROUP:
> @@ -1123,7 +1144,7 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> err = cbs_av1_write_tile_group_obu(ctx, pbc,
> &obu->obu.tile_group);
> if (err < 0)
> - return err;
> + goto error;
>
> td = &obu->obu.tile_group.tile_data;
> }
> @@ -1132,7 +1153,7 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> {
> err = cbs_av1_write_frame_obu(ctx, pbc, &obu->obu.frame, NULL);
> if (err < 0)
> - return err;
> + goto error;
>
> td = &obu->obu.frame.tile_group.tile_data;
> }
> @@ -1141,7 +1162,7 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> {
> err = cbs_av1_write_tile_list_obu(ctx, pbc, &obu->obu.tile_list);
> if (err < 0)
> - return err;
> + goto error;
>
> td = &obu->obu.tile_list.tile_data;
> }
> @@ -1150,18 +1171,19 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> {
> err = cbs_av1_write_metadata_obu(ctx, pbc, &obu->obu.metadata);
> if (err < 0)
> - return err;
> + goto error;
> }
> break;
> case AV1_OBU_PADDING:
> {
> err = cbs_av1_write_padding_obu(ctx, pbc, &obu->obu.padding);
> if (err < 0)
> - return err;
> + goto error;
> }
> break;
> default:
> - return AVERROR(ENOSYS);
> + err = AVERROR(ENOSYS);
> + goto error;
> }
>
> end_pos = put_bits_count(pbc);
> @@ -1172,7 +1194,7 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> // Add trailing bits and recalculate.
> err = cbs_av1_write_trailing_bits(ctx, pbc, 8 - end_pos % 8);
> if (err < 0)
> - return err;
> + goto error;
> end_pos = put_bits_count(pbc);
> obu->obu_size = header_size = (end_pos - start_pos + 7) / 8;
> } else {
> @@ -1190,14 +1212,36 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> *pbc = pbc_tmp;
> err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size);
> if (err < 0)
> - return err;
> + goto error;
>
> data_pos = put_bits_count(pbc) / 8;
> flush_put_bits(pbc);
> av_assert0(data_pos <= start_pos);
>
> - if (8 * obu->obu_size > put_bits_left(pbc))
> - return AVERROR(ENOSPC);
> + if (8 * obu->obu_size > put_bits_left(pbc)) {
> + av_buffer_unref(&priv->sequence_header_ref);
> + av_buffer_unref(&priv->frame_header_ref);
> + *priv = av1ctx;
> +
> + if (av1ctx.sequence_header_ref) {
> + priv->sequence_header_ref =
> av_buffer_ref(av1ctx.sequence_header_ref);
> + if (!priv->sequence_header_ref) {
> + err = AVERROR(ENOMEM);
> + goto error;
> + }
> + }
> +
> + if (av1ctx.frame_header_ref) {
> + priv->frame_header_ref = av_buffer_ref(av1ctx.frame_header_ref);
> + if (!priv->frame_header_ref) {
> + err = AVERROR(ENOMEM);
> + goto error;
> + }
> + }
> +
> + err = AVERROR(ENOSPC);
> + goto error;
> + }
>
> if (obu->obu_size > 0) {
> memmove(pbc->buf + data_pos,
> @@ -1213,8 +1257,13 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
>
> // OBU data must be byte-aligned.
> av_assert0(put_bits_count(pbc) % 8 == 0);
> + err = 0;
>
> - return 0;
> +error:
> + av_buffer_unref(&av1ctx.sequence_header_ref);
> + av_buffer_unref(&av1ctx.frame_header_ref);
> +
> + return err;
> }
>
> static int cbs_av1_assemble_fragment(CodedBitstreamContext *ctx,
> --
> 2.17.1
This patch also fixes my issue. Thanks.
LGTM
>
> _______________________________________________
> 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".
_______________________________________________
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] 4+ messages in thread* Re: [FFmpeg-devel] [PATCH] lavc/cbs_av1: restore CodedBitstreamAV1Context when AVERROR(ENOSPC)
2022-09-26 7:23 [FFmpeg-devel] [PATCH] lavc/cbs_av1: restore CodedBitstreamAV1Context when AVERROR(ENOSPC) Xiang, Haihao
2022-09-27 2:44 ` Chen, Wenbin
@ 2022-09-27 3:31 ` James Almer
2022-09-27 5:38 ` Xiang, Haihao
1 sibling, 1 reply; 4+ messages in thread
From: James Almer @ 2022-09-27 3:31 UTC (permalink / raw)
To: ffmpeg-devel
On 9/26/2022 4:23 AM, Xiang, Haihao wrote:
> From: Haihao Xiang <haihao.xiang@intel.com>
>
> The current pbc might be small for an obu frame, so a new pbc is
> required then parse this obu frame again. Because
> CodedBitstreamAV1Context has already been updated for this obu frame, we
> need to restore CodedBitstreamAV1Context, otherwise
> CodedBitstreamAV1Context doesn't match this obu frame when parsing obu
> frame again, e.g. CodedBitstreamAV1Context.order_hint.
>
> $ ffmpeg -i input.ivf -c:v copy -f null -
> [...]
> [av1_frame_merge @ 0x558bc3d6f880] ref_order_hint[i] does not match
> inferred value: 20, but should be 22.
> [av1_frame_merge @ 0x558bc3d6f880] Failed to write unit 1 (type 6).
> [av1_frame_merge @ 0x558bc3d6f880] Failed to write packet.
> [obu @ 0x558bc3d6e040] av1_frame_merge filter failed to send output
> packet
> ---
> libavcodec/cbs_av1.c | 85 ++++++++++++++++++++++++++++++++++----------
> 1 file changed, 67 insertions(+), 18 deletions(-)
>
> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> index 154d9156cf..585bc72cdb 100644
> --- a/libavcodec/cbs_av1.c
> +++ b/libavcodec/cbs_av1.c
> @@ -1058,15 +1058,33 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> AV1RawTileData *td;
> size_t header_size;
> int err, start_pos, end_pos, data_pos;
> + CodedBitstreamAV1Context av1ctx;
>
> // OBUs in the normal bitstream format must contain a size field
> // in every OBU (in annex B it is optional, but we don't support
> // writing that).
> obu->header.obu_has_size_field = 1;
> + av1ctx = *priv;
> +
> + if (priv->sequence_header_ref) {
> + av1ctx.sequence_header_ref = av_buffer_ref(priv->sequence_header_ref);
> + if (!av1ctx.sequence_header_ref) {
> + err = AVERROR(ENOMEM);
> + goto error;
If this is called, av1ctx.frame_header_ref will be equal to
priv->frame_header_ref and as such unreferenced twice if not NULL. So
just return AVERROR(ENOMEM) here.
> + }
> + }
> +
> + if (priv->frame_header_ref) {
> + av1ctx.frame_header_ref = av_buffer_ref(priv->frame_header_ref);
> + if (!av1ctx.frame_header_ref) {
> + err = AVERROR(ENOMEM);
> + goto error;
> + }
> + }
>
> err = cbs_av1_write_obu_header(ctx, pbc, &obu->header);
> if (err < 0)
> - return err;
> + goto error;
>
> if (obu->header.obu_has_size_field) {
> pbc_tmp = *pbc;
> @@ -1084,18 +1102,21 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> err = cbs_av1_write_sequence_header_obu(ctx, pbc,
> &obu->obu.sequence_header);
> if (err < 0)
> - return err;
> + goto error;
>
> av_buffer_unref(&priv->sequence_header_ref);
> priv->sequence_header = NULL;
>
> err = ff_cbs_make_unit_refcounted(ctx, unit);
> if (err < 0)
> - return err;
> + goto error;
>
> priv->sequence_header_ref = av_buffer_ref(unit->content_ref);
> - if (!priv->sequence_header_ref)
> - return AVERROR(ENOMEM);
> + if (!priv->sequence_header_ref) {
> + err = AVERROR(ENOMEM);
> + goto error;
> + }
> +
> priv->sequence_header = &obu->obu.sequence_header;
> }
> break;
> @@ -1103,7 +1124,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> {
> err = cbs_av1_write_temporal_delimiter_obu(ctx, pbc);
> if (err < 0)
> - return err;
> + goto error;
> }
> break;
> case AV1_OBU_FRAME_HEADER:
> @@ -1115,7 +1136,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> AV1_OBU_REDUNDANT_FRAME_HEADER,
> NULL);
> if (err < 0)
> - return err;
> + goto error;
> }
> break;
> case AV1_OBU_TILE_GROUP:
> @@ -1123,7 +1144,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> err = cbs_av1_write_tile_group_obu(ctx, pbc,
> &obu->obu.tile_group);
> if (err < 0)
> - return err;
> + goto error;
>
> td = &obu->obu.tile_group.tile_data;
> }
> @@ -1132,7 +1153,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> {
> err = cbs_av1_write_frame_obu(ctx, pbc, &obu->obu.frame, NULL);
> if (err < 0)
> - return err;
> + goto error;
>
> td = &obu->obu.frame.tile_group.tile_data;
> }
> @@ -1141,7 +1162,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> {
> err = cbs_av1_write_tile_list_obu(ctx, pbc, &obu->obu.tile_list);
> if (err < 0)
> - return err;
> + goto error;
>
> td = &obu->obu.tile_list.tile_data;
> }
> @@ -1150,18 +1171,19 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> {
> err = cbs_av1_write_metadata_obu(ctx, pbc, &obu->obu.metadata);
> if (err < 0)
> - return err;
> + goto error;
> }
> break;
> case AV1_OBU_PADDING:
> {
> err = cbs_av1_write_padding_obu(ctx, pbc, &obu->obu.padding);
> if (err < 0)
> - return err;
> + goto error;
> }
> break;
> default:
> - return AVERROR(ENOSYS);
> + err = AVERROR(ENOSYS);
> + goto error;
> }
>
> end_pos = put_bits_count(pbc);
> @@ -1172,7 +1194,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> // Add trailing bits and recalculate.
> err = cbs_av1_write_trailing_bits(ctx, pbc, 8 - end_pos % 8);
> if (err < 0)
> - return err;
> + goto error;
> end_pos = put_bits_count(pbc);
> obu->obu_size = header_size = (end_pos - start_pos + 7) / 8;
> } else {
> @@ -1190,14 +1212,36 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> *pbc = pbc_tmp;
> err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size);
> if (err < 0)
> - return err;
> + goto error;
>
> data_pos = put_bits_count(pbc) / 8;
> flush_put_bits(pbc);
> av_assert0(data_pos <= start_pos);
>
> - if (8 * obu->obu_size > put_bits_left(pbc))
> - return AVERROR(ENOSPC);
> + if (8 * obu->obu_size > put_bits_left(pbc)) {
> + av_buffer_unref(&priv->sequence_header_ref);
> + av_buffer_unref(&priv->frame_header_ref);
> + *priv = av1ctx;
> +
> + if (av1ctx.sequence_header_ref) {
> + priv->sequence_header_ref = av_buffer_ref(av1ctx.sequence_header_ref);
> + if (!priv->sequence_header_ref) {
> + err = AVERROR(ENOMEM);
> + goto error;
> + }
> + }
> +
> + if (av1ctx.frame_header_ref) {
> + priv->frame_header_ref = av_buffer_ref(av1ctx.frame_header_ref);
> + if (!priv->frame_header_ref) {
> + err = AVERROR(ENOMEM);
> + goto error;
> + }
> + }
Why are you creating new references? They were already copied to priv
and av1ctx is in the stack, so just return AVERROR(ENOSPC) directly.
> +
> + err = AVERROR(ENOSPC);
> + goto error;
> + }
>
> if (obu->obu_size > 0) {
> memmove(pbc->buf + data_pos,
> @@ -1213,8 +1257,13 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
>
> // OBU data must be byte-aligned.
> av_assert0(put_bits_count(pbc) % 8 == 0);
> + err = 0;
>
> - return 0;
> +error:
> + av_buffer_unref(&av1ctx.sequence_header_ref);
> + av_buffer_unref(&av1ctx.frame_header_ref);
> +
> + return err;
> }
>
> static int cbs_av1_assemble_fragment(CodedBitstreamContext *ctx,
_______________________________________________
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] 4+ messages in thread* Re: [FFmpeg-devel] [PATCH] lavc/cbs_av1: restore CodedBitstreamAV1Context when AVERROR(ENOSPC)
2022-09-27 3:31 ` James Almer
@ 2022-09-27 5:38 ` Xiang, Haihao
0 siblings, 0 replies; 4+ messages in thread
From: Xiang, Haihao @ 2022-09-27 5:38 UTC (permalink / raw)
To: ffmpeg-devel
On Tue, 2022-09-27 at 00:31 -0300, James Almer wrote:
> On 9/26/2022 4:23 AM, Xiang, Haihao wrote:
> > From: Haihao Xiang <haihao.xiang@intel.com>
> >
> > The current pbc might be small for an obu frame, so a new pbc is
> > required then parse this obu frame again. Because
> > CodedBitstreamAV1Context has already been updated for this obu frame, we
> > need to restore CodedBitstreamAV1Context, otherwise
> > CodedBitstreamAV1Context doesn't match this obu frame when parsing obu
> > frame again, e.g. CodedBitstreamAV1Context.order_hint.
> >
> > $ ffmpeg -i input.ivf -c:v copy -f null -
> > [...]
> > [av1_frame_merge @ 0x558bc3d6f880] ref_order_hint[i] does not match
> > inferred value: 20, but should be 22.
> > [av1_frame_merge @ 0x558bc3d6f880] Failed to write unit 1 (type 6).
> > [av1_frame_merge @ 0x558bc3d6f880] Failed to write packet.
> > [obu @ 0x558bc3d6e040] av1_frame_merge filter failed to send output
> > packet
> > ---
> > libavcodec/cbs_av1.c | 85 ++++++++++++++++++++++++++++++++++----------
> > 1 file changed, 67 insertions(+), 18 deletions(-)
> >
> > diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> > index 154d9156cf..585bc72cdb 100644
> > --- a/libavcodec/cbs_av1.c
> > +++ b/libavcodec/cbs_av1.c
> > @@ -1058,15 +1058,33 @@ static int cbs_av1_write_obu(CodedBitstreamContext
> > *ctx,
> > AV1RawTileData *td;
> > size_t header_size;
> > int err, start_pos, end_pos, data_pos;
> > + CodedBitstreamAV1Context av1ctx;
> >
> > // OBUs in the normal bitstream format must contain a size field
> > // in every OBU (in annex B it is optional, but we don't support
> > // writing that).
> > obu->header.obu_has_size_field = 1;
> > + av1ctx = *priv;
> > +
> > + if (priv->sequence_header_ref) {
> > + av1ctx.sequence_header_ref = av_buffer_ref(priv-
> > >sequence_header_ref);
> > + if (!av1ctx.sequence_header_ref) {
> > + err = AVERROR(ENOMEM);
> > + goto error;
>
> If this is called, av1ctx.frame_header_ref will be equal to
> priv->frame_header_ref and as such unreferenced twice if not NULL. So
> just return AVERROR(ENOMEM) here.
Thanks for pointing it out, I'll update it in v2.
>
> > + }
> > + }
> > +
> > + if (priv->frame_header_ref) {
> > + av1ctx.frame_header_ref = av_buffer_ref(priv->frame_header_ref);
> > + if (!av1ctx.frame_header_ref) {
> > + err = AVERROR(ENOMEM);
> > + goto error;
> > + }
> > + }
> >
> > err = cbs_av1_write_obu_header(ctx, pbc, &obu->header);
> > if (err < 0)
> > - return err;
> > + goto error;
> >
> > if (obu->header.obu_has_size_field) {
> > pbc_tmp = *pbc;
> > @@ -1084,18 +1102,21 @@ static int cbs_av1_write_obu(CodedBitstreamContext
> > *ctx,
> > err = cbs_av1_write_sequence_header_obu(ctx, pbc,
> > &obu-
> > >obu.sequence_header);
> > if (err < 0)
> > - return err;
> > + goto error;
> >
> > av_buffer_unref(&priv->sequence_header_ref);
> > priv->sequence_header = NULL;
> >
> > err = ff_cbs_make_unit_refcounted(ctx, unit);
> > if (err < 0)
> > - return err;
> > + goto error;
> >
> > priv->sequence_header_ref = av_buffer_ref(unit->content_ref);
> > - if (!priv->sequence_header_ref)
> > - return AVERROR(ENOMEM);
> > + if (!priv->sequence_header_ref) {
> > + err = AVERROR(ENOMEM);
> > + goto error;
> > + }
> > +
> > priv->sequence_header = &obu->obu.sequence_header;
> > }
> > break;
> > @@ -1103,7 +1124,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext
> > *ctx,
> > {
> > err = cbs_av1_write_temporal_delimiter_obu(ctx, pbc);
> > if (err < 0)
> > - return err;
> > + goto error;
> > }
> > break;
> > case AV1_OBU_FRAME_HEADER:
> > @@ -1115,7 +1136,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext
> > *ctx,
> > AV1_OBU_REDUNDANT_FRAME_H
> > EADER,
> > NULL);
> > if (err < 0)
> > - return err;
> > + goto error;
> > }
> > break;
> > case AV1_OBU_TILE_GROUP:
> > @@ -1123,7 +1144,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext
> > *ctx,
> > err = cbs_av1_write_tile_group_obu(ctx, pbc,
> > &obu->obu.tile_group);
> > if (err < 0)
> > - return err;
> > + goto error;
> >
> > td = &obu->obu.tile_group.tile_data;
> > }
> > @@ -1132,7 +1153,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext
> > *ctx,
> > {
> > err = cbs_av1_write_frame_obu(ctx, pbc, &obu->obu.frame,
> > NULL);
> > if (err < 0)
> > - return err;
> > + goto error;
> >
> > td = &obu->obu.frame.tile_group.tile_data;
> > }
> > @@ -1141,7 +1162,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext
> > *ctx,
> > {
> > err = cbs_av1_write_tile_list_obu(ctx, pbc, &obu-
> > >obu.tile_list);
> > if (err < 0)
> > - return err;
> > + goto error;
> >
> > td = &obu->obu.tile_list.tile_data;
> > }
> > @@ -1150,18 +1171,19 @@ static int cbs_av1_write_obu(CodedBitstreamContext
> > *ctx,
> > {
> > err = cbs_av1_write_metadata_obu(ctx, pbc, &obu-
> > >obu.metadata);
> > if (err < 0)
> > - return err;
> > + goto error;
> > }
> > break;
> > case AV1_OBU_PADDING:
> > {
> > err = cbs_av1_write_padding_obu(ctx, pbc, &obu->obu.padding);
> > if (err < 0)
> > - return err;
> > + goto error;
> > }
> > break;
> > default:
> > - return AVERROR(ENOSYS);
> > + err = AVERROR(ENOSYS);
> > + goto error;
> > }
> >
> > end_pos = put_bits_count(pbc);
> > @@ -1172,7 +1194,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext
> > *ctx,
> > // Add trailing bits and recalculate.
> > err = cbs_av1_write_trailing_bits(ctx, pbc, 8 - end_pos % 8);
> > if (err < 0)
> > - return err;
> > + goto error;
> > end_pos = put_bits_count(pbc);
> > obu->obu_size = header_size = (end_pos - start_pos + 7) / 8;
> > } else {
> > @@ -1190,14 +1212,36 @@ static int cbs_av1_write_obu(CodedBitstreamContext
> > *ctx,
> > *pbc = pbc_tmp;
> > err = cbs_av1_write_leb128(ctx, pbc, "obu_size", obu->obu_size);
> > if (err < 0)
> > - return err;
> > + goto error;
> >
> > data_pos = put_bits_count(pbc) / 8;
> > flush_put_bits(pbc);
> > av_assert0(data_pos <= start_pos);
> >
> > - if (8 * obu->obu_size > put_bits_left(pbc))
> > - return AVERROR(ENOSPC);
> > + if (8 * obu->obu_size > put_bits_left(pbc)) {
> > + av_buffer_unref(&priv->sequence_header_ref);
> > + av_buffer_unref(&priv->frame_header_ref);
> > + *priv = av1ctx;
> > +
> > + if (av1ctx.sequence_header_ref) {
> > + priv->sequence_header_ref =
> > av_buffer_ref(av1ctx.sequence_header_ref);
> > + if (!priv->sequence_header_ref) {
> > + err = AVERROR(ENOMEM);
> > + goto error;
> > + }
> > + }
> > +
> > + if (av1ctx.frame_header_ref) {
> > + priv->frame_header_ref =
> > av_buffer_ref(av1ctx.frame_header_ref);
> > + if (!priv->frame_header_ref) {
> > + err = AVERROR(ENOMEM);
> > + goto error;
> > + }
> > + }
>
> Why are you creating new references? They were already copied to priv
> and av1ctx is in the stack, so just return AVERROR(ENOSPC) directly.
I wanted to return the value in the same place, but you are right, here it is
better to return AVERROR(ENOSPC) directly.
Thanks
Haihao
>
> > +
> > + err = AVERROR(ENOSPC);
> > + goto error;
> > + }
> >
> > if (obu->obu_size > 0) {
> > memmove(pbc->buf + data_pos,
> > @@ -1213,8 +1257,13 @@ static int cbs_av1_write_obu(CodedBitstreamContext
> > *ctx,
> >
> > // OBU data must be byte-aligned.
> > av_assert0(put_bits_count(pbc) % 8 == 0);
> > + err = 0;
> >
> > - return 0;
> > +error:
> > + av_buffer_unref(&av1ctx.sequence_header_ref);
> > + av_buffer_unref(&av1ctx.frame_header_ref);
> > +
> > + return err;
> > }
> >
> > static int cbs_av1_assemble_fragment(CodedBitstreamContext *ctx,
>
> _______________________________________________
> 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".
_______________________________________________
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] 4+ messages in thread
end of thread, other threads:[~2022-09-27 5:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-26 7:23 [FFmpeg-devel] [PATCH] lavc/cbs_av1: restore CodedBitstreamAV1Context when AVERROR(ENOSPC) Xiang, Haihao
2022-09-27 2:44 ` Chen, Wenbin
2022-09-27 3:31 ` James Almer
2022-09-27 5:38 ` Xiang, Haihao
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