[FFmpeg-devel] libdav1d: use film grain export flag to export AVFilmGrainParams side data
James Almer
jamrial at gmail.com
Thu Nov 12 16:51:55 EET 2020
On 11/12/2020 11:34 AM, Lynne wrote:
> Nov 12, 2020, 14:49 by jamrial at gmail.com:
>
>> On 11/12/2020 8:59 AM, Lynne wrote:
>>
>>> + if (dav1d->apply_grain < 0 &&
>>> + c->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN)
>>> + dav1d->apply_grain = 0;
>>>
>>
>> Don't overwrite a Libdav1dContext field that was set by an AVOption. Instead, just change the check below and set s.apply_grain directly based on both user options (film_grain and export_side_data).
>>
>>> if (dav1d->apply_grain >= 0)
>>> s.apply_grain = dav1d->apply_grain;
>>> @@ -395,6 +401,64 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>> break;
>>> }
>>> }
>>> + if ((!dav1d->apply_grain || (c->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN)) &&
>>>
>>
>> With the above you should in theory be able to just check for the export_side_data flag and frame_hdr->film_grain.present here.
>>
>
> Changed to:
>> if (dav1d->apply_grain >= 0)
>> s.apply_grain = dav1d->apply_grain;
>> + else if (c->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN)
>> + s.apply_grain = 0;
>
>> + if (p->frame_hdr->film_grain.present && (!dav1d->apply_grain ||
>> + (c->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN)) {
>
> New version attached.
> From 520ca00eb1e5ef7ea56fffc8d5449114d200ee3e Mon Sep 17 00:00:00 2001
> From: Lynne <dev at lynne.ee>
> Date: Thu, 12 Nov 2020 12:48:20 +0100
> Subject: [PATCH] libdav1d: use film grain export flag to export
> AVFilmGrainParams side data
>
> This patch is relatively straightforward with one exception:
> the decoder option flag.
> The option was introduced to troubleshoot but its existence is conflicting
> and redundant now that we have a codec-generic flag.
> Hence this patch deprecates it.
>
> The way it interacts with AV_CODEC_EXPORT_DATA_FILM_GRAIN is as follows:
>
> If filmgrain is unset and AV_CODEC_EXPORT_DATA_FILM_GRAIN is
> present, disable film grain application and export side data.
>
> If filmgrain is set to 0, disable film grain and export side data.
>
> If filmgrain is set to 1, apply film grain but export side data if
> the AV_CODEC_EXPORT_DATA_FILM_GRAIN flag is set. This may result in
> double film grain application, but the user has requested it by setting
> both.
> ---
> libavcodec/libdav1d.c | 65 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> index 3af7ef4edc..319366c0bd 100644
> --- a/libavcodec/libdav1d.c
> +++ b/libavcodec/libdav1d.c
> @@ -22,6 +22,7 @@
> #include <dav1d/dav1d.h>
>
> #include "libavutil/avassert.h"
> +#include "libavutil/film_grain_params.h"
> #include "libavutil/mastering_display_metadata.h"
> #include "libavutil/imgutils.h"
> #include "libavutil/opt.h"
> @@ -37,6 +38,7 @@ typedef struct Libdav1dContext {
> Dav1dContext *c;
> AVBufferPool *pool;
> int pool_size;
> + AVBufferRef *film_grain_params;
>
> Dav1dData data;
> int tile_threads;
> @@ -137,6 +139,8 @@ static av_cold int libdav1d_init(AVCodecContext *c)
> s.frame_size_limit = c->max_pixels;
> if (dav1d->apply_grain >= 0)
> s.apply_grain = dav1d->apply_grain;
> + else if (c->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN)
> + s.apply_grain = 0;
>
> s.all_layers = dav1d->all_layers;
> if (dav1d->operating_point >= 0)
> @@ -395,6 +399,64 @@ FF_ENABLE_DEPRECATION_WARNINGS
> break;
> }
> }
> + if (p->frame_hdr->film_grain.present && (!dav1d->apply_grain ||
> + (c->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN)) {
You're exporting the data here even when it's not requested with
export_side_data.
This should either export it, or tell dav1d to apply it, or neither. But
not both. That's how you defined AV_CODEC_EXPORT_DATA_FILM_GRAIN.
That means you also need to decide what should take priority when the
user sets apply_grain to 1 and requests to export the side data as well
during the deprecation time until the apply_grain AVOption is removed.
Also please check my review for the other patch before sending a new
version of this one.
> + if (p->frame_hdr->film_grain.update) {
> + AVFilmGrainParams *fgp;
> +
> + av_buffer_unref(&dav1d->film_grain_params);
> + dav1d->film_grain_params = av_film_grain_params_buffer_alloc();
> + if (!dav1d->film_grain_params) {
> + res = AVERROR(ENOMEM);
> + goto fail;
> + }
> +
> + fgp = (AVFilmGrainParams *)dav1d->film_grain_params->data;
> +
> + fgp->type = AV_FILM_GRAM_PARAMS_AV1;
> + fgp->seed = p->frame_hdr->film_grain.data.seed;
> + fgp->num_y_points = p->frame_hdr->film_grain.data.num_y_points;
> + fgp->chroma_scaling_from_luma = p->frame_hdr->film_grain.data.chroma_scaling_from_luma;
> + fgp->scaling_shift = p->frame_hdr->film_grain.data.scaling_shift;
> + fgp->ar_coeff_lag = p->frame_hdr->film_grain.data.ar_coeff_lag;
> + fgp->ar_coeff_shift = p->frame_hdr->film_grain.data.ar_coeff_shift;
> + fgp->grain_scale_shift = p->frame_hdr->film_grain.data.grain_scale_shift;
> + fgp->overlap_flag = p->frame_hdr->film_grain.data.overlap_flag;
> + fgp->clip_to_limited_range = p->frame_hdr->film_grain.data.clip_to_restricted_range;
> +
> + memcpy(&fgp->y_points, &p->frame_hdr->film_grain.data.y_points,
> + sizeof(fgp->y_points));
> + memcpy(&fgp->num_uv_points, &p->frame_hdr->film_grain.data.num_uv_points,
> + sizeof(fgp->num_uv_points));
> + memcpy(&fgp->uv_points, &p->frame_hdr->film_grain.data.uv_points,
> + sizeof(fgp->uv_points));
> + memcpy(&fgp->ar_coeffs_y, &p->frame_hdr->film_grain.data.ar_coeffs_y,
> + sizeof(fgp->ar_coeffs_y));
> + memcpy(&fgp->ar_coeffs_uv, &p->frame_hdr->film_grain.data.ar_coeffs_uv,
> + sizeof(fgp->ar_coeffs_uv));
> + memcpy(&fgp->uv_mult, &p->frame_hdr->film_grain.data.uv_mult,
> + sizeof(fgp->uv_mult));
> + memcpy(&fgp->uv_mult_luma, &p->frame_hdr->film_grain.data.uv_luma_mult,
> + sizeof(fgp->uv_mult_luma));
> + memcpy(&fgp->uv_offset, &p->frame_hdr->film_grain.data.uv_offset,
> + sizeof(fgp->uv_offset));
> + }
> +
> + if (dav1d->film_grain_params) {
> + AVBufferRef *fgp_ref = av_buffer_ref(dav1d->film_grain_params);
> + if (!fgp_ref) {
> + res = AVERROR(ENOMEM);
> + goto fail;
> + }
> + if (!av_frame_new_side_data_from_buf(frame,
> + AV_FRAME_DATA_FILM_GRAIN_PARAMS,
> + fgp_ref)) {
> + av_buffer_unref(&fgp_ref);
> + res = AVERROR(ENOMEM);
> + goto fail;
> + }
> + }
> + }
>
> res = 0;
> fail:
> @@ -408,6 +470,7 @@ static av_cold int libdav1d_close(AVCodecContext *c)
> {
> Libdav1dContext *dav1d = c->priv_data;
>
> + av_buffer_unref(&dav1d->film_grain_params);
> av_buffer_pool_uninit(&dav1d->pool);
> dav1d_data_unref(&dav1d->data);
> dav1d_close(&dav1d->c);
> @@ -420,7 +483,7 @@ static av_cold int libdav1d_close(AVCodecContext *c)
> static const AVOption libdav1d_options[] = {
> { "tilethreads", "Tile threads", OFFSET(tile_threads), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_TILE_THREADS, VD },
> { "framethreads", "Frame threads", OFFSET(frame_threads), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, DAV1D_MAX_FRAME_THREADS, VD },
> - { "filmgrain", "Apply Film Grain", OFFSET(apply_grain), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD },
> + { "filmgrain", "Apply Film Grain", OFFSET(apply_grain), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD | AV_OPT_FLAG_DEPRECATED },
> { "oppoint", "Select an operating point of the scalable bitstream", OFFSET(operating_point), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 31, VD },
> { "alllayers", "Output all spatial layers", OFFSET(all_layers), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD },
> { NULL }
> --
> 2.29.2
>
More information about the ffmpeg-devel
mailing list