From 2dc0bdcaeee77ae8b40ff9eb82a9e03a7cecaf04 Mon Sep 17 00:00:00 2001 From: Jehan Date: Sat, 26 Nov 2016 01:25:49 +0100 Subject: [PATCH] Fix "all|no frames are keyframes" settings. Documentation says: "if kmin == 0, then key-frame insertion is disabled; and if kmax == 0, then all frames will be key-frames." Reading this, you'd expect that if kmax == 0, then with any kmin <= 0 all frames will be key-frames. But actually the kmin <= 0 test is caught first and you get the opposite (no keyframes but the first). You'd have instead to set kmax == 0 and any value kmin > 0, which is absolutely counter-intuitive (reversing order). Moreover kmax == 1 has no valid kmin (kmin == 1 conflicts with the `kmax > kmin` rule and kmin == 0 conflicts with `kmin >= kmax / 2 + 1`). So it should be considered an exception too. Instead I propose this new logic: - kmax == 1 means that all frames are keyframes (you are explicitly requesting a keyframe every 1 frame at most, i.e. all frames). - kmax == 0 means no keyframes (you ask for a keyframe every 0 frames, i.e. never). This is more "logical" language-wise, and also does not involve any conflicts about what if both kmax and kmin are 0, since now a single property value is meaningful for the 2 exceptional cases. Change-Id: Ia90fb963bc26904ff078d2e4ef9f74b22b13a0fd --- man/gif2webp.1 | 7 ++++--- src/mux/anim_encode.c | 9 ++++----- src/webp/mux.h | 9 +++++---- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/man/gif2webp.1 b/man/gif2webp.1 index 91e24fae..cf4b1dc9 100644 --- a/man/gif2webp.1 +++ b/man/gif2webp.1 @@ -1,5 +1,5 @@ .\" Hey, EMACS: -*- nroff -*- -.TH GIF2WEBP 1 "June 23, 2016" +.TH GIF2WEBP 1 "January 25, 2017" .SH NAME gif2webp \- Convert a GIF image to WebP .SH SYNOPSIS @@ -68,8 +68,9 @@ Specify the minimum and maximum distance between consecutive key frames some key frames into the output animation as needed so that this criteria is satisfied. .br -A 'kmin' value of 0 will turn off insertion of key frames. A 'kmax' value of 0 -will result in all frames being key frames. +A 'kmax' value of 0 will turn off insertion of key frames. A 'kmax' value of 1 +will result in all frames being key frames. 'kmin' value is not taken into +account in both these special cases. Typical values are in the range 3 to 30. Default values are kmin = 9, kmax = 17 for lossless compression and kmin = 3, kmax = 5 for lossy compression. .br diff --git a/src/mux/anim_encode.c b/src/mux/anim_encode.c index 7a388ac5..60663887 100644 --- a/src/mux/anim_encode.c +++ b/src/mux/anim_encode.c @@ -129,14 +129,13 @@ static void SanitizeEncoderOptions(WebPAnimEncoderOptions* const enc_options) { DisableKeyframes(enc_options); } - if (enc_options->kmin <= 0) { - DisableKeyframes(enc_options); - print_warning = 0; - } - if (enc_options->kmax <= 0) { // All frames will be key-frames. + if (enc_options->kmax == 1) { // All frames will be key-frames. enc_options->kmin = 0; enc_options->kmax = 0; return; + } else if (enc_options->kmax <= 0) { + DisableKeyframes(enc_options); + print_warning = 0; } if (enc_options->kmin >= enc_options->kmax) { diff --git a/src/webp/mux.h b/src/webp/mux.h index de46ee97..daccc65e 100644 --- a/src/webp/mux.h +++ b/src/webp/mux.h @@ -21,7 +21,7 @@ extern "C" { #endif -#define WEBP_MUX_ABI_VERSION 0x0107 // MAJOR(8b) + MINOR(8b) +#define WEBP_MUX_ABI_VERSION 0x0108 // MAJOR(8b) + MINOR(8b) //------------------------------------------------------------------------------ // Mux API @@ -430,9 +430,10 @@ struct WebPAnimEncoderOptions { // frames in the output. The library may insert some key // frames as needed to satisfy this criteria. // Note that these conditions should hold: kmax > kmin - // and kmin >= kmax / 2 + 1. Also, if kmin == 0, then - // key-frame insertion is disabled; and if kmax == 0, - // then all frames will be key-frames. + // and kmin >= kmax / 2 + 1. Also, if kmax <= 0, then + // key-frame insertion is disabled; and if kmax == 1, + // then all frames will be key-frames (kmin value does + // not matter for these special cases). int allow_mixed; // If true, use mixed compression mode; may choose // either lossy and lossless for each frame. int verbose; // If true, print info and warning messages to stderr.