Thanks, will resolve these issues, the alphabetical issues are mostly
because I originally was calling it underwater filter.
Regarding the iteration over the buffer, I wasn't sure that buffer would
always be planar, can tidy this up. I assume it may be strided however?
Paul
On Tue, Aug 24, 2021 at 7:28 PM Paul B Mahol <one...@gmail.com> wrote:
>
>
> On Tue, Aug 24, 2021 at 6:58 PM Paul Buxton <
> paulbuxton.m...@googlemail.com> wrote:
>
>> Signed-off-by: Paul Buxton <paulbuxton.m...@googlemail.com>
>> ---
>> MAINTAINERS | 1 +
>> doc/filters.texi | 14 ++
>> libavfilter/Makefile | 1 +
>> libavfilter/allfilters.c | 1 +
>> libavfilter/version.h | 2 +-
>> libavfilter/vf_grayworld.c | 401 +++++++++++++++++++++++++++++++++++++
>> 6 files changed, 419 insertions(+), 1 deletion(-)
>> create mode 100644 libavfilter/vf_grayworld.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index dcac46003e..207a53871b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -369,6 +369,7 @@ Filters:
>> vf_tonemap_opencl.c Ruiling Song
>> vf_yadif.c Michael Niedermayer
>> vf_zoompan.c Paul B Mahol
>> + vf_grayworld.c Paul Buxton (CC
>> paulbuxton.m...@googlemail.com)
>>
>
> alphabetical order
>
>
>>
>> Sources:
>> vsrc_mandelbrot.c Michael Niedermayer
>> diff --git a/doc/filters.texi b/doc/filters.texi
>> index b902aca12d..06d2eb3a5a 100644
>> --- a/doc/filters.texi
>> +++ b/doc/filters.texi
>> @@ -13087,6 +13087,20 @@ greyedge=difford=1:minknorm=0:sigma=2
>>
>> @end itemize
>>
>> +
>> +@section greyworld
>> +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 linear light, so input
>
>
> missing word(s)
>
>
>> +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 guided
>> Apply guided filter for edge-preserving smoothing, dehazing and so on.
>>
>> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
>> index 102ce7beff..84ba69bba7 100644
>> --- a/libavfilter/Makefile
>> +++ b/libavfilter/Makefile
>> @@ -464,6 +464,7 @@ OBJS-$(CONFIG_TRANSPOSE_NPP_FILTER) +=
>> vf_transpose_npp.o
>> OBJS-$(CONFIG_TRANSPOSE_OPENCL_FILTER) += vf_transpose_opencl.o
>> opencl.o opencl/transpose.o
>> OBJS-$(CONFIG_TRANSPOSE_VAAPI_FILTER) += vf_transpose_vaapi.o
>> vaapi_vpp.o
>> OBJS-$(CONFIG_TRIM_FILTER) += trim.o
>> +OBJS-$(CONFIG_GRAYWORLD_FILTER) += vf_grayworld.o
>>
>
> alphabetical order
>
>
>> OBJS-$(CONFIG_UNPREMULTIPLY_FILTER) += vf_premultiply.o
>> framesync.o
>> OBJS-$(CONFIG_UNSHARP_FILTER) += vf_unsharp.o
>> OBJS-$(CONFIG_UNSHARP_OPENCL_FILTER) += vf_unsharp_opencl.o
>> opencl.o \
>> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
>> index 73040d2824..bf9356cee5 100644
>> --- a/libavfilter/allfilters.c
>> +++ b/libavfilter/allfilters.c
>> @@ -442,6 +442,7 @@ extern const AVFilter ff_vf_transpose_npp;
>> extern const AVFilter ff_vf_transpose_opencl;
>> extern const AVFilter ff_vf_transpose_vaapi;
>> extern const AVFilter ff_vf_trim;
>> +extern const AVFilter ff_vf_grayworld;
>>
>
> alphabetical order
>
>
>> extern const AVFilter ff_vf_unpremultiply;
>> extern const AVFilter ff_vf_unsharp;
>> extern const AVFilter ff_vf_unsharp_opencl;
>> diff --git a/libavfilter/version.h b/libavfilter/version.h
>> index bcd27aa6e8..e9a76c5ac3 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 3
>> +#define LIBAVFILTER_VERSION_MINOR 4
>> #define LIBAVFILTER_VERSION_MICRO 100
>>
>>
>> diff --git a/libavfilter/vf_grayworld.c b/libavfilter/vf_grayworld.c
>> new file mode 100644
>> index 0000000000..d41b116576
>> --- /dev/null
>> +++ b/libavfilter/vf_grayworld.c
>> @@ -0,0 +1,401 @@
>> +/*
>> + * 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;
>> + int64_t stretch_histogram;
>> + int64_t smoothing_frames;
>> + int64_t correction_type;
>> +
>> + float* tmppl;
>> + float* tmppa;
>> + float* tmppb;
>>
>
> float *tmppl,
> and so on.
>
>
>> +
>> + float* line_suml;
>> + float* line_suma;
>> + float* line_sumb;
>> +
>> + int* line_count_pels;
>> +} 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(float matrix[3][3], float input[3], float output[3]);
>> +void rgb2lab(float rgb[3], float lab[3]);
>> +void lab2rgb(float lab[3], float rgb[3]);
>> +
>>
>
> no need for several new lines, keep just one empty line.
>
>> +
>> +
>> +static int query_formats(AVFilterContext* ctx)
>> +{
>> + static const enum AVPixelFormat pix_fmts[] = {
>> + AV_PIX_FMT_GBRPF32,
>> + AV_PIX_FMT_GBRAPF32,
>> + AV_PIX_FMT_NONE,
>> + };
>> +
>> + AVFilterFormats* fmts_list = ff_make_format_list(pix_fmts);
>> + if (!fmts_list)
>> + return AVERROR(ENOMEM);
>> + return ff_set_common_formats(ctx, fmts_list);
>> +}
>>
>
> There is now new function to handle this, formats_from_list IIRC.
>
>
>> +
>> +void apply_matrix(float matrix[3][3], 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];
>> + }
>> +}
>> +
>> +
>> +float lms2lab[3][3] = {
>> + {0.5774, 0.5774, 0.5774},
>> + {0.40825, 0.40825, -0.816458},
>> + {0.707, -0.707, 0}
>> +};
>> +
>> +float lab2lms[3][3] = {
>> + {0.57735, 0.40825, 0.707},
>> + {0.57735, 0.40825, -0.707},
>> + {0.57735, -0.8165, 0}
>> +};
>> +
>> +float rgb2lms[3][3] = {
>> + {0.3811, 0.5783, 0.0402},
>> + {0.1967, 0.7244, 0.0782},
>> + {0.0241, 0.1288, 0.8444}
>> +};
>> +
>> +float lms2rgb[3][3] = {
>> + {4.4679, -3.5873, 0.1193},
>> + {-1.2186, 2.3809, -0.1624},
>> + {0.0497, -0.2439, 1.2045}
>> +};
>> +
>> +/**
>> + * Convert from Linear RGB to logspace LAB
>> + *
>> + * @param rgb Input array of rgb components
>> + * @param lab output array of lab components
>> + */
>> +void rgb2lab(float rgb[3], float lab[3])
>> +{
>> + float lms[3];
>> + apply_matrix(rgb2lms, rgb, lms);
>> + lms[0] = log(lms[0]);
>>
>
> what about using logf, float variant of functions in all cases?
>
>
>> + lms[1] = log(lms[1]);
>> + lms[2] = log(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(float lab[3], float rgb[3])
>> +{
>> + float lms[3];
>> + apply_matrix(lab2lms, lab, lms);
>> + lms[0] = exp(lms[0]);
>>
>
> same as above.
>
>
>> + lms[1] = exp(lms[1]);
>> + lms[2] = exp(lms[2]);
>> + apply_matrix(lms2rgb, lms, rgb);
>> +}
>> +
>> +/**
>> + *
>
>
> excessive line
>
>
>>
>> + * 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];
>> +
>> +
>>
>
> again.
>
>
>> +
>> + for (unsigned int i = slice_start; i < slice_end; i++) {
>> + float* g_in_row = (float*)(in->data[0] + i * in->linesize[0]);
>> + float* b_in_row = (float*)(in->data[1] + i * in->linesize[1]);
>> + float* r_in_row = (float*)(in->data[2] + i * in->linesize[2]);
>> + float* lcur = s->tmppl + (i * outlink->w);
>> + float* acur = s->tmppa + (i * outlink->w);
>> + float* bcur = s->tmppb + (i * outlink->w);
>> +
>> + s->line_suml[i] = 0.0;
>> + s->line_suma[i] = 0.0;
>> + s->line_sumb[i] = 0.0;
>> + s->line_count_pels[i] = 0;
>> +
>> + for (unsigned int j = 0; j < outlink->w; j++) {
>> + rgb[0] = r_in_row[j * td->desc->comp[0].step /
>> sizeof(float)];
>> + rgb[1] = g_in_row[j * td->desc->comp[0].step /
>> sizeof(float)];
>> + rgb[2] = b_in_row[j * td->desc->comp[0].step /
>> sizeof(float)];
>> + rgb2lab(rgb, lab);
>> + *(lcur++) = lab[0];
>> + *(acur++) = lab[1];
>> + *(bcur++) = lab[2];
>> + s->line_suml[i] += lab[0];
>> + s->line_suma[i] += lab[1];
>> + s->line_sumb[i] += lab[2];
>> + s->line_count_pels[i]++;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> +/**
>> + *
>
>
> excessive line
>
>
>>
>> + * 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 lsum = 0.0;
>> + float asum = 0.0;
>> + float bsum = 0.0;
>> + float pixels = 0;
>>
>
> please initialize floats with 0.f;
>
>
> + for (int y = 0; y < td->out->height; ++y) {
>> + lsum += s->line_suml[y];
>> + asum += s->line_suma[y];
>> + bsum += s->line_sumb[y];
>> + pixels += s->line_count_pels[y];
>> + }
>> + td->l_avg = lsum / pixels;
>> + td->a_avg = asum / pixels;
>> + td->b_avg = bsum / pixels;
>> +}
>> +
>> +
>> +/**
>> + *
>> + * Subtract the median logspace AB values from each pixel.
>>
>
> mean, median is something else.
>
>
>> + *
>> + * @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)
>>
>
> Please keep style to match with rest of lavfi codebase, thus pointers are
> written in another way.
>
>
>> +{
>> + 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;
>> +
>>
>
> excessive empty line.
>
>
>> + 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->tmppl + (i * outlink->w);
>> + float* acur = s->tmppa + (i * outlink->w);
>> + float* bcur = s->tmppb + (i * outlink->w);
>> +
>> + for (int j = 0; j < outlink->w; j++) {
>> + lab[0] = *lcur++;
>>
>
> IIUC first component is unchanged, thus this step can be skipped
> completely.
>
>
>> + 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 * td->odesc->comp[0].step / sizeof(float)] =
>> rgb[0];
>> + g_out_row[j * td->odesc->comp[1].step / sizeof(float)] =
>> rgb[1];
>> + b_out_row[j * td->odesc->comp[2].step / sizeof(float)] =
>> rgb[2];
>>
>
> This can be written in better way, format is planar, so this can be
> dramatically simplified.
>
>
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> +
>> +static int config_input(AVFilterLink* inlink)
>> +{
>> + GrayWorldContext* s = inlink->dst->priv;
>> +
>> + s->tmppl = av_malloc(inlink->h * inlink->w * sizeof(float));
>> + s->tmppa = av_malloc(inlink->h * inlink->w * sizeof(float));
>> + s->tmppb = av_malloc(inlink->h * inlink->w * sizeof(float));
>>
>
> use av_malloc_array. And checking of return value is mandatory.
>
>
>> +
>> + s->line_count_pels = av_malloc(inlink->h * sizeof(int));
>> +
>> + s->line_suml = av_malloc(inlink->h * sizeof(float));
>> + s->line_suma = av_malloc(inlink->h * sizeof(float));
>> + s->line_sumb = av_malloc(inlink->h * sizeof(float));
>>
>
> ditto.
>
>
>> +
>> + return 0;
>> +}
>> +
>> +static av_cold void uninit(AVFilterContext* ctx)
>> +{
>> + GrayWorldContext* s = ctx->priv;
>> +
>> + av_freep(&s->tmppl);
>> + av_freep(&s->tmppa);
>> + av_freep(&s->tmppb);
>> +
>> + av_freep(&s->line_count_pels);
>> +
>> + av_freep(&s->line_suml);
>> + av_freep(&s->line_suma);
>> + av_freep(&s->line_sumb);
>> +}
>> +
>> +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, "Tonemapping works on linear light
>> only\n");
>> + }
>> +
>> + td.in = in;
>> + td.out = out;
>> + td.desc = desc;
>> + td.odesc = odesc;
>> +
>> + ctx->internal->execute(ctx, convert_frame, &td, NULL,
>> FFMIN(outlink->h, ff_filter_get_nb_threads(ctx)));
>> + compute_correction(s, &td);
>> + ctx->internal->execute(ctx, correct_frame, &td, NULL,
>> FFMIN(outlink->h, ff_filter_get_nb_threads(ctx)));
>> +
>> + if (desc->flags & AV_PIX_FMT_FLAG_ALPHA && desc->flags &
>> AV_PIX_FMT_FLAG_ALPHA) {
>> + av_image_copy_plane(out->data[3], out->linesize[3],
>> + in->data[3], in->linesize[3],
>> + out->linesize[3], outlink->h);
>> + } else if (desc->flags & AV_PIX_FMT_FLAG_ALPHA) {
>> + for (int y = 0; y < out->height; y++) {
>> + for (int x = 0; x < out->width; x++) {
>> + AV_WN32(out->data[3] + x * desc->comp[3].step + y *
>> out->linesize[3],
>> + av_float2int(1.0f));
>> + }
>> + }
>> + }
>> +
>> + if (in != out)
>> + 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,
>> + },
>> + {NULL} };
>> +
>> +static const AVFilterPad grayworld_outputs[] = {
>> + {
>> + .name = "default",
>> + .type = AVMEDIA_TYPE_VIDEO,
>> + },
>> + {NULL} };
>>
>
> {NULL} is not needed any more on master ffmpeg version.
>
> +
>> +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,
>> + .inputs = grayworld_inputs,
>> + .outputs = grayworld_outputs,
>>
>
> This have been recently changed. Look at other filters for example how it
> is done now.
>
>
>
>> + .flags = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC |
>> AVFILTER_FLAG_SLICE_THREADS,
>> + .process_command = ff_filter_process_command,
>>
>
> This one is pointless, since filter have 0 options to configure from
> userspace.
>
>
>> + .uninit = uninit,
>> +};
>> --
>> 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".
