[FFmpeg-devel] av_parse_color() improvement
Aurelien Jacobs
aurel
Fri Nov 12 23:53:10 CET 2010
On Fri, Nov 12, 2010 at 07:38:40PM +0100, Michael Niedermayer wrote:
> On Thu, Nov 11, 2010 at 02:29:27AM +0100, Aurelien Jacobs wrote:
> > Hi,
> >
> > Attached patch makes the av_parse_color() a bit more versatile.
> > It allows parsing of html formated hex color (ie. starting with a #
> > instead of 0x), including its loose form where the # is missing.
> > This makes quite some sens as the function is already able to parse
> > html named colors.
> > This is required to be able easily use this function to parse colors
> > out of SubRip files.
> >
> > Aurel
> > parseutils.c | 22 +++++++++++++++++-----
> > parseutils.h | 2 +-
> > 2 files changed, 18 insertions(+), 6 deletions(-)
> > 4ae48624cebdcb2a693371dd00f51f6cff086711 parse_color.diff
> > Index: libavfilter/parseutils.h
> > ===================================================================
> > --- libavfilter/parseutils.h (revision 25719)
> > +++ libavfilter/parseutils.h (working copy)
> > @@ -31,7 +31,7 @@
> > * Put the RGBA values that correspond to color_string in rgba_color.
> > *
> > * @param color_string a string specifying a color. It can be the name of
> > - * a color (case insensitive match) or a 0xRRGGBB[AA] sequence,
> > + * a color (case insensitive match) or a [0x|#]RRGGBB[AA] sequence,
> > * possibly followed by "@" and a string representing the alpha
> > * component.
> > * The alpha component may be a string composed by "0x" followed by an
> > Index: libavfilter/parseutils.c
> > ===================================================================
> > --- libavfilter/parseutils.c (revision 25719)
> > +++ libavfilter/parseutils.c (working copy)
> > @@ -187,9 +187,17 @@
> > {
> > char *tail, color_string2[128];
> > const ColorEntry *entry;
> > - av_strlcpy(color_string2, color_string, sizeof(color_string2));
> > + int len, hex_offset = 0;
> > +
> > + if (color_string[0] == '#')
> > + hex_offset = 1;
> > + else if (!strncmp(color_string, "0x", 2))
> > + hex_offset = 2;
> > +
> > + av_strlcpy(color_string2, color_string + hex_offset, sizeof(color_string2));
> > if ((tail = strchr(color_string2, ALPHA_SEP)))
> > *tail++ = 0;
> > + len = strlen(color_string2);
> > rgba_color[3] = 255;
> >
> > if (!strcasecmp(color_string2, "random") || !strcasecmp(color_string2, "bikeshed")) {
> > @@ -198,16 +206,16 @@
> > rgba_color[1] = rgba >> 16;
> > rgba_color[2] = rgba >> 8;
> > rgba_color[3] = rgba;
> > - } else if (!strncmp(color_string2, "0x", 2)) {
> > + } else if (hex_offset ||
>
> > + strspn(color_string2, "0123456789ABCDEFabcdef") == len) {
>
> why is that needed?
> i thought things failing this would fail some other tests too
yes and no...
The code has 3 branches looking like this:
if("random") {
color = random();
} else if(has_hex_prefix()) {
color = parse_as_hex_number();
} else {
color = parse_as_color_name();
}
Right now, the has_hex_prefix() branch will only be entered if the
string start with 0x.
I need this function to also be able to parse hex numbers without a
prefix. Right now, such a string fall back into the
parse_as_color_name() case and just fail.
I see to solution to solve this:
1) Always try to parse as a color name first, and if it fail, try to
parse as a hex number. But this would slow down parsing hex numbers by
first trying to match them with the entries of the color name table.
2) If we don't have a hex prefix, check if the string is composed only
of hex charaters, and if so parse it as a hex number anyway. Every
single entry of the color name table has at least one non-hex character,
so there is no chance to wrongly detect a color name as a hex number.
My patch implements solution 2.
Anything wrong with it ?
Any better solution ?
Aurel
More information about the ffmpeg-devel
mailing list