acelp: Fix possible endless loop when decoding amr

Message ID 1324323046-12317-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö Dec. 19, 2011, 7:30 p.m.
From: Carl Eugen Hoyos <cehoyos@ag.or.at>

Reviewed-by: Vitor Sessak
---
 libavcodec/acelp_vectors.c |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)

Comments

Ronald Bultje Dec. 19, 2011, 8:13 p.m. | #1
Hi,

On Mon, Dec 19, 2011 at 11:30 AM, Martin Storsjö <martin@martin.st> wrote:

> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
>
> Reviewed-by: Vitor Sessak
> ---
>  libavcodec/acelp_vectors.c |   22 +++++++++++++---------
>  1 files changed, 13 insertions(+), 9 deletions(-)
>

They scrape our bugzilla for bugs from our GCI students that I've analyzed
for our GCI students to pick on for their "fix a bug" tasks, and then take
authorship from it? That's just plain rude.

http://bugzilla.libav.org/show_bug.cgi?id=151

Ronald
Luca Barbato Dec. 20, 2011, 1:36 a.m. | #2
On 19/12/11 20:30, Martin Storsjö wrote:
> +        if (in->pitch_lag>  0) {

Can pitch_lag be <0 in a normal bitstream ?

lu
Ronald Bultje Dec. 20, 2011, 1:42 a.m. | #3
Hi,

On Mon, Dec 19, 2011 at 5:36 PM, Luca Barbato <lu_zero@gentoo.org> wrote:

> On 19/12/11 20:30, Martin Storsjö wrote:
>
>> +        if (in->pitch_lag>  0) {
>>
>
> Can pitch_lag be <0 in a normal bitstream ?
>

No. Even a pitch_lag of 1 or 2 is highly questionable in its meaning
although probably syntactically correct. I'd say a pitch_lag of 0 should
error out, not silently generate noise.

Ronald
Luca Barbato Dec. 20, 2011, 1:46 a.m. | #4
On 20/12/11 02:42, Ronald S. Bultje wrote:
> Hi,
>
> On Mon, Dec 19, 2011 at 5:36 PM, Luca Barbato<lu_zero@gentoo.org>  wrote:
>
>> On 19/12/11 20:30, Martin Storsjö wrote:
>>
>>> +        if (in->pitch_lag>   0) {
>>>
>>
>> Can pitch_lag be<0 in a normal bitstream ?
>>
>
> No. Even a pitch_lag of 1 or 2 is highly questionable in its meaning
> although probably syntactically correct. I'd say a pitch_lag of 0 should
> error out, not silently generate noise.

That I was wondering... Any student willing to check that and winning 
some points?

Patch

diff --git a/libavcodec/acelp_vectors.c b/libavcodec/acelp_vectors.c
index b7c05e7..be182c6 100644
--- a/libavcodec/acelp_vectors.c
+++ b/libavcodec/acelp_vectors.c
@@ -217,11 +217,13 @@  void ff_set_fixed_vector(float *out, const AMRFixed *in, float scale, int size)
         int x   = in->x[i], repeats = !((in->no_repeat_mask >> i) & 1);
         float y = in->y[i] * scale;
 
-        do {
-            out[x] += y;
-            y *= in->pitch_fac;
-            x += in->pitch_lag;
-        } while (x < size && repeats);
+        if (in->pitch_lag > 0) {
+            do {
+                out[x] += y;
+                y *= in->pitch_fac;
+                x += in->pitch_lag;
+            } while (x < size && repeats);
+        }
     }
 }
 
@@ -232,9 +234,11 @@  void ff_clear_fixed_vector(float *out, const AMRFixed *in, int size)
     for (i=0; i < in->n; i++) {
         int x  = in->x[i], repeats = !((in->no_repeat_mask >> i) & 1);
 
-        do {
-            out[x] = 0.0;
-            x += in->pitch_lag;
-        } while (x < size && repeats);
+        if (in->pitch_lag > 0) {
+            do {
+                out[x] = 0.0;
+                x += in->pitch_lag;
+            } while (x < size && repeats);
+        }
     }
 }