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

[Stefano Sabatini]

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.
-- 
FFmpeg = Faithless Fanciful Muttering Puritan Elfic Gymnast
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavu-parseutils-add-av_get_color_name.patch
Type: text/x-diff
Size: 2005 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131021/12dd8b66/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-cmdutils-add-colors-option.patch
Type: text/x-diff
Size: 2989 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131021/12dd8b66/attachment-0001.bin>