[FFmpeg-devel] [PATCH v2 3/3] avcodec/h264dec: apply H.274 film grain
Eoff, Ullysses A
ullysses.a.eoff at intel.com
Wed Aug 25 21:03:16 EEST 2021
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Niklas Haas
> Sent: Tuesday, August 17, 2021 12:26 PM
> To: ffmpeg-devel at ffmpeg.org
> Cc: Niklas Haas <git at haasn.dev>
> Subject: [FFmpeg-devel] [PATCH v2 3/3] avcodec/h264dec: apply H.274 film grain
>
> From: Niklas Haas <git at haasn.dev>
>
> Because we need access to ref frames without film grain applied, we have
> to add an extra AVFrame to H264Picture to avoid messing with the
> original. This requires some amount of overhead to make the reference
> moves work out, but it allows us to benefit from frame multithreading
> for film grain application "for free".
>
> Unfortunately, this approach requires twice as much RAM to be constantly
> allocated for ref frames, due to the need for an extra buffer per
> H264Picture. In theory, we could get away with freeing up this memory as
> soon as it's no longer needed (since ref frames do not need film grain
> buffers any longer), but trying to call ff_thread_release_buffer() from
> output_frame() conflicts with possible later accesses to that same frame
> and I'm not sure how to synchronize that well.
>
> Tested on all three cases of (no fg), (fg present but exported) and (fg
> present and not exported), with and without threading.
>
> Signed-off-by: Niklas Haas <git at haasn.dev>
> ---
> libavcodec/h264_picture.c | 35 +++++++++++++++++++++++++++++++++--
> libavcodec/h264_slice.c | 16 ++++++++++++++--
> libavcodec/h264dec.c | 39 +++++++++++++++++++++++++++------------
> libavcodec/h264dec.h | 6 ++++++
> 4 files changed, 80 insertions(+), 16 deletions(-)
>
> diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c
> index ff30166b4d..5944798394 100644
> --- a/libavcodec/h264_picture.c
> +++ b/libavcodec/h264_picture.c
> @@ -43,13 +43,14 @@
>
> void ff_h264_unref_picture(H264Context *h, H264Picture *pic)
> {
> - int off = offsetof(H264Picture, tf) + sizeof(pic->tf);
> + int off = offsetof(H264Picture, tf_grain) + sizeof(pic->tf_grain);
> int i;
>
> if (!pic->f || !pic->f->buf[0])
> return;
>
> ff_thread_release_buffer(h->avctx, &pic->tf);
> + ff_thread_release_buffer(h->avctx, &pic->tf_grain);
> av_buffer_unref(&pic->hwaccel_priv_buf);
>
> av_buffer_unref(&pic->qscale_table_buf);
> @@ -93,6 +94,7 @@ static void h264_copy_picture_params(H264Picture *dst, const H264Picture *src)
> dst->mb_width = src->mb_width;
> dst->mb_height = src->mb_height;
> dst->mb_stride = src->mb_stride;
> + dst->needs_fg = src->needs_fg;
> }
>
> int ff_h264_ref_picture(H264Context *h, H264Picture *dst, H264Picture *src)
> @@ -108,6 +110,14 @@ int ff_h264_ref_picture(H264Context *h, H264Picture *dst, H264Picture *src)
> if (ret < 0)
> goto fail;
>
> + if (src->needs_fg) {
> + av_assert0(src->tf_grain.f == src->f_grain);
> + dst->tf_grain.f = dst->f_grain;
> + ret = ff_thread_ref_frame(&dst->tf_grain, &src->tf_grain);
> + if (ret < 0)
> + goto fail;
> + }
> +
> dst->qscale_table_buf = av_buffer_ref(src->qscale_table_buf);
> dst->mb_type_buf = av_buffer_ref(src->mb_type_buf);
> dst->pps_buf = av_buffer_ref(src->pps_buf);
> @@ -159,6 +169,15 @@ int ff_h264_replace_picture(H264Context *h, H264Picture *dst, const H264Picture
> if (ret < 0)
> goto fail;
>
> + if (src->needs_fg) {
> + av_assert0(src->tf_grain.f == src->f_grain);
> + dst->tf_grain.f = dst->f_grain;
> + ff_thread_release_buffer(h->avctx, &dst->tf_grain);
> + ret = ff_thread_ref_frame(&dst->tf_grain, &src->tf_grain);
> + if (ret < 0)
> + goto fail;
> + }
> +
> ret = av_buffer_replace(&dst->qscale_table_buf, src->qscale_table_buf);
> ret |= av_buffer_replace(&dst->mb_type_buf, src->mb_type_buf);
> ret |= av_buffer_replace(&dst->pps_buf, src->pps_buf);
> @@ -212,6 +231,7 @@ void ff_h264_set_erpic(ERPicture *dst, H264Picture *src)
> int ff_h264_field_end(H264Context *h, H264SliceContext *sl, int in_setup)
> {
> AVCodecContext *const avctx = h->avctx;
> + H264Picture *cur = h->cur_pic_ptr;
> int err = 0;
> h->mb_y = 0;
>
> @@ -230,10 +250,21 @@ int ff_h264_field_end(H264Context *h, H264SliceContext *sl, int in_setup)
> if (err < 0)
> av_log(avctx, AV_LOG_ERROR,
> "hardware accelerator failed to decode picture\n");
> + } else if (!in_setup && cur->needs_fg) {
> + AVFrameSideData *sd = av_frame_get_side_data(cur->f, AV_FRAME_DATA_FILM_GRAIN_PARAMS);
> + av_assert0(sd); // always present if `cur->needs_fg`
> + 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));
> + cur->needs_fg = 0;
> + err = 0;
> + }
> }
>
> if (!in_setup && !h->droppable)
> - ff_thread_report_progress(&h->cur_pic_ptr->tf, INT_MAX,
> + ff_thread_report_progress(&cur->tf, INT_MAX,
> h->picture_structure == PICT_BOTTOM_FIELD);
> emms_c();
>
> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> index 9244d2d5dd..98ca8836db 100644
> --- a/libavcodec/h264_slice.c
> +++ b/libavcodec/h264_slice.c
> @@ -197,6 +197,16 @@ static int alloc_picture(H264Context *h, H264Picture *pic)
> if (ret < 0)
> goto fail;
>
> + if (pic->needs_fg) {
> + pic->tf_grain.f = pic->f_grain;
> + pic->f_grain->format = pic->f->format;
> + pic->f_grain->width = pic->f->width;
> + pic->f_grain->height = pic->f->height;
> + ret = ff_thread_get_buffer(h->avctx, &pic->tf_grain, 0);
> + if (ret < 0)
> + goto fail;
> + }
> +
> if (h->avctx->hwaccel) {
> const AVHWAccel *hwaccel = h->avctx->hwaccel;
> av_assert0(!pic->hwaccel_picture_private);
> @@ -517,6 +527,9 @@ 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.film_grain_characteristics.present &&
> + !(h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN);
> +
> if ((ret = alloc_picture(h, pic)) < 0)
> return ret;
>
> @@ -1328,8 +1341,7 @@ static int h264_export_frame_props(H264Context *h)
> }
> h->sei.unregistered.nb_buf_ref = 0;
>
> - if (h->sei.film_grain_characteristics.present &&
> - (h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN)) {
> + if (h->sei.film_grain_characteristics.present) {
> H264SEIFilmGrainCharacteristics *fgc = &h->sei.film_grain_characteristics;
> AVFilmGrainParams *fgp = av_film_grain_params_create_side_data(out);
> if (!fgp)
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index dc99ee995e..b88ca54f05 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -275,9 +275,22 @@ int ff_h264_slice_context_init(H264Context *h, H264SliceContext *sl)
> return 0;
> }
>
> +static int h264_init_pic(H264Picture *pic)
> +{
> + pic->f = av_frame_alloc();
> + if (!pic->f)
> + return AVERROR(ENOMEM);
> +
> + pic->f_grain = av_frame_alloc();
> + if (!pic->f_grain)
> + return AVERROR(ENOMEM);
> +
> + return 0;
> +}
> +
> static int h264_init_context(AVCodecContext *avctx, H264Context *h)
> {
> - int i;
> + int i, ret;
>
> h->avctx = avctx;
> h->cur_chroma_format_idc = -1;
> @@ -308,18 +321,15 @@ static int h264_init_context(AVCodecContext *avctx, H264Context *h)
> }
>
> for (i = 0; i < H264_MAX_PICTURE_COUNT; i++) {
> - h->DPB[i].f = av_frame_alloc();
> - if (!h->DPB[i].f)
> - return AVERROR(ENOMEM);
> + if ((ret = h264_init_pic(&h->DPB[i])) < 0)
> + return ret;
> }
>
> - h->cur_pic.f = av_frame_alloc();
> - if (!h->cur_pic.f)
> - return AVERROR(ENOMEM);
> + if ((ret = h264_init_pic(&h->cur_pic)) < 0)
> + return ret;
>
> - h->last_pic_for_ec.f = av_frame_alloc();
> - if (!h->last_pic_for_ec.f)
> - return AVERROR(ENOMEM);
> + if ((ret = h264_init_pic(&h->last_pic_for_ec)) < 0)
> + return ret;
>
> for (i = 0; i < h->nb_slice_ctx; i++)
> h->slice_ctx[i].h264 = h;
> @@ -837,13 +847,15 @@ static int h264_export_enc_params(AVFrame *f, H264Picture *p)
>
> static int output_frame(H264Context *h, AVFrame *dst, H264Picture *srcp)
> {
> - AVFrame *src = srcp->f;
> int ret;
>
> - ret = av_frame_ref(dst, src);
> + ret = av_frame_ref(dst, srcp->needs_fg ? srcp->f_grain : srcp->f);
> if (ret < 0)
> return ret;
>
> + if (srcp->needs_fg && (ret = av_frame_copy_props(dst, srcp->f)) < 0)
> + return ret;
> +
> av_dict_set(&dst->metadata, "stereo_mode", ff_h264_sei_stereo_mode(&h->sei.frame_packing), 0);
>
> if (srcp->sei_recovery_frame_cnt == 0)
> @@ -855,6 +867,9 @@ static int output_frame(H264Context *h, AVFrame *dst, H264Picture *srcp)
> goto fail;
> }
>
> + if (!(h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN))
> + av_frame_remove_side_data(dst, AV_FRAME_DATA_FILM_GRAIN_PARAMS);
> +
> return 0;
> fail:
> av_frame_unref(dst);
> diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
> index 7c419de051..87c4e4e539 100644
> --- a/libavcodec/h264dec.h
> +++ b/libavcodec/h264dec.h
> @@ -43,6 +43,7 @@
> #include "h264dsp.h"
> #include "h264pred.h"
> #include "h264qpel.h"
> +#include "h274.h"
> #include "internal.h"
> #include "mpegutils.h"
> #include "parser.h"
> @@ -130,6 +131,9 @@ typedef struct H264Picture {
> AVFrame *f;
> ThreadFrame tf;
>
> + AVFrame *f_grain;
> + ThreadFrame tf_grain;
> +
> AVBufferRef *qscale_table_buf;
> int8_t *qscale_table;
>
> @@ -162,6 +166,7 @@ typedef struct H264Picture {
> int recovered; ///< picture at IDR or recovery point + recovery count
> int invalid_gap;
> int sei_recovery_frame_cnt;
> + int needs_fg; ///< whether picture needs film grain synthesis (see `f_grain`)
>
> AVBufferRef *pps_buf;
> const PPS *pps;
> @@ -349,6 +354,7 @@ typedef struct H264Context {
> H264DSPContext h264dsp;
> H264ChromaContext h264chroma;
> H264QpelContext h264qpel;
> + H274FilmGrainDatabase h274db;
>
> H264Picture DPB[H264_MAX_PICTURE_COUNT];
> H264Picture *cur_pic_ptr;
> --
> 2.32.0
>
This causes regression reported here https://trac.ffmpeg.org/ticket/9389
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list