[FFmpeg-devel] [PATCH] fixed granularity of video quality when encoding with theora codec

[Nicolas George]

Le quintidi 15 nivôse, an CCXXI, Maximilian Seesslen a écrit :
> Subject: Re: [FFmpeg-devel] [PATCH] fixed granularity of video quality when
>  encoding with theora codec

Nit.: "libtheoraenc: fix granularity of video quality."

> A floating point version of av_clip has to be used when converting the quality level.
> 
> Signed-off-by: Maximilian Seesslen <mes at seesslen.net>
> ---
>  libavcodec/libtheoraenc.c |    2 +-
>  libavutil/common.h        |   14 ++++++++++++++
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/libtheoraenc.c b/libavcodec/libtheoraenc.c
> index 1419723..350ac67 100644
> --- a/libavcodec/libtheoraenc.c
> +++ b/libavcodec/libtheoraenc.c
> @@ -212,7 +212,7 @@ static av_cold int encode_init(AVCodecContext* avc_context)
>                  * 0 <= p <=63
>                  * an int value
>           */
> -        t_info.quality        = av_clip(avc_context->global_quality / (float)FF_QP2LAMBDA, 0, 10) * 6.3;
> +        t_info.quality        = av_fclip(avc_context->global_quality / (float)FF_QP2LAMBDA, 0, 10) * 6.3;
>          t_info.target_bitrate = 0;
>      } else {
>          t_info.target_bitrate = avc_context->bit_rate;
> diff --git a/libavutil/common.h b/libavutil/common.h
> index 0f36309..c9e843b 100644
> --- a/libavutil/common.h
> +++ b/libavutil/common.h
> @@ -103,6 +103,20 @@ static av_always_inline av_const int av_clip_c(int a, int amin, int amax)
>  }
>  
>  /**

> + * Clips a float value into the amin-amax range.
> + * @param a value to clip
> + * @param amin minimum value of the clip range
> + * @param amax maximum value of the clip range
> + * @return clipped value
> + */
> +static inline av_const float av_fclip(float a, float amin, float amax)

IMHO, using floats instead of doubles is rarely justified, except in big
arrays and structures to reduce memory consumption, but I may be mistaken.

Also, introducing a new API in libavutil is not a trivial fact, it deserves
at least a prominent mention in the commit message, and ideally a separate
patch, an entry in doc/APIChanges and a minor version bump.

> +{
> +    if      (a < amin) return amin;
> +    else if (a > amax) return amax;
> +    else               return a;
> +}
> +
> +/**
>   * Clip a signed 64bit integer value into the amin-amax range.
>   * @param a value to clip
>   * @param amin minimum value of the clip range

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130104/c688db6a/attachment.asc>