[FFmpeg-devel] [PATCH] avfilter/palettegen: export color quantization ratio

[Clément Bœsch]

On Wed, Feb 25, 2015 at 04:14:11PM +0100, Clément Bœsch wrote:
> On Wed, Feb 25, 2015 at 03:43:23PM +0100, Nicolas George wrote:
> > Le septidi 7 ventôse, an CCXXIII, Clement Boesch a écrit :
> > > +    float ratio = (float)nb_out / nb_in;
> > > +    snprintf(buf, sizeof(buf), "%f", ratio);
> > 
> > I wonder if both values could be useful instead individually. If so, either
> > set the string to %d/%d or set two metadata keys.
> > 
> 
> The numerator is the user input, so only one value is necessary. We could
> export the number of colors found in the input (nb_in), but this code is
> all about the quantization process so I think the ratio is actually more
> relevant.
> 
> > If not, use double instead of float, float is very rarely useful. And in
> > this particular case it will be converted to double by the vararg call.
> 
> Sure, OK
> 
> > 
> > > +    av_log(ctx, AV_LOG_INFO, "%d%s colors generated out of %d colors; ratio=%f\n",
> > > +           s->nb_boxes, s->reserve_transparent ? "(+1)" : "", s->nb_refs,
> > > +           set_colorquant_ratio_meta(out, s->nb_boxes, s->nb_refs));
> > 
> > This is your code, but I must say that I find returning the double value as
> > a secondary effect of a function called set_something() not very readable.
> 
> The alternative was to have the ratio declared on top of the function (far
> away), or pass the context to set_colorquant_ratio_meta() to make it log
> as well. This is really a cosmetic issue, maybe I'll just declare ratio on
> top so it's more readable.
> 

Applied with these changes.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150226/faf5a333/attachment.asc>