[FFmpeg-devel] [PATCH v2 1/3] libavutil: introduce AVFilmGrainParams side data
Lynne
dev at lynne.ee
Mon Nov 23 14:08:36 EET 2020
Nov 22, 2020, 21:28 by jamrial at gmail.com:
> On 11/12/2020 4:28 PM, Lynne wrote:
>
>> Nov 12, 2020, 19:46 by jamrial at gmail.com:
>>
>>> On 11/12/2020 2:42 PM, Lynne wrote:
>>>
>>>> This patch introduces a new frame side data type AVFilmGrainParams for use
>>>> with video codecs which are able to use it.
>>>>
>>>> It is generalized rather than being AV1 specific as AV2 is expected to carry
>>>> the same data, as well as the fact there already exist rarely-used specifications
>>>> for both H.264 and HEVC.
>>>>
>>>> Patch attached.
>>>>
>>>> From 522e3a80f44129c98f0c564d44815dbe6a740568 Mon Sep 17 00:00:00 2001
>>>> From: Lynne <dev at lynne.ee>
>>>> Date: Thu, 12 Nov 2020 12:44:30 +0100
>>>> Subject: [PATCH v2 1/3] libavutil: introduce AVFilmGrainParams side data
>>>>
>>>> This patch introduces a new frame side data type AVFilmGrainParams for use
>>>> with video codecs which are able to use it.
>>>>
>>>> It is generalized rather than being AV1 specific as AV2 is expected to carry
>>>> the same data, as well as the fact there already exist rarely-used specifications
>>>> for both H.264 and HEVC.
>>>> ---
>>>> doc/APIchanges | 4 +
>>>> libavutil/Makefile | 2 +
>>>> libavutil/film_grain_params.c | 42 +++++++++
>>>> libavutil/film_grain_params.h | 159 ++++++++++++++++++++++++++++++++++
>>>> libavutil/frame.c | 1 +
>>>> libavutil/frame.h | 6 ++
>>>> libavutil/version.h | 2 +-
>>>> 7 files changed, 215 insertions(+), 1 deletion(-)
>>>> create mode 100644 libavutil/film_grain_params.c
>>>> create mode 100644 libavutil/film_grain_params.h
>>>>
>>>> diff --git a/doc/APIchanges b/doc/APIchanges
>>>> index b70c78a483..41248724d9 100644
>>>> --- a/doc/APIchanges
>>>> +++ b/doc/APIchanges
>>>> @@ -15,6 +15,10 @@ libavutil: 2017-10-21
>>>> API changes, most recent first:
>>>> +2020-xx-xx - xxxxxxxxxx - lavu 56.61.100 - film_grain_params.h
>>>> + Adds a new API for extracting codec film grain parameters as side data.
>>>> + Adds a new AVFrameSideDataType entry AV_FRAME_DATA_FILM_GRAIN_PARAMS for it.
>>>> +
>>>> 2020-xx-xx - xxxxxxxxxx - lavf 58.64.100 - avformat.h
>>>> Add AVSTREAM_EVENT_FLAG_NEW_PACKETS.
>>>> diff --git a/libavutil/Makefile b/libavutil/Makefile
>>>> index 9b08372eb2..27bafe9e12 100644
>>>> --- a/libavutil/Makefile
>>>> +++ b/libavutil/Makefile
>>>> @@ -84,6 +84,7 @@ HEADERS = adler32.h \
>>>> xtea.h \
>>>> tea.h \
>>>> tx.h \
>>>> + film_grain_params.h \
>>>> HEADERS-$(CONFIG_LZO) += lzo.h
>>>> @@ -170,6 +171,7 @@ OBJS = adler32.o \
>>>> tx_double.o \
>>>> tx_int32.o \
>>>> video_enc_params.o \
>>>> + film_grain_params.o \
>>>> OBJS-$(CONFIG_CUDA) += hwcontext_cuda.o
>>>> diff --git a/libavutil/film_grain_params.c b/libavutil/film_grain_params.c
>>>> new file mode 100644
>>>> index 0000000000..930d23c7fe
>>>> --- /dev/null
>>>> +++ b/libavutil/film_grain_params.c
>>>> @@ -0,0 +1,42 @@
>>>> +/**
>>>> + * This file is part of FFmpeg.
>>>> + *
>>>> + * FFmpeg is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU Lesser General Public
>>>> + * License as published by the Free Software Foundation; either
>>>> + * version 2.1 of the License, or (at your option) any later version.
>>>> + *
>>>> + * FFmpeg is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>>> + * Lesser General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU Lesser General Public
>>>> + * License along with FFmpeg; if not, write to the Free Software
>>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>>>> + */
>>>> +
>>>> +#include "film_grain_params.h"
>>>> +
>>>> +AVFilmGrainParams *av_film_grain_params_alloc(size_t *size)
>>>> +{
>>>> + AVFilmGrainParams *params = av_mallocz(sizeof(AVFilmGrainParams));
>>>> +
>>>> + if (size)
>>>> + *size = sizeof(*params);
>>>> +
>>>> + return params;
>>>> +}
>>>> +
>>>> +AVFilmGrainParams *av_film_grain_params_create_side_data(AVFrame *frame)
>>>> +{
>>>> + AVFrameSideData *side_data = av_frame_new_side_data(frame,
>>>> + AV_FRAME_DATA_FILM_GRAIN_PARAMS,
>>>> + sizeof(AVFilmGrainParams));
>>>> + if (!side_data)
>>>> + return NULL;
>>>> +
>>>> + memset(side_data->data, 0, sizeof(AVFilmGrainParams));
>>>> +
>>>> + return (AVFilmGrainParams *)side_data->data;
>>>> +}
>>>> diff --git a/libavutil/film_grain_params.h b/libavutil/film_grain_params.h
>>>> new file mode 100644
>>>> index 0000000000..ba20fe7fa6
>>>> --- /dev/null
>>>> +++ b/libavutil/film_grain_params.h
>>>> @@ -0,0 +1,159 @@
>>>> +/*
>>>> + * Copyright (c) 2016 Neil Birkbeck <neil.birkbeck at gmail.com>
>>>>
>>>
>>> Copy paste mistake?
>>>
>>
>> Yup.
>>
>>
>>
>>>> +typedef struct AVFilmGrainAOMParams {
>>>>
>>>
>>> If you believe AV2 will have a compatible implementation, then this name is fine.
>>>
>>
>> I don't think they're going to change it at all. They haven't even proven all this
>> system is capable of in order to replace it with something better, so they might
>> just do some minor tweaks. After all, its already somewhat ossified thanks to
>> hardware.
>>
>>
>>
>>>> + int8_t ar_coeffs_uv[2][25 + 3 /* padding for alignment purposes */];
>>>>
>>> Shouldn't the compiler take care of alignment?
>>>
>>
>> This simplified dav1d's SIMD, hence why its there, so I decided to leave it as-is.
>> The comment above says so too, but this small comment draws too much attention,
>> so I removed it.
>>
>
> I don't think that's something we should consider in our public API. We should prioritize small portable structs, and not trying to optimize fields for one potential use in one potential target out of many.
>
> This applies to every other case where you gave SIMD as reason.
>
I think I'd disagree with that in this case, some lenience here could help,
but I don't have a strong opinion on it, so whatever.
Looked through the spec and mapped all options to the smallest type they'll
fit, so num_y_points, num_uv_points, scaling_shift, ar_coeff_lag, ar_coeff_shift
grain_scale_shift are now uint8_t, ar_coeffs_uv, uv_mult and uv_mult_luma are
int8_t and uv_offset is int16_t.
Patch attached.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v3-0001-libavutil-introduce-AVFilmGrainParams-side-data.patch
Type: text/x-patch
Size: 10686 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20201123/a65962e4/attachment.bin>
More information about the ffmpeg-devel
mailing list