[FFmpeg-devel] av_parse_color() more improvement

[Aurelien Jacobs]

On Sat, Nov 13, 2010 at 02:31:19AM +0100, Michael Niedermayer wrote:
> On Sat, Nov 13, 2010 at 12:27:49AM +0100, Aurelien Jacobs wrote:
> > Hi,
> > 
> > Attached patch adds one more little improvement to av_parse_color(), to
> > finally make it fully usable in my upcoming SubRip decoder.
> > It simply allows specifying the color_string length so that the SubRip
> > decoder can just point color_string into the SubRip formatted text
> > buffer instead of making a copy of the color string with the sole
> > purpose to null-terminate it.
> > Anyway, av_parse_color() already makes a copy of the string so better not
> > force the caller to make one more copy.
> > 
> > Aurel
> >  parseutils.c |   10 +++++++---
> >  parseutils.h |    6 +++++-
> >  vf_drawbox.c |    2 +-
> >  vf_pad.c     |    4 ++--
> >  4 files changed, 15 insertions(+), 7 deletions(-)
> > c609ac10d59382113b81b11218c60c12ae495663  parse_color_len.diff
> > commit 7c17e28d9d380f49642672da31c692bf6da24ae7
> > Author: Aurelien Jacobs <aurel at gnuage.org>
> > Date:   Fri Nov 12 00:13:19 2010 +0100
> > 
> >     improve av_parse_color() to allow for non-null terminated color string
> > 
> > diff --git a/libavfilter/parseutils.c b/libavfilter/parseutils.c
> > index 470a42b..7c7d22d 100644
> > --- a/libavfilter/parseutils.c
> > +++ b/libavfilter/parseutils.c
> > @@ -183,7 +183,8 @@ static int color_table_compare(const void *lhs, const void *rhs)
> >  
> >  #define ALPHA_SEP '@'
> >  
> > -int av_parse_color(uint8_t *rgba_color, const char *color_string, void *log_ctx)
> > +int av_parse_color(uint8_t *rgba_color, const char *color_string, int slen,
> > +                   void *log_ctx)
> >  {
> >      char *tail, color_string2[128];
> >      const ColorEntry *entry;
> > @@ -194,7 +195,10 @@ int av_parse_color(uint8_t *rgba_color, const char *color_string, void *log_ctx)
> >      else if (!strncmp(color_string, "0x", 2))
> >          hex_offset = 2;
> >  
> > -    av_strlcpy(color_string2, color_string + hex_offset, sizeof(color_string2));
> > +    if (slen <= 0)
> 
> i think this should be <0

Indeed.

> otherwise lgtm

Applied with this change.

Aurel