Re: [FFmpeg-devel] [PATCH] aacpsy: avoid norm_fac becoming NaN

2015-04-16 Thread Claudio Freire
On Thu, Apr 16, 2015 at 1:00 PM, Andreas Cadhalpun
andreas.cadhal...@googlemail.com wrote:
 If both band-active_lines and band-thr are 0.0f, the division is
 undefined, making norm_fac not a number.

 NaN is passed on to other variables until it finally reaches
 sce-sf_idx and is converted to an integer (-2147483648).

 This causes a segmentation fault when it is used as array index.

 Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
 ---
  libavcodec/aacpsy.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

 diff --git a/libavcodec/aacpsy.c b/libavcodec/aacpsy.c
 index d1e65f6..b71933b 100644
 --- a/libavcodec/aacpsy.c
 +++ b/libavcodec/aacpsy.c
 @@ -727,7 +727,10 @@ static void psy_3gpp_analyze_channel(FFPsyContext *ctx, 
 int channel,
  if (active_lines  0.0f)
  band-thr = calc_reduced_thr_3gpp(band, 
 coeffs[g].min_snr, reduction);
  pe += calc_pe_3gpp(band);
 -band-norm_fac = band-active_lines / band-thr;
 +if (band-active_lines != 0.0f)
 +band-norm_fac = band-active_lines / band-thr;
 +else
 +band-norm_fac = 0.0f;
  norm_fac += band-norm_fac;
  }
  }


It should be if band-thr  0.0f, all divisions by zero return
something that casts into an ~1:

Try:

#include stdio.h
int main() {
   printf(%d\n, int(0.0f / 0.0f));
   printf(%d\n, int(1.0f / 0.0f));
   return 0;
}
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] aacpsy: avoid norm_fac becoming NaN

2015-04-16 Thread Timothy Gu
On Thu, Apr 16, 2015 at 10:41 AM Claudio Freire klaussfre...@gmail.com
wrote:

 It should be if band-thr  0.0f, all divisions by zero return
 something that casts into an ~1:

Isn't division by zero undefined behavior?

Timothy
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] aacpsy: avoid norm_fac becoming NaN

2015-04-16 Thread Michael Niedermayer
On Thu, Apr 16, 2015 at 05:47:50PM +, Timothy Gu wrote:
 On Thu, Apr 16, 2015 at 10:41 AM Claudio Freire klaussfre...@gmail.com
 wrote:
 
  It should be if band-thr  0.0f, all divisions by zero return
  something that casts into an ~1:
 
 Isn't division by zero undefined behavior?

for floats its defined by IEEE754 if the implementation supports that
if iam not mistaken

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

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] aacpsy: avoid norm_fac becoming NaN

2015-04-16 Thread Michael Niedermayer
On Thu, Apr 16, 2015 at 02:41:39PM -0300, Claudio Freire wrote:
 On Thu, Apr 16, 2015 at 1:00 PM, Andreas Cadhalpun
 andreas.cadhal...@googlemail.com wrote:
  If both band-active_lines and band-thr are 0.0f, the division is
  undefined, making norm_fac not a number.
 
  NaN is passed on to other variables until it finally reaches
  sce-sf_idx and is converted to an integer (-2147483648).
 
  This causes a segmentation fault when it is used as array index.
 
  Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
  ---
   libavcodec/aacpsy.c | 5 -
   1 file changed, 4 insertions(+), 1 deletion(-)
 
  diff --git a/libavcodec/aacpsy.c b/libavcodec/aacpsy.c
  index d1e65f6..b71933b 100644
  --- a/libavcodec/aacpsy.c
  +++ b/libavcodec/aacpsy.c
  @@ -727,7 +727,10 @@ static void psy_3gpp_analyze_channel(FFPsyContext 
  *ctx, int channel,
   if (active_lines  0.0f)
   band-thr = calc_reduced_thr_3gpp(band, 
  coeffs[g].min_snr, reduction);
   pe += calc_pe_3gpp(band);
  -band-norm_fac = band-active_lines / band-thr;
  +if (band-active_lines != 0.0f)
  +band-norm_fac = band-active_lines / band-thr;
  +else
  +band-norm_fac = 0.0f;
   norm_fac += band-norm_fac;
   }
   }
 
 

 It should be if band-thr  0.0f,

