[PATCH] vf_unsharp: provide better names for the MIN/MAX_SIZE macros
--- 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; int32_t res; int x, y, z; -- 1.7.2.5
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
On Sun, Aug 14, 2011 at 02:22:24PM +0200, Michael Niedermayer wrote:
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
hmm, seems iam a little late with finding that, your other patches already fix most of it. Still, iam a bit pissed at the quality of the original port [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No human being will ever know the Truth, for even if they happen to say it by chance, they would not even known they had done so. -- Xenophanes
On date Sunday 2011-08-14 14:28:32 +0200, Michael Niedermayer encoded:
On Sun, Aug 14, 2011 at 02:22:24PM +0200, Michael Niedermayer wrote:
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 ?
Agree.
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
hmm, seems iam a little late with finding that, your other patches already fix most of it.
Still, iam a bit pissed at the quality of the original port
Well I suppose it is partly my fault, as I reviewed the port and missed all those (pretty severe) flaws. -- FFmpeg = Funny & Faithful Multipurpose Perennial Exploitable Gangster
On date Sunday 2011-08-14 16:26:56 +0200, Stefano Sabatini encoded:
On date Sunday 2011-08-14 14:28:32 +0200, Michael Niedermayer encoded:
On Sun, Aug 14, 2011 at 02:22:24PM +0200, Michael Niedermayer wrote:
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 ?
Agree.
Updated. -- FFmpeg = Faithless & Fabulous Martial Perennial Enlightened Gadget
On Mon, Feb 06, 2012 at 03:29:33PM +0100, Stefano Sabatini wrote:
On date Sunday 2011-08-14 16:26:56 +0200, Stefano Sabatini encoded:
On date Sunday 2011-08-14 14:28:32 +0200, Michael Niedermayer encoded:
On Sun, Aug 14, 2011 at 02:22:24PM +0200, Michael Niedermayer wrote:
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 ?
Agree.
Updated.
LGTM sorry for the delay, ive missed the new message in the old thread [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong.
On date Monday 2012-03-12 02:35:08 +0100, Michael Niedermayer encoded:
On Mon, Feb 06, 2012 at 03:29:33PM +0100, Stefano Sabatini wrote:
On date Sunday 2011-08-14 16:26:56 +0200, Stefano Sabatini encoded:
On date Sunday 2011-08-14 14:28:32 +0200, Michael Niedermayer encoded:
On Sun, Aug 14, 2011 at 02:22:24PM +0200, Michael Niedermayer wrote:
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 ?
Agree.
Updated.
LGTM
sorry for the delay, ive missed the new message in the old thread
And finally applied. -- FFmpeg = Frightening and Fast Murdering Powerful Ecumenical Gnome
participants (3)
-
Michael Niedermayer -
Stefano Sabatini -
Stefano Sabatini