[FFmpeg-devel] [RFC] Scalar color conversion utils (colorutils.[hc])?

[Michael Niedermayer]

On Sun, May 03, 2009 at 03:42:04PM +0200, Stefano Sabatini wrote:
> On date Saturday 2009-05-02 22:11:42 +0200, Michael Niedermayer encoded:
> > On Sat, May 02, 2009 at 09:37:09PM +0200, Stefano Sabatini wrote:
> > > On date Saturday 2009-05-02 18:45:49 +0200, Michael Niedermayer encoded:
> [...]
> > > > Once lavfi is merged this could be revissited but we surely wont combine
> > > > the string parsing with colorspace convertion because frankly that is
> > > > unflexible.
> > > 
> > > Would be:
> > > int av_parse_color(uint16_t *rgba_color, const char *color_string, void *log_ctx);
> > > 
> > > an acceptable solution?
> > 
> > this looks better
> 
> Patch updated (as usage example I'm going to post an updated version
> of the RGBA source).
> 
> Regards.
> -- 
> FFmpeg = Fierce and Faithless Marvellous Portable Ecstatic Gadget

>  Makefile     |    3 
>  colorutils.c |  249 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  colorutils.h |   41 +++++++++
>  3 files changed, 293 insertions(+)
> 9b57e8b85216d7af4169a2aec2df2aca4a10b1e3  implement-colorutils.patch
> Index: libavfilter-soc/ffmpeg/libavfilter/colorutils.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ libavfilter-soc/ffmpeg/libavfilter/colorutils.c	2009-05-03 14:38:23.000000000 +0200

wouldnt that stuff fit in parseutils.* ?


[...]
> +int av_parse_color(uint8_t *rgba_color, const char *color_string, void *log_ctx)
> +{
> +    if (!strncmp(color_string, "0x", 2)) {
> +        char *tail;
> +        int len = strlen(color_string);
> +        int rgba = strtol(color_string, &tail, 16);
> +
> +        if (*tail || (len != 8 && len != 10)) {
> +            av_log(log_ctx, AV_LOG_ERROR, "Invalid RGB color string: '%s'\n", color_string);
> +            return -1;
> +        }
> +        if (len == 10) {
> +            rgba_color[3] = rgba & 0xFF;
> +            rgba >>= 8;
> +        }
> +        rgba_color[0] = rgba >> 16;
> +        rgba_color[1] = (rgba >> 8) & 0xFF;
> +        rgba_color[2] = rgba & 0xFF;

the & 0xff is superflous


> +    } else {
> +        const ColorEntry *entry = NULL;

redundant assignment


> +        entry = bsearch(color_string,
> +                        color_table,
> +                        FF_ARRAY_ELEMS(color_table),
> +                        sizeof(ColorEntry),
> +                        color_table_compare);
> +        if (!entry) {
> +            av_log(log_ctx, AV_LOG_DEBUG, "Cannot find color '%s'\n", color_string);
> +            return -1;
> +        }
> +        rgba_color[0] = entry->color[0];
> +        rgba_color[1] = entry->color[1];
> +        rgba_color[2] = entry->color[2];
> +        rgba_color[3] = entry->color[3];

memcpy

[...]
> +#ifndef AVFILTER_COLORUTILS_H
> +#define AVFILTER_COLORUTILS_H
> +
> +#include "libavutil/pixfmt.h"
> +#include "libavcodec/pixdesc.h"
> +
> +/**
> + * Parses the name in name,

ehm

> and puts in color the component values of the parsed color.

uhm


[...]
> +int av_parse_color(uint8_t *rgba_color, const char *color_string, void *log_ctx);

and there is no name nor color in this ...


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090504/38e6a59c/attachment.pgp>