Regarding the indentation. I think you mean adding whitespace to line up
the values?
I don't believe that is K&R, but sure. How far should I take it? Just the
initialisers in the structure you indicated, or should I try and line up
some of the accesses to other structure members, which I see some filters
(e.g. vf_guided) are doing?
I will resolve the the other issues at the same time.
Anything else?
On Sat, Aug 28, 2021 at 6:31 PM Paul B Mahol <one...@gmail.com> wrote:
>
>
> On Fri, Aug 27, 2021 at 11:18 AM Paul Buxton <
> paulbuxton.m...@googlemail.com> wrote:
>
>> Signed-off-by: Paul Buxton <paulbuxton.m...@googlemail.com>
>> ---
>> doc/filters.texi | 12 ++
>> libavfilter/Makefile | 1 +
>> libavfilter/allfilters.c | 1 +
>> libavfilter/version.h | 2 +-
>> libavfilter/vf_grayworld.c | 362 +++++++++++++++++++++++++++++++++++++
>> 5 files changed, 377 insertions(+), 1 deletion(-)
>> create mode 100644 libavfilter/vf_grayworld.c
>>
>> diff --git a/doc/filters.texi b/doc/filters.texi
>> index 269f2920d1..bb20ff538e 100644
>> --- a/doc/filters.texi
>> +++ b/doc/filters.texi
>> @@ -13054,6 +13054,18 @@ Set upper limit for video rate of output stream,
>> Default value is @var{25}.
>> This guarantee that output video frame rate will not be higher than this
>> value.
>> @end table
>>
>> +@section grayworld
>> +A color constancy filter that applies color correction based on the
>> grayworld assumption
>> +
>> +See: @url{
>> https://www.researchgate.net/publication/275213614_A_New_Color_Correction_Method_for_Underwater_Imaging
>> }
>> +
>> +The algorithm uses linear light, so input
>> +data should be linearized beforehand (and possibly correctly tagged).
>> +
>> +@example
>> +ffmpeg -i INPUT -vf
>> zscale=transfer=linear,grayworld,zscale=transfer=bt709,format=yuv420p OUTPUT
>> +@end example
>> +
>> @section greyedge
>> A color constancy variation filter which estimates scene illumination
>> via grey edge algorithm
>> and corrects the scene colors accordingly.
>> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
>> index 3166e70b58..1d86110d6b 100644
>> --- a/libavfilter/Makefile
>> +++ b/libavfilter/Makefile
>> @@ -287,6 +287,7 @@ OBJS-$(CONFIG_GBLUR_FILTER) +=
>> vf_gblur.o
>> OBJS-$(CONFIG_GEQ_FILTER) += vf_geq.o
>> OBJS-$(CONFIG_GRADFUN_FILTER) += vf_gradfun.o
>> OBJS-$(CONFIG_GRAPHMONITOR_FILTER) += f_graphmonitor.o
>> +OBJS-$(CONFIG_GRAYWORLD_FILTER) += vf_grayworld.o
>> OBJS-$(CONFIG_GREYEDGE_FILTER) += vf_colorconstancy.o
>> OBJS-$(CONFIG_GUIDED_FILTER) += vf_guided.o
>> OBJS-$(CONFIG_HALDCLUT_FILTER) += vf_lut3d.o framesync.o
>> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
>> index c49c831203..73e5fe1a41 100644
>> --- a/libavfilter/allfilters.c
>> +++ b/libavfilter/allfilters.c
>> @@ -272,6 +272,7 @@ extern const AVFilter ff_vf_gblur;
>> extern const AVFilter ff_vf_geq;
>> extern const AVFilter ff_vf_gradfun;
>> extern const AVFilter ff_vf_graphmonitor;
>> +extern const AVFilter ff_vf_grayworld;
>> extern const AVFilter ff_vf_greyedge;
>> extern const AVFilter ff_vf_guided;
>> extern const AVFilter ff_vf_haldclut;
>> diff --git a/libavfilter/version.h b/libavfilter/version.h
>> index e9a76c5ac3..e3a86d9b01 100644
>> --- a/libavfilter/version.h
>> +++ b/libavfilter/version.h
>> @@ -30,7 +30,7 @@
>> #include "libavutil/version.h"
>>
>> #define LIBAVFILTER_VERSION_MAJOR 8
>> -#define LIBAVFILTER_VERSION_MINOR 4
>> +#define LIBAVFILTER_VERSION_MINOR 5
>> #define LIBAVFILTER_VERSION_MICRO 100
>>
>>
>> diff --git a/libavfilter/vf_grayworld.c b/libavfilter/vf_grayworld.c
>> new file mode 100644
>> index 0000000000..b81dc2f28f
>> --- /dev/null
>> +++ b/libavfilter/vf_grayworld.c
>> @@ -0,0 +1,362 @@
>> +/*
>> + * Copyright (c) 2021 Paul Buxton
>> + *
>> + * 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
>> + */
>> +
>> +/**
>> + * @file
>> + * Color correction filter based on
>> + *
>> https://www.researchgate.net/publication/275213614_A_New_Color_Correction_Method_for_Underwater_Imaging
>> + *
>> + */
>> +
>> +#include "libavutil/imgutils.h"
>> +#include "libavutil/opt.h"
>> +#include "libavutil/pixdesc.h"
>> +#include "libavutil/intreadwrite.h"
>> +
>> +#include "avfilter.h"
>> +#include "drawutils.h"
>> +#include "formats.h"
>> +#include "internal.h"
>> +#include "video.h"
>> +
>> +typedef struct ThreadData {
>> + AVFrame *in, *out;
>> + float l_avg;
>> + float a_avg;
>> + float b_avg;
>> + const AVPixFmtDescriptor *desc;
>> + const AVPixFmtDescriptor *odesc;
>> +} ThreadData;
>> +
>> +typedef struct GrayWorldContext {
>> + const AVClass* class;
>> + float *tmpplab;
>> + int *line_count_pels;
>> + float *line_sum;
>> +
>> +} GrayWorldContext;
>> +
>> +#define OFFSET(x) offsetof(GrayWorldContext, x)
>> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM | AV_OPT_FLAG_VIDEO_PARAM |
>> AV_OPT_FLAG_RUNTIME_PARAM
>> +static const AVOption grayworld_options[] = {NULL};
>> +
>> +AVFILTER_DEFINE_CLASS(grayworld);
>> +
>> +void compute_correction(GrayWorldContext *s, ThreadData *td);
>> +void apply_matrix(const float matrix[3][3], const float input[3], float
>> output[3]);
>> +void rgb2lab(const float rgb[3], float lab[3]);
>> +void lab2rgb(const float lab[3], float rgb[3]);
>> +
>> +static int query_formats(AVFilterContext *ctx)
>> +{
>> + static const enum AVPixelFormat pix_fmts[] = {
>> + AV_PIX_FMT_GBRPF32,
>> + AV_PIX_FMT_GBRAPF32,
>> + AV_PIX_FMT_NONE,
>> + };
>> + return ff_set_common_formats_from_list(ctx, pix_fmts);
>> +}
>> +
>> +void apply_matrix(const float matrix[3][3], const float input[3], float
>> output[3])
>> +{
>> + for (int row = 0; row < 3; ++row) {
>> + output[row] = matrix[row][0] * input[0] + matrix[row][1] *
>> input[1] + matrix[row][2] * input[2];
>> + }
>> +}
>> +
>> +const float lms2lab[3][3] = {
>> + {0.5774, 0.5774, 0.5774},
>> + {0.40825, 0.40825, -0.816458},
>> + {0.707, -0.707, 0}
>> +};
>> +
>> +const float lab2lms[3][3] = {
>> + {0.57735, 0.40825, 0.707},
>> + {0.57735, 0.40825, -0.707},
>> + {0.57735, -0.8165, 0}
>> +};
>> +
>> +const float rgb2lms[3][3] = {
>> + {0.3811, 0.5783, 0.0402},
>> + {0.1967, 0.7244, 0.0782},
>> + {0.0241, 0.1288, 0.8444}
>> +};
>> +
>> +const float lms2rgb[3][3] = {
>> + {4.4679, -3.5873, 0.1193},
>> + {-1.2186, 2.3809, -0.1624},
>> + {0.0497, -0.2439, 1.2045}
>> +};
>> +
>>
>
> all those tables should be static.
>
>
>> +/**
>> + * Convert from Linear RGB to logspace LAB
>> + *
>> + * @param rgb Input array of rgb components
>> + * @param lab output array of lab components
>> + */
>> +void rgb2lab(const float rgb[3], float lab[3])
>> +{
>> + float lms[3];
>> + apply_matrix(rgb2lms, rgb, lms);
>> + lms[0] = logf(lms[0]);
>> + lms[1] = logf(lms[1]);
>> + lms[2] = logf(lms[2]);
>> + apply_matrix(lms2lab, lms, lab);
>> +}
>> +
>> +/**
>> + * Convert from Logspace LAB to Linear RGB
>> + *
>> + * @param lab input array of lab components
>> + * @param rgb output array of rgb components
>> + */
>> +void lab2rgb(const float lab[3], float rgb[3])
>> +{
>> + float lms[3];
>> + apply_matrix(lab2lms, lab, lms);
>> + lms[0] = expf(lms[0]);
>> + lms[1] = expf(lms[1]);
>> + lms[2] = expf(lms[2]);
>> + apply_matrix(lms2rgb, lms, rgb);
>> +}
>> +
>> +/**
>> + *
>> + * Convert a frame from linear RGB to logspace LAB, and accumulate
>> channel totals for each row
>> + * Convert from RGB -> lms using equation 4 in color transfer paper.
>> + *
>> + * @param ctx Filter context
>> + * @param arg Thread data pointer
>> + * @param jobnr job number
>> + * @param nb_jobs number of jobs
>> + */
>> +static int convert_frame(AVFilterContext *ctx, void *arg, int jobnr, int
>> nb_jobs)
>> +{
>> + GrayWorldContext *s = ctx->priv;
>> + ThreadData *td = arg;
>> + AVFrame *in = td->in;
>> + AVFrame *out = td->out;
>> + AVFilterLink *outlink = ctx->outputs[0];
>> + const int slice_start = (out->height * jobnr) / nb_jobs;
>> + const int slice_end = (out->height * (jobnr + 1)) / nb_jobs;
>> + float rgb[3];
>> + float lab[3];
>> +
>> + for (unsigned int i = slice_start; i < slice_end; i++) {
>> + float *b_in_row = (float*)(in->data[1] + i * in->linesize[1]);
>> + float *g_in_row = (float*)(in->data[0] + i * in->linesize[0]);
>> + float *r_in_row = (float*)(in->data[2] + i * in->linesize[2]);
>> + float *acur = s->tmpplab + i * outlink->w + outlink->w *
>> outlink->h;
>> + float *bcur = s->tmpplab + i * outlink->w + 2 * outlink->w *
>> outlink->h;
>> + float *lcur = s->tmpplab + i * outlink->w;
>> +
>> + s->line_sum[i] = 0.0f;
>> + s->line_sum[i + outlink->h] = 0.0f;
>> + s->line_count_pels[i] = 0;
>> +
>> + for (unsigned int j = 0; j < outlink->w; j++) {
>> + rgb[0] = r_in_row[j];
>> + rgb[1] = g_in_row[j];
>> + rgb[2] = b_in_row[j];
>> + rgb2lab(rgb, lab);
>> + *(lcur++) = lab[0];
>> + *(acur++) = lab[1];
>> + *(bcur++) = lab[2];
>> + s->line_sum[i] += lab[1];
>> + s->line_sum[i + outlink->h] += lab[2];
>> + s->line_count_pels[i]++;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> +/**
>> + *
>> + * Sum the channel totals and compute the median for each channel
>> + *
>> + * @param s Frame context
>> + * @param td thread data
>> + */
>> +void compute_correction(GrayWorldContext *s, ThreadData *td)
>> +{
>> + float asum = 0.0f;
>> + float bsum = 0.0f;
>> + float pixels = 0;
>> + for (int y = 0; y < td->out->height; ++y) {
>> + asum += s->line_sum[y];
>> + bsum += s->line_sum[y + td->out->height];
>> + pixels += s->line_count_pels[y];
>> + }
>> + td->a_avg = asum / pixels;
>> + td->b_avg = bsum / pixels;
>> +}
>> +
>> +/**
>> + *
>> + * Subtract the mean logspace AB values from each pixel.
>> + *
>> + * @param ctx Filter context
>> + * @param arg Thread data pointer
>> + * @param jobnr job number
>> + * @param nb_jobs number of jobs
>> + */
>> +static int correct_frame(AVFilterContext *ctx, void *arg, int jobnr, int
>> nb_jobs)
>> +{
>> + GrayWorldContext *s = ctx->priv;
>> + ThreadData *td = arg;
>> + AVFrame *out = td->out;
>> + AVFilterLink *outlink = ctx->outputs[0];
>> + const int slice_start = (out->height * jobnr) / nb_jobs;
>> + const int slice_end = (out->height * (jobnr + 1)) / nb_jobs;
>> + float rgb[3];
>> + float lab[3];
>> +
>> + for (int i = slice_start; i < slice_end; i++) {
>> + float *g_out_row = (float*)(out->data[0] + i * out->linesize[0]);
>> + float *b_out_row = (float*)(out->data[1] + i * out->linesize[1]);
>> + float *r_out_row = (float*)(out->data[2] + i * out->linesize[2]);
>> + float *lcur = s->tmpplab + i * outlink->w;
>> + float *acur = s->tmpplab + i * outlink->w + outlink->w *
>> outlink->h;
>> + float *bcur = s->tmpplab + i * outlink->w + 2 * outlink->w *
>> outlink->h;
>> +
>> + for (int j = 0; j < outlink->w; j++) {
>> + lab[0] = *lcur++;
>> + lab[1] = *acur++;
>> + lab[2] = *bcur++;
>> +
>> + // subtract the average for the color channels
>> + lab[1] -= td->a_avg;
>> + lab[2] -= td->b_avg;
>> +
>> + //convert back to linear rgb
>> + lab2rgb(lab, rgb);
>> + r_out_row[j] = rgb[0];
>> + g_out_row[j] = rgb[1];
>> + b_out_row[j] = rgb[2];
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> +static int config_input(AVFilterLink *inlink)
>> +{
>> + GrayWorldContext *s = inlink->dst->priv;
>> +
>> + s->tmpplab = av_malloc_array(inlink->h * inlink->w * 3,
>> sizeof(float) );
>> + if (!s->tmpplab){
>> + return AVERROR(ENOMEM);
>> + }
>> + FF_ALLOC_TYPED_ARRAY(s->line_count_pels, inlink->h);
>> + if (!s->line_count_pels){
>> + return AVERROR(ENOMEM);
>> + }
>> + FF_ALLOC_TYPED_ARRAY(s->line_sum, inlink->h * 2);
>> + if (!s->line_sum){
>> + return AVERROR(ENOMEM);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static av_cold void uninit(AVFilterContext *ctx)
>> +{
>> + GrayWorldContext *s = ctx->priv;
>> +
>> + av_freep(&s->tmpplab);
>> + av_freep(&s->line_count_pels);
>> + av_freep(&s->line_sum);
>> +}
>> +
>> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>> +{
>> + AVFilterContext *ctx = inlink->dst;
>> + GrayWorldContext *s = ctx->priv;
>> + AVFilterLink *outlink = ctx->outputs[0];
>> + ThreadData td;
>> + AVFrame *out;
>> + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
>> + const AVPixFmtDescriptor *odesc =
>> av_pix_fmt_desc_get(outlink->format);
>> +
>> + if (av_frame_is_writable(in)) {
>> + out = in;
>> + } else {
>> + out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
>> + if (!out) {
>> + av_frame_free(&in);
>> + return AVERROR(ENOMEM);
>> + }
>> + av_frame_copy_props(out, in);
>> + }
>> + /* input and output transfer will be linear */
>> + if (in->color_trc == AVCOL_TRC_UNSPECIFIED) {
>> + av_log(s, AV_LOG_WARNING, "Untagged transfer, assuming linear
>> light\n");
>> + out->color_trc = AVCOL_TRC_LINEAR;
>> + } else if (in->color_trc != AVCOL_TRC_LINEAR) {
>> + av_log(s, AV_LOG_WARNING, "Grey world color correction works on
>> linear light only\n");
>> + }
>> +
>> + td.in = in;
>> + td.out = out;
>> + td.desc = desc;
>> + td.odesc = odesc;
>> +
>> + ff_filter_execute(ctx, convert_frame, &td, NULL, FFMIN(outlink->h,
>> ff_filter_get_nb_threads(ctx)));
>> + compute_correction(s, &td);
>> + ff_filter_execute(ctx, correct_frame, &td, NULL, FFMIN(outlink->h,
>> ff_filter_get_nb_threads(ctx)));
>> +
>> + if (in != out){
>> + // copy alpha channel from input frame to output
>> + av_image_copy_plane(out->data[3], out->linesize[3],
>> + in->data[3], in->linesize[3],
>> + out->linesize[3], outlink->h);
>> + // free the input frame
>>
>
> Please remove obvious comments, like this one above.
>
>> + av_frame_free(&in);
>> + }
>> +
>> + return ff_filter_frame(outlink, out);
>> +}
>> +
>> +static const AVFilterPad grayworld_inputs[] = {
>> + {
>> + .name = "default",
>> + .type = AVMEDIA_TYPE_VIDEO,
>> + .filter_frame = filter_frame,
>> + .config_props = config_input,
>> + }
>> +};
>> +
>> +static const AVFilterPad grayworld_outputs[] = {
>> + {
>> + .name = "default",
>> + .type = AVMEDIA_TYPE_VIDEO,
>> + }
>> +};
>> +
>> +const AVFilter ff_vf_grayworld = {
>> + .name = "grayworld",
>> + .description = NULL_IF_CONFIG_SMALL("Adjust white balance using LAB
>> gray world algorithm"),
>> + .priv_size = sizeof(GrayWorldContext),
>> + .priv_class = &grayworld_class,
>> + .query_formats = query_formats,
>> + FILTER_INPUTS(grayworld_inputs),
>> + FILTER_OUTPUTS(grayworld_outputs),
>> + .flags = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC |
>> AVFILTER_FLAG_SLICE_THREADS,
>> + .uninit = uninit,
>>
>
> Why this is not nicely indented like in any other filter?
>
> +};
>> --
>> 2.17.1
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
