From 19b550d20a2f00e776c3e17b56b3ec55abf71752 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt 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 --- 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