[FFmpeg-devel] [PATCH 1/2] libvpx: allow setting 0 for min quantizer

[Michael Niedermayer]

On Mon, Feb 25, 2013 at 01:34:29PM +0100, slhck wrote:
> Allow setting 0 for min quantizer in libvpx. Check whether CQ level is within correct
> bounds and throw error if not. Fixes #2136.
> ---
>  libavcodec/libvpxenc.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index a749e07..8d5a7ee 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -285,10 +285,17 @@ static av_cold int vpx_init(AVCodecContext *avctx,
>          }
>      }
>  
> -    if (avctx->qmin > 0)
> +    if (avctx->qmin >= 0)
>          enccfg.rc_min_quantizer = avctx->qmin;
> -    if (avctx->qmax > 0)
> +    if (avctx->qmax > 0 && avctx->qmax > avctx->qmin)
>          enccfg.rc_max_quantizer = avctx->qmax;
> +    if (ctx->crf < enccfg.rc_min_quantizer || ctx->crf > enccfg.rc_max_quantizer) {
> +        av_log(avctx, AV_LOG_ERROR,
> +               "CQ level must be between minimum and maximum quantizer value (%d-%d)\n",
> +               enccfg.rc_min_quantizer, enccfg.rc_max_quantizer);
> +        return AVERROR(EINVAL);
> +    }
> +
>      enccfg.rc_dropframe_thresh = avctx->frame_skip_threshold;
[...]
> @@ -289,11 +289,13 @@ static av_cold int vpx_init(AVCodecContext *avctx,
>          enccfg.rc_min_quantizer = avctx->qmin;
>      if (avctx->qmax > 0 && avctx->qmax > avctx->qmin)
>          enccfg.rc_max_quantizer = avctx->qmax;
> -    if (ctx->crf < enccfg.rc_min_quantizer || ctx->crf > enccfg.rc_max_quantizer) {
> -        av_log(avctx, AV_LOG_ERROR,
> -               "CQ level must be between minimum and maximum quantizer value (%d-%d)\n",
> -               enccfg.rc_min_quantizer, enccfg.rc_max_quantizer);
> -        return AVERROR(EINVAL);
> +    if (enccfg.rc_end_usage == VPX_CQ) {
> +        if (ctx->crf < enccfg.rc_min_quantizer || ctx->crf > enccfg.rc_max_quantizer) {
> +                av_log(avctx, AV_LOG_ERROR,
> +                       "CQ level must be between minimum and maximum quantizer value (%d-%d)\n",
> +                       enccfg.rc_min_quantizer, enccfg.rc_max_quantizer);
> +                return AVERROR(EINVAL);
> +        }
>      }

please dont add code in one patch that then gets fixed in the next

also each issue should be dealt with in a seperate patch
1 patch to fix the qmin bug
1 patch to add bounds checking

also qmin >= qmax should not be silently ignored

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- 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/20130225/a9ca5637/attachment.asc>