[FFmpeg-devel] [PATCH] vf_colorspace: add floyd-steinberg dithering option to full conversion.
Michael Niedermayer
michael at niedermayer.cc
Thu May 5 14:16:00 CEST 2016
On Tue, May 03, 2016 at 01:53:42PM -0400, Ronald S. Bultje wrote:
> ---
> doc/filters.texi | 13 ++++
> libavfilter/colorspacedsp.c | 12 ++++
> libavfilter/colorspacedsp.h | 6 ++
> libavfilter/colorspacedsp_template.c | 128 +++++++++++++++++++++++++++++++++++
> libavfilter/vf_colorspace.c | 58 +++++++++++++++-
> 5 files changed, 214 insertions(+), 3 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index b17b115..98a002b 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -5104,6 +5104,19 @@ YUV 4:4:4 planar 12-bits
> Do a fast conversion, which skips gamma/primary correction. This will take
> significantly less CPU, but will be mathematically incorrect. To get output
> compatible with that produced by the colormatrix filter, use fast=1.
> +
> + at item dither
> +Specify dithering mode.
> +
> +The accepted values are:
> + at table @samp
> + at item none
> +No dithering
> +
> + at item fsb
> +Floyd-Steinberg dithering
> + at end table
> +
> @end table
>
> The filter converts the transfer characteristics, color space and color
> diff --git a/libavfilter/colorspacedsp.c b/libavfilter/colorspacedsp.c
> index d4c43c3..f95805b 100644
> --- a/libavfilter/colorspacedsp.c
> +++ b/libavfilter/colorspacedsp.c
> @@ -20,6 +20,9 @@
>
> #include "colorspacedsp.h"
>
> +/*
> + * SS_W/H are synonyms for AVPixFmtDescriptor->log2_chroma_w/h.
doesnt explain what SS stands for, that makes this harder to remember
> + */
> #define SS_W 0
> #define SS_H 0
>
> @@ -114,6 +117,15 @@ void ff_colorspacedsp_init(ColorSpaceDSPContext *dsp)
> init_rgb2yuv_fn(1, 10);
> init_rgb2yuv_fn(2, 12);
>
> +#define init_rgb2yuv_fsb_fn(idx, bit) \
> + dsp->rgb2yuv_fsb[idx][0] = rgb2yuv_fsb_444p##bit##_c; \
> + dsp->rgb2yuv_fsb[idx][1] = rgb2yuv_fsb_422p##bit##_c; \
> + dsp->rgb2yuv_fsb[idx][2] = rgb2yuv_fsb_420p##bit##_c
> +
> + init_rgb2yuv_fsb_fn(0, 8);
> + init_rgb2yuv_fsb_fn(1, 10);
> + init_rgb2yuv_fsb_fn(2, 12);
is there a plan for bit depth other than these and subsamplings the
than these ?
0,1,2 is bad, these should be named constants/enums for the subsamplings
or maybe a 2d array
it arguably may be nitpicking but if you dont want this to turn into
a worse mess than swscale then it should be done clean & well
documented from the begin
> +
> #define init_yuv2yuv_fn(idx1, idx2, bit1, bit2) \
> dsp->yuv2yuv[idx1][idx2][0] = yuv2yuv_444p##bit1##to##bit2##_c; \
> dsp->yuv2yuv[idx1][idx2][1] = yuv2yuv_422p##bit1##to##bit2##_c; \
> diff --git a/libavfilter/colorspacedsp.h b/libavfilter/colorspacedsp.h
> index 4e70c6c..2ca7b19 100644
> --- a/libavfilter/colorspacedsp.h
> +++ b/libavfilter/colorspacedsp.h
> @@ -32,6 +32,11 @@ typedef void (*rgb2yuv_fn)(uint8_t *yuv[3], ptrdiff_t yuv_stride[3],
> int16_t *rgb[3], ptrdiff_t rgb_stride,
> int w, int h, const int16_t rgb2yuv_coeffs[3][3][8],
> const int16_t yuv_offset[8]);
> +typedef void (*rgb2yuv_fsb_fn)(uint8_t *yuv[3], ptrdiff_t yuv_stride[3],
> + int16_t *rgb[3], ptrdiff_t rgb_stride,
> + int w, int h, const int16_t rgb2yuv_coeffs[3][3][8],
> + const int16_t yuv_offset[8],
> + int *rnd[3][2]);
lacks documentation
the strides also can be const as well as rgb
> typedef void (*yuv2yuv_fn)(uint8_t *yuv_out[3], ptrdiff_t yuv_out_stride[3],
> uint8_t *yuv_in[3], ptrdiff_t yuv_in_stride[3],
off topic but, same here
> int w, int h, const int16_t yuv2yuv_coeffs[3][3][8],
> @@ -40,6 +45,7 @@ typedef void (*yuv2yuv_fn)(uint8_t *yuv_out[3], ptrdiff_t yuv_out_stride[3],
> typedef struct ColorSpaceDSPContext {
> yuv2rgb_fn yuv2rgb[3 /* 0: 8bit, 1: 10bit, 2: 12bit */][3 /* 0: 444, 1: 422, 2: 420 */];
> rgb2yuv_fn rgb2yuv[3 /* 0: 8bit, 1: 10bit, 2: 12bit */][3 /* 0: 444, 1: 422, 2: 420 */];
> + rgb2yuv_fsb_fn rgb2yuv_fsb[3 /* 0: 8bit, 1: 10bit, 2: 12bit */][3 /* 0: 444, 1: 422, 2: 420 */];
> yuv2yuv_fn yuv2yuv[3 /* in_depth */][3 /* out_depth */][3 /* 0: 444, 1: 422, 2: 420 */];
>
> void (*multiply3x3)(int16_t *data[3], ptrdiff_t stride,
> diff --git a/libavfilter/colorspacedsp_template.c b/libavfilter/colorspacedsp_template.c
> index f225391..db4a8d2 100644
> --- a/libavfilter/colorspacedsp_template.c
> +++ b/libavfilter/colorspacedsp_template.c
> @@ -199,6 +199,134 @@ static void fn(rgb2yuv)(uint8_t *_yuv[3], ptrdiff_t yuv_stride[3],
> }
> }
>
> +/* floyd-steinberg dithering - for any mid-top pixel A in a 3x2 block of pixels:
> + * 1 A 2
> + * 3 4 5
> + * the rounding error is distributed over the neighbouring pixels:
> + * 2: 7/16th, 3: 3/16th, 4: 5/16th and 5: 1/16th
> + */
do we want non public functions to be available in doxygen ?
if no then ok otherwise missing /**
[...]
> @@ -888,15 +910,39 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
> else if (s->iall != CS_UNSPECIFIED)
> in->color_trc = default_trc[s->iall];
> if (rgb_sz != s->rgb_sz) {
> + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(out->format);
> + int uvw = in->width >> desc->log2_chroma_w;
> +
> av_freep(&s->rgb[0]);
> av_freep(&s->rgb[1]);
> av_freep(&s->rgb[2]);
> s->rgb_sz = 0;
> + av_freep(&s->dither_scratch_base[0][0]);
> + av_freep(&s->dither_scratch_base[0][1]);
> + av_freep(&s->dither_scratch_base[1][0]);
> + av_freep(&s->dither_scratch_base[1][1]);
> + av_freep(&s->dither_scratch_base[2][0]);
> + av_freep(&s->dither_scratch_base[2][1]);
>
> s->rgb[0] = av_malloc(rgb_sz);
> s->rgb[1] = av_malloc(rgb_sz);
> s->rgb[2] = av_malloc(rgb_sz);
> - if (!s->rgb[0] || !s->rgb[1] || !s->rgb[2]) {
> + s->dither_scratch_base[0][0] = av_malloc(sizeof(int) * (in->width + 4));
> + s->dither_scratch_base[0][1] = av_malloc(sizeof(int) * (in->width + 4));
> + s->dither_scratch_base[1][0] = av_malloc(sizeof(int) * (uvw + 4));
> + s->dither_scratch_base[1][1] = av_malloc(sizeof(int) * (uvw + 4));
> + s->dither_scratch_base[2][0] = av_malloc(sizeof(int) * (uvw + 4));
> + s->dither_scratch_base[2][1] = av_malloc(sizeof(int) * (uvw + 4));
sizeof should be using dither_scratch_base not duplicating its type
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Never trust a computer, one day, it may think you are the virus. -- Compn
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160505/1ee79f54/attachment.sig>
More information about the ffmpeg-devel
mailing list