# [FFmpeg-devel] [PATCH 2/5] lavfi/gradfun: fix rounding in MMX code.

Clément Bœsch ubitux at gmail.com
Tue Dec 18 21:50:14 CET 2012

```On Tue, Dec 18, 2012 at 02:09:06PM +0000, Loren Merritt wrote:
> On Tue, 18 Dec 2012, Clément Bsch wrote:
> > On Tue, 18 Dec 2012, Clément Bsch wrote:
> >> On Fri, Dec 07, 2012 at 11:49:40PM +0000, Loren Merritt wrote:
> >>
> >>> The original order probably was about avoiding overflow, but limiting
> >>> strength is better.
> >>
> >> Just to make sure I understand the issue correctly:
> >>
> >> In the ASM, delta is expressed as u16, so delta<<2 will overflow for
> >> values > 0x3fff. In these cases we want that m, defined by:
> >>     int m = abs(delta) * thresh >> 16;     (1)
> >>     m = FFMAX(0, 127 - m);
> >> ...to be 0, so whatever the value of delta, the expression:
> >>     m = m * m * delta >> 14;
> >> ...will always be 0.
> >>
> >> To do so, we just need to make sure that abs(delta) * thresh >> 16 to be
> >> always >= 127.
> >>
> >> We currently have the threshold in the range from (1<<15)/0.51 to
> >> (1<<15)/255, with [0.51;255] being the range the user can specify. The
> >> higher the user specified threshold, the closer we get to 127 (from +oo).
> >>
> >> We need to solve:
> >>
> >>    delta * ((1<<15)/x) >> 16 >= 127 (based on (1)),
> >>      with delta > 0x3fff and x >= 0.51
> >>
> >> => delta * (1<<15)/x / (1<<16) >= 127
> >>    delta / (2x) >= 127
> >>
> >> so we have 0.51 <= x <= 2 * delta / 127
> >
> > Here is where is my mistake, the solution is 0.51 <= x <= 129/2, which is
> > indeed not in the range.
> >
> > Does a threshold > 64 makes sense?
>
> No. A threshold of 64 makes gradfun pretty much equivalent to blur. I have
> never seen a video that wanted >5.
>

OK, I'm adding the following patch to the patchset. I'll push everything
soon. Thanks everyone for the review.

Note: I get good results with the FATE sample with values in 20-30.

[...]

--
Clément B.
-------------- next part --------------
From 8567debbaf6c8b02fcab5f6471ad69eb8e7221d0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Tue, 18 Dec 2012 21:44:51 +0100
Subject: [PATCH 5/6] lavfi/gradfun: reduce up limit for threshold.

This will prevent an overflow in the SSSE3 and MMX filter_line code:
delta is expressed as an u16 being shifted by 2 to the left. If it
overflows, having a strength not above 64 will make sure that m is set
to 0 (making the m*m*delta >> 14 expression void).

A value above 64 should not make any sense unless gradfun is used as a
blur filter.
---
doc/filters.texi         | 2 +-
libavfilter/version.h    | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 827dcf3..2f50d65 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -2670,7 +2670,7 @@ The filter takes two optional parameters, separated by ':':

@var{strength} is the maximum amount by which the filter will change
any one pixel. Also the threshold for detecting nearly flat
-regions. Acceptable values range from .51 to 255, default value is
+regions. Acceptable values range from .51 to 64, default value is
1.2, out-of-range values will be clipped to the valid range.

diff --git a/libavfilter/version.h b/libavfilter/version.h
index 048cace..26b6645 100644
--- a/libavfilter/version.h
+++ b/libavfilter/version.h
@@ -30,7 +30,7 @@

#define LIBAVFILTER_VERSION_MAJOR  3
#define LIBAVFILTER_VERSION_MINOR  28
-#define LIBAVFILTER_VERSION_MICRO 100
+#define LIBAVFILTER_VERSION_MICRO 101

#define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
LIBAVFILTER_VERSION_MINOR, \
index 9255f61..6319bf7 100644
@@ -128,7 +128,7 @@ static av_cold int init(AVFilterContext *ctx, const char *args)
if (args)

-    thresh = av_clipf(thresh, 0.51, 255);
+    thresh = av_clipf(thresh, 0.51, 64);
gf->thresh = (1 << 15) / thresh;

--
1.8.0.2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121218/78ea622d/attachment.asc>
```