[FFmpeg-devel] [PATCH] vf_unsharp: provide better names for the MIN/MAX_SIZE macros

[Michael Niedermayer]

On Sat, Aug 13, 2011 at 01:11:42AM +0200, Stefano Sabatini wrote:
> ---
>  libavfilter/vf_unsharp.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/libavfilter/vf_unsharp.c b/libavfilter/vf_unsharp.c
> index b770437..e41e76f 100644
> --- a/libavfilter/vf_unsharp.c
> +++ b/libavfilter/vf_unsharp.c
> @@ -41,8 +41,9 @@
>  #include "libavutil/mem.h"
>  #include "libavutil/pixdesc.h"
>  
> -#define MIN_SIZE 3
> -#define MAX_SIZE 13
> +#define MATRIX_MIN_LINE_SIZE 3
> +#define MATRIX_MAX_LINE_SIZE 13
> +#define MATRIX_MAX_SIZE (MATRIX_MAX_LINE_SIZE * MATRIX_MAX_LINE_SIZE)
>
>  /* right-shift and round-up */
>  #define UPSHIFT(x,shift) (-((-(x))>>(shift)))
> @@ -55,7 +56,7 @@ typedef struct FilterParam {
>      int steps_y;                             ///< vertical step count
>      int scalebits;                           ///< bits to shift pixel
>      int32_t halfscale;                       ///< amount to add to pixel
> -    uint32_t *sc[(MAX_SIZE * MAX_SIZE) - 1]; ///< finite state machine storage
> +    uint32_t *sc[MATRIX_MAX_SIZE - 1]; ///< finite state machine storage
>  } FilterParam;
>  
>  typedef struct {

> @@ -69,7 +70,7 @@ static void apply_unsharp(      uint8_t *dst, int dst_stride,
>                            int width, int height, FilterParam *fp)
>  {
>      uint32_t **sc = fp->sc;
> -    uint32_t sr[(MAX_SIZE * MAX_SIZE) - 1], tmp1, tmp2;
> +    uint32_t sr[MATRIX_MAX_SIZE - 1], tmp1, tmp2;

The original code in libmpcodecs makes more sense to me then both the
before and afterwards of this patch.
why was this ever changed to MAX_SIZE * MAX_SIZE ?

also the saftey checks on the parameters have been lost in the port
from libmpcodecs, its exploitable now, try -vf unsharp=999:3:3 as
example

and at that, the command line syntax was more readable in libmpcodecs
too, and is incompatibel now
  l7x5:0.8:c3x3:-0.2
  vs
  7:5:0.8:3:3:0.2

What do you think is better?
Should the port be redone from scratch?
Or do we have a volunteer to go over the code line by line to find out
what has been changed and make sure that each change is harmless?

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

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- 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/20110814/a027d084/attachment.asc>