Patches attached.
- Andreas
From 19b550d20a2f00e776c3e17b56b3ec55abf71752 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
Date: Fri, 30 May 2025 18:06:55 +0200
Subject: [PATCH 1/4] avcodec/dovi_rpuenc: Avoid intermediate codec par in
ff_dovi_configure()
It invalidates (removes by duplicates) AVCodecContext.extradata
and AVCodecContext.coded_side_data which is quite surprising
and leads to bugs like #11617 where an AVCPBProperties
is used after it has been freed in ff_dovi_configure().
Reported-by: Ayose
Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
---
libavcodec/bsf/dovi_rpu.c | 4 +-
libavcodec/dovi_rpu.h | 17 ++++---
libavcodec/dovi_rpuenc.c | 100 ++++++++++++++++++++------------------
3 files changed, 64 insertions(+), 57 deletions(-)
diff --git a/libavcodec/bsf/dovi_rpu.c b/libavcodec/bsf/dovi_rpu.c
index 5dccd4bc7e..84b271f736 100644
--- a/libavcodec/bsf/dovi_rpu.c
+++ b/libavcodec/bsf/dovi_rpu.c
@@ -228,8 +228,8 @@ static int dovi_rpu_init(AVBSFContext *bsf)
} else {
av_log(bsf, AV_LOG_WARNING, "No Dolby Vision configuration record "
"found? Generating one, but results may be invalid.\n");
- ret = ff_dovi_configure_ext(&s->enc, bsf->par_out, NULL, s->compression,
- FF_COMPLIANCE_NORMAL);
+ ret = ff_dovi_configure_from_codedpar(&s->enc, bsf->par_out, NULL, s->compression,
+ FF_COMPLIANCE_NORMAL);
if (ret < 0)
return ret;
/* Be conservative in accepting all compressed RPUs */
diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h
index f3ccc27ae8..1b74983205 100644
--- a/libavcodec/dovi_rpu.h
+++ b/libavcodec/dovi_rpu.h
@@ -133,9 +133,10 @@ int ff_dovi_attach_side_data(DOVIContext *s, AVFrame *frame);
/**
* Configure the encoder for Dolby Vision encoding. Generates a configuration
- * record in s->cfg, and attaches it to avctx->coded_side_data. Sets the correct
- * profile and compatibility ID based on the tagged AVCodecParameters colorspace
- * metadata, and the correct level based on the resolution and tagged framerate.
+ * record in s->cfg, and attaches it to codecpar->coded_side_data. Sets the
+ * correct profile and compatibility ID based on the tagged AVCodecParameters
+ * colorspace metadata, and the correct level based on the resolution and
+ * tagged framerate.
*
* `metadata` should point to the first frame's RPU, if available. If absent,
* auto-detection will be performed, but this can sometimes lead to inaccurate
@@ -143,13 +144,13 @@ int ff_dovi_attach_side_data(DOVIContext *s, AVFrame *frame);
*
* Returns 0 or a negative error code.
*/
-int ff_dovi_configure_ext(DOVIContext *s, AVCodecParameters *codecpar,
- const AVDOVIMetadata *metadata,
- enum AVDOVICompression compression,
- int strict_std_compliance);
+int ff_dovi_configure_from_codedpar(DOVIContext *s, AVCodecParameters *codecpar,
+ const AVDOVIMetadata *metadata,
+ enum AVDOVICompression compression,
+ int strict_std_compliance);
/**
- * Helper wrapper around `ff_dovi_configure_ext` which infers the codec
+ * Variant of `ff_dovi_configure_from_codedpar` which infers the codec
* parameters from an AVCodecContext.
*/
int ff_dovi_configure(DOVIContext *s, AVCodecContext *avctx);
diff --git a/libavcodec/dovi_rpuenc.c b/libavcodec/dovi_rpuenc.c
index 2e1f8be08e..e04bce6431 100644
--- a/libavcodec/dovi_rpuenc.c
+++ b/libavcodec/dovi_rpuenc.c
@@ -52,10 +52,18 @@ static const struct {
[13] = {7680*4320*120u, 7680, 240, 800},
};
-int ff_dovi_configure_ext(DOVIContext *s, AVCodecParameters *codecpar,
- const AVDOVIMetadata *metadata,
- enum AVDOVICompression compression,
- int strict_std_compliance)
+static int dovi_configure_ext(DOVIContext *s, enum AVCodecID codec_id,
+ const AVDOVIMetadata *metadata,
+ enum AVDOVICompression compression,
+ int strict_std_compliance,
+ int width, int height,
+ AVRational framerate,
+ enum AVPixelFormat pix_format,
+ enum AVColorSpace color_space,
+ enum AVColorPrimaries color_primaries,
+ enum AVColorTransferCharacteristic color_trc,
+ AVPacketSideData **coded_side_data,
+ int *nb_coded_side_data)
{
AVDOVIDecoderConfigurationRecord *cfg;
const AVDOVIRpuDataHeader *hdr = NULL;
@@ -76,7 +84,7 @@ int ff_dovi_configure_ext(DOVIContext *s, AVCodecParameters *codecpar,
compression > AV_DOVI_COMPRESSION_EXTENDED)
return AVERROR(EINVAL);
- switch (codecpar->codec_id) {
+ switch (codec_id) {
case AV_CODEC_ID_AV1: dv_profile = 10; break;
case AV_CODEC_ID_H264: dv_profile = 9; break;
case AV_CODEC_ID_HEVC:
@@ -86,9 +94,9 @@ int ff_dovi_configure_ext(DOVIContext *s, AVCodecParameters *codecpar,
}
/* This is likely to be proprietary IPTPQc2 */
- if (codecpar->color_space == AVCOL_SPC_IPT_C2 ||
- (codecpar->color_space == AVCOL_SPC_UNSPECIFIED &&
- codecpar->color_trc == AVCOL_TRC_UNSPECIFIED))
+ if (color_space == AVCOL_SPC_IPT_C2 ||
+ (color_space == AVCOL_SPC_UNSPECIFIED &&
+ color_trc == AVCOL_TRC_UNSPECIFIED))
dv_profile = 5;
else
dv_profile = 8;
@@ -101,10 +109,10 @@ int ff_dovi_configure_ext(DOVIContext *s, AVCodecParameters *codecpar,
if (strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) {
if (dv_profile == 9) {
- if (codecpar->format != AV_PIX_FMT_YUV420P)
+ if (pix_format != AV_PIX_FMT_YUV420P)
dv_profile = 0;
} else {
- if (codecpar->format != AV_PIX_FMT_YUV420P10)
+ if (pix_format != AV_PIX_FMT_YUV420P10)
dv_profile = 0;
}
}
@@ -131,17 +139,17 @@ int ff_dovi_configure_ext(DOVIContext *s, AVCodecParameters *codecpar,
}
/* fall through */
case 8: /* HEVC (or AV1) with BL compatibility */
- if (codecpar->color_space == AVCOL_SPC_BT2020_NCL &&
- codecpar->color_primaries == AVCOL_PRI_BT2020 &&
- codecpar->color_trc == AVCOL_TRC_SMPTE2084) {
+ if (color_space == AVCOL_SPC_BT2020_NCL &&
+ color_primaries == AVCOL_PRI_BT2020 &&
+ color_trc == AVCOL_TRC_SMPTE2084) {
bl_compat_id = 1;
- } else if (codecpar->color_space == AVCOL_SPC_BT2020_NCL &&
- codecpar->color_primaries == AVCOL_PRI_BT2020 &&
- codecpar->color_trc == AVCOL_TRC_ARIB_STD_B67) {
+ } else if (color_space == AVCOL_SPC_BT2020_NCL &&
+ color_primaries == AVCOL_PRI_BT2020 &&
+ color_trc == AVCOL_TRC_ARIB_STD_B67) {
bl_compat_id = 4;
- } else if (codecpar->color_space == AVCOL_SPC_BT709 &&
- codecpar->color_primaries == AVCOL_PRI_BT709 &&
- codecpar->color_trc == AVCOL_TRC_BT709) {
+ } else if (color_space == AVCOL_SPC_BT709 &&
+ color_primaries == AVCOL_PRI_BT709 &&
+ color_trc == AVCOL_TRC_BT709) {
bl_compat_id = 2;
}
}
@@ -175,9 +183,9 @@ int ff_dovi_configure_ext(DOVIContext *s, AVCodecParameters *codecpar,
}
}
- pps = codecpar->width * codecpar->height;
- if (codecpar->framerate.num) {
- pps = pps * codecpar->framerate.num / codecpar->framerate.den;
+ pps = width * height;
+ if (framerate.num) {
+ pps = pps * framerate.num / framerate.den;
} else {
pps *= 25; /* sanity fallback */
}
@@ -186,7 +194,7 @@ int ff_dovi_configure_ext(DOVIContext *s, AVCodecParameters *codecpar,
for (int i = 1; i < FF_ARRAY_ELEMS(dv_levels); i++) {
if (pps > dv_levels[i].pps)
continue;
- if (codecpar->width > dv_levels[i].width)
+ if (width > dv_levels[i].width)
continue;
/* In theory, we should also test the bitrate when known, and
* distinguish between main and high tier. In practice, just ignore
@@ -199,12 +207,12 @@ int ff_dovi_configure_ext(DOVIContext *s, AVCodecParameters *codecpar,
if (!dv_level) {
if (strict_std_compliance >= FF_COMPLIANCE_STRICT) {
av_log(s->logctx, AV_LOG_ERROR, "Coded PPS (%"PRIu64") and width (%d) "
- "exceed Dolby Vision limitations\n", pps, codecpar->width);
+ "exceed Dolby Vision limitations\n", pps, width);
return AVERROR(EINVAL);
} else {
av_log(s->logctx, AV_LOG_WARNING, "Coded PPS (%"PRIu64") and width (%d) "
"exceed Dolby Vision limitations. Ignoring, resulting file "
- "may be non-conforming.\n", pps, codecpar->width);
+ "may be non-conforming.\n", pps, width);
dv_level = FF_ARRAY_ELEMS(dv_levels) - 1;
}
}
@@ -213,8 +221,8 @@ int ff_dovi_configure_ext(DOVIContext *s, AVCodecParameters *codecpar,
if (!cfg)
return AVERROR(ENOMEM);
- if (!av_packet_side_data_add(&codecpar->coded_side_data,
- &codecpar->nb_coded_side_data,
+ if (!av_packet_side_data_add(coded_side_data,
+ nb_coded_side_data,
AV_PKT_DATA_DOVI_CONF, cfg, cfg_size, 0)) {
av_free(cfg);
return AVERROR(ENOMEM);
@@ -238,19 +246,22 @@ skip:
return 0;
}
+int ff_dovi_configure_from_codedpar(DOVIContext *s, AVCodecParameters *par,
+ const AVDOVIMetadata *metadata,
+ enum AVDOVICompression compression,
+ int strict_std_compliance)
+{
+ return dovi_configure_ext(s, par->codec_id, metadata, compression,
+ strict_std_compliance, par->width, par->height,
+ par->framerate, par->format, par->color_space,
+ par->color_primaries, par->color_trc,
+ &par->coded_side_data, &par->nb_coded_side_data);
+}
+
int ff_dovi_configure(DOVIContext *s, AVCodecContext *avctx)
{
- int ret;
- const AVFrameSideData *sd;
const AVDOVIMetadata *metadata = NULL;
- AVCodecParameters *codecpar = avcodec_parameters_alloc();
- if (!codecpar)
- return AVERROR(ENOMEM);
-
- ret = avcodec_parameters_from_context(codecpar, avctx);
- if (ret < 0)
- goto fail;
-
+ const AVFrameSideData *sd;
sd = av_frame_side_data_get(avctx->decoded_side_data,
avctx->nb_decoded_side_data,
AV_FRAME_DATA_DOVI_METADATA);
@@ -258,16 +269,11 @@ int ff_dovi_configure(DOVIContext *s, AVCodecContext *avctx)
metadata = (const AVDOVIMetadata *) sd->data;
/* Current encoders cannot handle metadata compression during encoding */
- ret = ff_dovi_configure_ext(s, codecpar, metadata, AV_DOVI_COMPRESSION_NONE,
- avctx->strict_std_compliance);
- if (ret < 0)
- goto fail;
-
- ret = avcodec_parameters_to_context(avctx, codecpar);
-
-fail:
- avcodec_parameters_free(&codecpar);
- return ret;
+ return dovi_configure_ext(s, avctx->codec_id, metadata, AV_DOVI_COMPRESSION_NONE,
+ avctx->strict_std_compliance, avctx->width,
+ avctx->height, avctx->framerate, avctx->pix_fmt,
+ avctx->colorspace, avctx->color_primaries, avctx->color_trc,
+ &avctx->coded_side_data, &avctx->nb_coded_side_data);
}
/* Compares only the static DM metadata parts of AVDOVIColorMetadata (excluding
--
2.45.2
From 38ccd832ec6853769ce0eea0cc4a580a26aa0f8d Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
Date: Fri, 30 May 2025 18:27:54 +0200
Subject: [PATCH 2/4] avcodec/dovi_rpuenc: Use av_cold for ff_dovi_configure
init functions
Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
---
libavcodec/dovi_rpuenc.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/libavcodec/dovi_rpuenc.c b/libavcodec/dovi_rpuenc.c
index e04bce6431..4efcbb4ce8 100644
--- a/libavcodec/dovi_rpuenc.c
+++ b/libavcodec/dovi_rpuenc.c
@@ -20,16 +20,17 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include "libavutil/attributes.h"
#include "libavutil/avassert.h"
#include "libavutil/crc.h"
#include "libavutil/mem.h"
+#include "libavutil/refstruct.h"
#include "avcodec.h"
#include "dovi_rpu.h"
#include "itut35.h"
#include "put_bits.h"
#include "put_golomb.h"
-#include "libavutil/refstruct.h"
static const struct {
uint64_t pps; // maximum pixels per second
@@ -52,18 +53,18 @@ static const struct {
[13] = {7680*4320*120u, 7680, 240, 800},
};
-static int dovi_configure_ext(DOVIContext *s, enum AVCodecID codec_id,
- const AVDOVIMetadata *metadata,
- enum AVDOVICompression compression,
- int strict_std_compliance,
- int width, int height,
- AVRational framerate,
- enum AVPixelFormat pix_format,
- enum AVColorSpace color_space,
- enum AVColorPrimaries color_primaries,
- enum AVColorTransferCharacteristic color_trc,
- AVPacketSideData **coded_side_data,
- int *nb_coded_side_data)
+static av_cold int dovi_configure_ext(DOVIContext *s, enum AVCodecID codec_id,
+ const AVDOVIMetadata *metadata,
+ enum AVDOVICompression compression,
+ int strict_std_compliance,
+ int width, int height,
+ AVRational framerate,
+ enum AVPixelFormat pix_format,
+ enum AVColorSpace color_space,
+ enum AVColorPrimaries color_primaries,
+ enum AVColorTransferCharacteristic color_trc,
+ AVPacketSideData **coded_side_data,
+ int *nb_coded_side_data)
{
AVDOVIDecoderConfigurationRecord *cfg;
const AVDOVIRpuDataHeader *hdr = NULL;
@@ -246,10 +247,10 @@ skip:
return 0;
}
-int ff_dovi_configure_from_codedpar(DOVIContext *s, AVCodecParameters *par,
- const AVDOVIMetadata *metadata,
- enum AVDOVICompression compression,
- int strict_std_compliance)
+av_cold int ff_dovi_configure_from_codedpar(DOVIContext *s, AVCodecParameters *par,
+ const AVDOVIMetadata *metadata,
+ enum AVDOVICompression compression,
+ int strict_std_compliance)
{
return dovi_configure_ext(s, par->codec_id, metadata, compression,
strict_std_compliance, par->width, par->height,
@@ -258,7 +259,7 @@ int ff_dovi_configure_from_codedpar(DOVIContext *s, AVCodecParameters *par,
&par->coded_side_data, &par->nb_coded_side_data);
}
-int ff_dovi_configure(DOVIContext *s, AVCodecContext *avctx)
+av_cold int ff_dovi_configure(DOVIContext *s, AVCodecContext *avctx)
{
const AVDOVIMetadata *metadata = NULL;
const AVFrameSideData *sd;
--
2.45.2
From 8753859d10c5c06f662f3290a462c1705307e01e Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
Date: Fri, 30 May 2025 18:35:48 +0200
Subject: [PATCH 3/4] avcodec/dovi_rpuenc: Use av_unreachable() instead of
av_assert0(0)
Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
---
libavcodec/dovi_rpuenc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/libavcodec/dovi_rpuenc.c b/libavcodec/dovi_rpuenc.c
index 4efcbb4ce8..b05ad0a358 100644
--- a/libavcodec/dovi_rpuenc.c
+++ b/libavcodec/dovi_rpuenc.c
@@ -103,9 +103,7 @@ static av_cold int dovi_configure_ext(DOVIContext *s, enum AVCodecID codec_id,
dv_profile = 8;
break;
default:
- /* No other encoder should be calling this! */
- av_assert0(0);
- return AVERROR_BUG;
+ av_unreachable("ff_dovi_configure only used with AV1, H.264 and HEVC");
}
if (strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) {
--
2.45.2
From df7593182958385f33ef91089b04c649bcadf021 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
Date: Fri, 30 May 2025 18:40:58 +0200
Subject: [PATCH 4/4] avcodec/libaomenc: Increase code locality
Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
---
libavcodec/libaomenc.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 9a384fcc39..903292d164 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -681,7 +681,6 @@ static av_cold int aom_init(AVCodecContext *avctx,
struct aom_codec_enc_cfg enccfg = { 0 };
aom_codec_flags_t flags =
(avctx->flags & AV_CODEC_FLAG_PSNR) ? AOM_CODEC_USE_PSNR : 0;
- AVCPBProperties *cpb_props;
int res;
aom_img_fmt_t img_fmt;
aom_codec_caps_t codec_caps = aom_codec_get_caps(iface);
@@ -989,10 +988,6 @@ static av_cold int aom_init(AVCodecContext *avctx,
if (codec_caps & AOM_CODEC_CAP_HIGHBITDEPTH)
ctx->rawimg.bit_depth = enccfg.g_bit_depth;
- cpb_props = ff_encode_add_cpb_side_data(avctx);
- if (!cpb_props)
- return AVERROR(ENOMEM);
-
ctx->dovi.logctx = avctx;
if ((res = ff_dovi_configure(&ctx->dovi, avctx)) < 0)
return res;
@@ -1019,6 +1014,10 @@ static av_cold int aom_init(AVCodecContext *avctx,
return ret;
}
+ AVCPBProperties *cpb_props = ff_encode_add_cpb_side_data(avctx);
+ if (!cpb_props)
+ return AVERROR(ENOMEM);
+
if (enccfg.rc_end_usage == AOM_CBR ||
enccfg.g_pass != AOM_RC_ONE_PASS) {
cpb_props->max_bitrate = avctx->rc_max_rate;
--
2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
