[FFmpeg-devel] [PATCH v3] avcodec/h264dec: apply H.274 film grain
James Almer
jamrial at gmail.com
Thu Aug 26 00:01:43 EEST 2021
On 8/25/2021 3:06 PM, Eoff, Ullysses A wrote:
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Niklas Haas
>> Sent: Tuesday, August 17, 2021 12:55 PM
>> To: ffmpeg-devel at ffmpeg.org
>> Cc: Niklas Haas <git at haasn.dev>
>> Subject: [FFmpeg-devel] [PATCH v3] 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 | 55 ++++++++++++++++++++++++++-------------
>> libavcodec/h264dec.h | 6 +++++
>> 4 files changed, 90 insertions(+), 22 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..a64c93eca4 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;
>> @@ -327,6 +337,13 @@ static int h264_init_context(AVCodecContext *avctx, H264Context *h)
>> return 0;
>> }
>>
>> +static void h264_free_pic(H264Context *h, H264Picture *pic)
>> +{
>> + ff_h264_unref_picture(h, pic);
>> + av_frame_free(&pic->f);
>> + av_frame_free(&pic->f_grain);
>> +}
>> +
>> static av_cold int h264_decode_end(AVCodecContext *avctx)
>> {
>> H264Context *h = avctx->priv_data;
>> @@ -336,8 +353,7 @@ static av_cold int h264_decode_end(AVCodecContext *avctx)
>> ff_h264_free_tables(h);
>>
>> for (i = 0; i < H264_MAX_PICTURE_COUNT; i++) {
>> - ff_h264_unref_picture(h, &h->DPB[i]);
>> - av_frame_free(&h->DPB[i].f);
>> + h264_free_pic(h, &h->DPB[i]);
>> }
>> memset(h->delayed_pic, 0, sizeof(h->delayed_pic));
>>
>> @@ -351,10 +367,8 @@ static av_cold int h264_decode_end(AVCodecContext *avctx)
>>
>> ff_h2645_packet_uninit(&h->pkt);
>>
>> - ff_h264_unref_picture(h, &h->cur_pic);
>> - av_frame_free(&h->cur_pic.f);
>> - ff_h264_unref_picture(h, &h->last_pic_for_ec);
>> - av_frame_free(&h->last_pic_for_ec.f);
>> + h264_free_pic(h, &h->cur_pic);
>> + h264_free_pic(h, &h->last_pic_for_ec);
>>
>> return 0;
>> }
>> @@ -837,13 +851,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 +871,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
>>
>
> Sorry, replied to v2 first...
>
> This causes regression reported here https://trac.ffmpeg.org/ticket/9389
Should be fixed.
More information about the ffmpeg-devel
mailing list