[FFmpeg-devel] [PATCH] lavu: add av_get_color_string() and use it in -colors option

[Stefano Sabatini]

On date Monday 2013-10-21 11:36:37 +0200, Stefano Sabatini encoded:
> On date Monday 2013-10-21 11:08:17 +0200, Nicolas George encoded:
> > Le sextidi 26 vendémiaire, an CCXXII, Stefano Sabatini a écrit :
> > >  /**
> > > + * Get color name with index color_idx, as recognized by the
> > > + * av_parse_color() function.
> > 
> > I suggest to try to make it clearer that this functions is intended to
> > enumerate the hardcoded color names known to the library, as opposed to, for
> > example, convert a color into a user-readable string.
> > 
> > > + *
> > > + * @param bufp      pointer to the address where the color name string address is written
> > 
> > What is the point of both returning it through the return value and a
> > pointer?
> > 
> > > + * @param rgb       pointer to the address where the 3-values array containing the RGB
> > > + *                  components is written, may be NULL
> > 
> > Inconsistent naming of the argument here and in the prototype, compared to
> > the function definition "rgbp".
> > 
> > And the description is really not clear.
> > 
> > > + * @param color_idx the number of the color entries, starting from 0
> > > + * @return the color name string or NULL if color_idx is not in the array
> > > + */
> > > +const char *av_get_color_name(const char **bufp, const uint8_t **rgb, int color_idx);
> > > +
> > > +/**
> > >   * Parse timestr and return in *time a corresponding number of
> > >   * microseconds.
> > >   *
> > 
> > May I suggest:
> > 
> > av_get_known_color_name(unsigned id, const uint8_t **rgbp);
> > 
> > Get the name of a color from the internal table of hard-coded named colors.
> > 
> > [in]  id:   index of the requested color, starting from 0
> > [out] rgbp: if not NULL, will point to a 3-elements array with the color
> >             value in RGB
> > 
> > OTOH, since it only concerns hard-coded colors, I believe it very unlikely
> > more than 8-bits will be required. But if you want more, than I suggest to
> > go directly to 32-bits.
> 
> Updated.

ping nicolas.
-- 
FFmpeg = Forgiving and Fostering Mastering Proud Energized Geek