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

Michael Niedermayer michaelni
Sat May 2 22:11:42 CEST 2009


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:
> > On Sat, May 02, 2009 at 10:27:22AM +0200, Stefano Sabatini wrote:
> [...]
> > > > > +        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.
> > 
> > I think its better if we drop this patch then.
>  >
> > I will add the enum to AVCodecContext myself, the rest is currently not of
> > any relevance as it isnt and would not be used by anything. It requires lavfi
> > to be merged first ...
> 
> I have a patch for this, but I have many things to study before to be
> confident with the field, also I'm not sure the idea of including CIE
> colorspaces is really a good idea.

my suggestion is attached, ill commit mine if there are no suggestions for
improvment.


>  
> > 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

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

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: colorspace.patch
Type: text/x-patch
Size: 3580 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090502/2f1fe6aa/attachment.bin>
-------------- 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/20090502/2f1fe6aa/attachment.pgp>



More information about the ffmpeg-devel mailing list