ok, thanks


 all divisions by zero return
 something that casts into an ~1:

casting NaN or any out of int range value to int is undefined AFAIK

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

The real ebay dictionary, page 3
Rare item - Common item with rare defect or maybe just a lie
Professional - 'Toy' made in china, not functional except as doorstop
Experts will know - The seller hopes you are not an expert


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] aacpsy: avoid norm_fac becoming NaN

2015-04-16 Thread Andreas Cadhalpun
On 16.04.2015 19:41, Claudio Freire wrote:
 It should be if band-thr  0.0f, all divisions by zero return
 something that casts into an ~1:

OK, new patch attached.

If band-thr is 0.0f and band-active_lines is not, it takes
a bit longer to get to NaN.
I assume band-thr  0.0f is not possible?

Best regards,
Andreas

From e9124e077f1e8fcef0669ceff2fc05116b942b5d Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun andreas.cadhal...@googlemail.com
Date: Thu, 16 Apr 2015 20:04:54 +0200
Subject: [PATCH] aacpsy: avoid psy_band-threshold becoming NaN

If band-thr is 0.0f, the division is undefined, making norm_fac not a
number or infinity, which causes psy_band-threshold to become NaN.

This is passed on to other variables until it finally reaches
sce-sf_idx and is converted to an integer (-2147483648).

This causes a segmentation fault when it is used as array index.

Signed-off-by: Andreas Cadhalpun andreas.cadhal...@googlemail.com
---
 libavcodec/aacpsy.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavcodec/aacpsy.c b/libavcodec/aacpsy.c
index d1e65f6..7205ee3 100644
--- a/libavcodec/aacpsy.c
+++ b/libavcodec/aacpsy.c
@@ -727,7 +727,10 @@ static void psy_3gpp_analyze_channel(FFPsyContext *ctx, int channel,
 if (active_lines  0.0f)
 band-thr = calc_reduced_thr_3gpp(band, coeffs[g].min_snr, reduction);
 pe += calc_pe_3gpp(band);
-band-norm_fac = band-active_lines / band-thr;
+if (band-thr  0.0f)
+band-norm_fac = band-active_lines / band-thr;
+else
+band-norm_fac = 0.0f;
 norm_fac += band-norm_fac;
 }
 }
-- 
2.1.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] aacpsy: avoid norm_fac becoming NaN

2015-04-16 Thread Claudio Freire
On Thu, Apr 16, 2015 at 3:19 PM, Andreas Cadhalpun
andreas.cadhal...@googlemail.com wrote:
 On 16.04.2015 19:41, Claudio Freire wrote:
 It should be if band-thr  0.0f, all divisions by zero return
 something that casts into an ~1:

 OK, new patch attached.

 If band-thr is 0.0f and band-active_lines is not, it takes
 a bit longer to get to NaN.
 I assume band-thr  0.0f is not possible?

It's not, it's an absolute energy threshold.

LGTM now.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] aacpsy: avoid norm_fac becoming NaN

2015-04-16 Thread Michael Niedermayer
On Thu, Apr 16, 2015 at 03:26:55PM -0300, Claudio Freire wrote:
 On Thu, Apr 16, 2015 at 3:19 PM, Andreas Cadhalpun
 andreas.cadhal...@googlemail.com wrote:
  On 16.04.2015 19:41, Claudio Freire wrote:
  It should be if band-thr  0.0f, all divisions by zero return
  something that casts into an ~1:
 
  OK, new patch attached.
 
  If band-thr is 0.0f and band-active_lines is not, it takes
  a bit longer to get to NaN.
  I assume band-thr  0.0f is not possible?
 
 It's not, it's an absolute energy threshold.
 
 LGTM now.

applied

thanks

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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel