From 91b6ba653e44f7e911fac848a32c9c431d47c433 Mon Sep 17 00:00:00 2001 From: James Almer Date: Fri, 25 Oct 2024 12:24:04 -0300 Subject: [PATCH] Revert "avcodec/h2645: allocate film grain metadata dynamically" AVFilmGrainAFGS1Params, the offending struct, is using sizeof(AVFilmGrainParams) when it should not. This change also forgot to make the necessary changes to the frame threading sync code. Both of these will be fixed by the following commit. H274FilmGrainDatabase will be handled later. This reverts commit 08b1bffa49715a9615acc025dfbea252d8409e1f. Signed-off-by: James Almer --- libavcodec/h2645_sei.c | 30 ++++++++---------------------- libavcodec/h2645_sei.h | 6 ++---- libavcodec/h264_picture.c | 11 ++--------- libavcodec/h264_sei.c | 1 + libavcodec/h264_slice.c | 5 +---- libavcodec/h264dec.c | 2 -- libavcodec/h264dec.h | 2 +- libavcodec/hevc/hevcdec.c | 20 +++++++------------- libavcodec/hevc/hevcdec.h | 2 +- 9 files changed, 23 insertions(+), 56 deletions(-) diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c index 33551f5406..c46a563308 100644 --- a/libavcodec/h2645_sei.c +++ b/libavcodec/h2645_sei.c @@ -245,12 +245,7 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb, provider_oriented_code = bytestream2_get_byteu(gb); if (provider_oriented_code == aom_grain_provider_oriented_code) { - if (!h->aom_film_grain) { - h->aom_film_grain = av_mallocz(sizeof(*h->aom_film_grain)); - if (!h->aom_film_grain) - return AVERROR(ENOMEM); - } - return ff_aom_parse_film_grain_sets(h->aom_film_grain, + return ff_aom_parse_film_grain_sets(&h->aom_film_grain, gb->buffer, bytestream2_get_bytes_left(gb)); } @@ -499,12 +494,7 @@ int ff_h2645_sei_message_decode(H2645SEI *h, enum SEIType type, case SEI_TYPE_DISPLAY_ORIENTATION: return decode_display_orientation(&h->display_orientation, gb); case SEI_TYPE_FILM_GRAIN_CHARACTERISTICS: - if (!h->film_grain_characteristics) { - h->film_grain_characteristics = av_mallocz(sizeof(*h->film_grain_characteristics)); - if (!h->film_grain_characteristics) - return AVERROR(ENOMEM); - } - return decode_film_grain_characteristics(h->film_grain_characteristics, codec_id, gb); + return decode_film_grain_characteristics(&h->film_grain_characteristics, codec_id, gb); case SEI_TYPE_FRAME_PACKING_ARRANGEMENT: return decode_frame_packing_arrangement(&h->frame_packing, gb, codec_id); case SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS: @@ -821,8 +811,8 @@ int ff_h2645_sei_to_frame(AVFrame *frame, H2645SEI *sei, return ret; } - if (sei->film_grain_characteristics && sei->film_grain_characteristics->present) { - H2645SEIFilmGrainCharacteristics *fgc = sei->film_grain_characteristics; + if (sei->film_grain_characteristics.present) { + H2645SEIFilmGrainCharacteristics *fgc = &sei->film_grain_characteristics; AVFilmGrainParams *fgp = av_film_grain_params_create_side_data(frame); AVFilmGrainH274Params *h274; @@ -894,11 +884,9 @@ FF_ENABLE_DEPRECATION_WARNINGS } #if CONFIG_HEVC_SEI - if (sei->aom_film_grain) { - ret = ff_aom_attach_film_grain_sets(sei->aom_film_grain, frame); - if (ret < 0) - return ret; - } + ret = ff_aom_attach_film_grain_sets(&sei->aom_film_grain, frame); + if (ret < 0) + return ret; #endif return 0; @@ -925,7 +913,5 @@ void ff_h2645_sei_reset(H2645SEI *s) s->ambient_viewing_environment.present = 0; s->mastering_display.present = 0; s->content_light.present = 0; - - av_freep(&s->film_grain_characteristics); - av_freep(&s->aom_film_grain); + s->aom_film_grain.enable = 0; } diff --git a/libavcodec/h2645_sei.h b/libavcodec/h2645_sei.h index 8bcdc2bc5f..598f78b585 100644 --- a/libavcodec/h2645_sei.h +++ b/libavcodec/h2645_sei.h @@ -135,13 +135,11 @@ typedef struct H2645SEI { H2645SEIFramePacking frame_packing; H2645SEIDisplayOrientation display_orientation; H2645SEIAlternativeTransfer alternative_transfer; + H2645SEIFilmGrainCharacteristics film_grain_characteristics; H2645SEIAmbientViewingEnvironment ambient_viewing_environment; H2645SEIMasteringDisplay mastering_display; H2645SEIContentLight content_light; - - // Dynamic allocations due to large size. - H2645SEIFilmGrainCharacteristics* film_grain_characteristics; - AVFilmGrainAFGS1Params* aom_film_grain; + AVFilmGrainAFGS1Params aom_film_grain; } H2645SEI; enum { diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c index 371a794ec2..3234141dbd 100644 --- a/libavcodec/h264_picture.c +++ b/libavcodec/h264_picture.c @@ -27,7 +27,6 @@ #include "libavutil/avassert.h" #include "libavutil/emms.h" -#include "libavutil/mem.h" #include "error_resilience.h" #include "avcodec.h" #include "h264dec.h" @@ -213,15 +212,9 @@ int ff_h264_field_end(H264Context *h, H264SliceContext *sl, int in_setup) const AVFrameSideData *sd = av_frame_get_side_data(cur->f, AV_FRAME_DATA_FILM_GRAIN_PARAMS); err = AVERROR_INVALIDDATA; - if (sd) { // a decoding error may have happened before the side data could be allocated - if (!h->h274db) { - h->h274db = av_mallocz(sizeof(*h->h274db)); - if (!h->h274db) - return AVERROR(ENOMEM); - } - err = ff_h274_apply_film_grain(cur->f_grain, cur->f, h->h274db, + if (sd) // a decoding error may have happened before the side data could be allocated + err = ff_h274_apply_film_grain(cur->f_grain, cur->f, &h->h274db, (AVFilmGrainParams *) sd->data); - } if (err < 0) { av_log(h->avctx, AV_LOG_WARNING, "Failed synthesizing film " "grain, ignoring: %s\n", av_err2str(err)); diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c index 15a5232209..8d6dc77943 100644 --- a/libavcodec/h264_sei.c +++ b/libavcodec/h264_sei.c @@ -55,6 +55,7 @@ void ff_h264_sei_uninit(H264SEIContext *h) h->picture_timing.present = 0; h->buffering_period.present = 0; h->common.frame_packing.present = 0; + h->common.film_grain_characteristics.present = 0; h->common.display_orientation.present = 0; h->common.afd.present = 0; diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c index 84595b1a8b..a66b75ca80 100644 --- a/libavcodec/h264_slice.c +++ b/libavcodec/h264_slice.c @@ -516,10 +516,7 @@ static int h264_frame_start(H264Context *h) pic->f->crop_top = h->crop_top; pic->f->crop_bottom = h->crop_bottom; - pic->needs_fg = - h->sei.common.film_grain_characteristics && - h->sei.common.film_grain_characteristics->present && - !h->avctx->hwaccel && + pic->needs_fg = h->sei.common.film_grain_characteristics.present && !h->avctx->hwaccel && !(h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN); if ((ret = alloc_picture(h, pic)) < 0) diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c index af0913ca2c..0154fe17b6 100644 --- a/libavcodec/h264dec.c +++ b/libavcodec/h264dec.c @@ -156,8 +156,6 @@ void ff_h264_free_tables(H264Context *h) av_freep(&h->mb2b_xy); av_freep(&h->mb2br_xy); - av_freep(&h->h274db); - ff_refstruct_pool_uninit(&h->qscale_table_pool); ff_refstruct_pool_uninit(&h->mb_type_pool); ff_refstruct_pool_uninit(&h->motion_val_pool); diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h index bbc2a185eb..ccd7583bf4 100644 --- a/libavcodec/h264dec.h +++ b/libavcodec/h264dec.h @@ -344,7 +344,7 @@ typedef struct H264Context { H264DSPContext h264dsp; H264ChromaContext h264chroma; H264QpelContext h264qpel; - H274FilmGrainDatabase* h274db; // Dyanmic allocation due to large size. + H274FilmGrainDatabase h274db; H264Picture DPB[H264_MAX_PICTURE_COUNT]; H264Picture *cur_pic_ptr; diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c index 1ea8df0fa0..4e3fa39e49 100644 --- a/libavcodec/hevc/hevcdec.c +++ b/libavcodec/hevc/hevcdec.c @@ -412,8 +412,8 @@ static int export_stream_params_from_sei(HEVCContext *s) avctx->color_trc = s->sei.common.alternative_transfer.preferred_transfer_characteristics; } - if ((s->sei.common.film_grain_characteristics && s->sei.common.film_grain_characteristics->present) || - (s->sei.common.aom_film_grain && s->sei.common.aom_film_grain->enable)) + if (s->sei.common.film_grain_characteristics.present || + s->sei.common.aom_film_grain.enable) avctx->properties |= FF_CODEC_PROPERTY_FILM_GRAIN; return 0; @@ -3267,8 +3267,8 @@ static int hevc_frame_start(HEVCContext *s, HEVCLayerContext *l, else s->cur_frame->f->flags &= ~AV_FRAME_FLAG_KEY; - s->cur_frame->needs_fg = ((s->sei.common.film_grain_characteristics && s->sei.common.film_grain_characteristics->present) || - (s->sei.common.aom_film_grain && s->sei.common.aom_film_grain->enable)) && + s->cur_frame->needs_fg = (s->sei.common.film_grain_characteristics.present || + s->sei.common.aom_film_grain.enable) && !(s->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN) && !s->avctx->hwaccel; @@ -3277,8 +3277,8 @@ static int hevc_frame_start(HEVCContext *s, HEVCLayerContext *l, goto fail; if (s->cur_frame->needs_fg && - (s->sei.common.film_grain_characteristics && s->sei.common.film_grain_characteristics->present && - !ff_h274_film_grain_params_supported(s->sei.common.film_grain_characteristics->model_id, + (s->sei.common.film_grain_characteristics.present && + !ff_h274_film_grain_params_supported(s->sei.common.film_grain_characteristics.model_id, s->cur_frame->f->format) || !av_film_grain_params_select(s->cur_frame->f))) { av_log_once(s->avctx, AV_LOG_WARNING, AV_LOG_DEBUG, &s->film_grain_warning_shown, @@ -3415,13 +3415,8 @@ static int hevc_frame_end(HEVCContext *s, HEVCLayerContext *l) av_assert0(0); return AVERROR_BUG; case AV_FILM_GRAIN_PARAMS_H274: - if (!s->h274db) { - s->h274db = av_mallocz(sizeof(*s->h274db)); - if (!s->h274db) - return AVERROR(ENOMEM); - } ret = ff_h274_apply_film_grain(out->frame_grain, out->f, - s->h274db, fgp); + &s->h274db, fgp); break; case AV_FILM_GRAIN_PARAMS_AV1: ret = ff_aom_apply_film_grain(out->frame_grain, out->f, fgp); @@ -3849,7 +3844,6 @@ static av_cold int hevc_decode_free(AVCodecContext *avctx) av_buffer_unref(&s->rpu_buf); av_freep(&s->md5_ctx); - av_freep(&s->h274db); ff_container_fifo_free(&s->output_fifo); diff --git a/libavcodec/hevc/hevcdec.h b/libavcodec/hevc/hevcdec.h index 73b792c880..473709b4e8 100644 --- a/libavcodec/hevc/hevcdec.h +++ b/libavcodec/hevc/hevcdec.h @@ -531,7 +531,7 @@ typedef struct HEVCContext { HEVCDSPContext hevcdsp; VideoDSPContext vdsp; BswapDSPContext bdsp; - H274FilmGrainDatabase* h274db; // Dynamically allocated due to large size. + H274FilmGrainDatabase h274db; /** used on BE to byteswap the lines for checksumming */ uint8_t *checksum_buf;