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

Stefano Sabatini stefano.sabatini-lala
Sat May 2 10:27:22 CEST 2009


On date Saturday 2009-05-02 04:32:32 +0200, Michael Niedermayer encoded:
> On Sat, May 02, 2009 at 12:48:11AM +0200, Stefano Sabatini wrote:
[...]
> > Attached stub, result of the test is:
> > 
> > Cannot find color 'foo'
> > Cannot find color 'red'
> > Cannot find color 'Red '
> > Cannot find color 'RED'
> > Red -> R(255) G(0) B(0)
> > 0x000000 -> R(0) G(0) B(0)
> > 0x3e34ff -> R(62) G(52) B(255)
> > Invalid RGB color string: '0xfoobar'
> > Invalid RGB color string: '0xffffeeeeeeee'
> > 
> > Conversion has to be implemented yet.
> > 
> > I wonder if the case-insensitive matching of the name is a feature
> > worth to be implemented, but as far as I understood it would require a
> > linear search as opposed to a binary search as currently implemented.
> 
> no

[...]
> > +        rgb_color[3] = 255;
> > +    } else {
> > +        const ColorEntry *entry = NULL;
> > +        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;
> > +        }
> > +        memcpy(rgb_color, entry->color, sizeof(*color) * 3);
> > +    }
> > +
> > +    return av_convert_color(color,     dst_pix_fmt,   dst_colorspace,
> > +                            rgb_color, PIX_FMT_RGB24, AV_COLORSPACE_RGB,
> > +                            log_ctx);
> 
> i dont think it makes sense to mix color convert and color parse

That was to perform parsing and conversion in a single step. The
alternative may be:

int av_parse_color(uint16_t *rgba_color, const char *color_string, void *log_ctx);

which would return an RGBA color, then the conversion may be done
explicitely using:

av_convert_color(color,     dst_pix_fmt,   dst_colorspace,
                 rgb_color, PIX_FMT_RGBA,  AV_COLORSPACE_RGB, log_ctx);

but I prefer the previous more flexible variant.

> [...]
> 
> > +#ifndef AVFILTER_COLORUTILS_H
> > +#define AVFILTER_COLORUTILS_H
> > +
> > +#include "libavutil/pixfmt.h"
> > +#include "libavcodec/pixdesc.h"
> > +
> > +enum AVColorspace {
> > +    AV_COLORSPACE_RGB,
> > +    AV_COLORSPACE_ITU709,
> > +    AV_COLORSPACE_FCC,
> > +    AV_COLORSPACE_ITU601,
> > +    AV_COLORSPACE_SMPTE240M,
> > +    AV_COLORSPACE_NB
> > +};
> 
> let me try again
> we need a AVColorspace in AVCodecContext not in a header inaccessible to
> avcodec.h
> 
> Also they miss doxy comments and i hope there are no duplicates amongth
> them

Of course, that was just to provide a sketch in a single patch, I'll
dive into that soon.

Regards.
-- 
FFmpeg = Forgiving Fiendish Mean Philosophical Exxagerate Gangster



More information about the ffmpeg-devel mailing list