[FFmpeg-devel] [PATCH] libavcodec/qdm2.c: fix warning due to misleading indentation

Michael Niedermayer michael at niedermayer.cc
Wed Sep 28 02:17:52 EEST 2016


On Tue, Sep 27, 2016 at 09:47:32PM +0200, Adriano Pallavicino wrote:
> Sure
> 
> Adriano
> 
> 2016-09-27 21:19 GMT+02:00 Josh de Kock <josh at itanimul.li>:
> 
> > On 27/09/2016 19:46, Adriano Pallavicino wrote:
> >
> >> ---
> >>  libavcodec/qdm2.c | 117
> >> +++++++++++++++++++++++++++---------------------------
> >>  1 file changed, 59 insertions(+), 58 deletions(-)
> >>
> >> diff --git a/libavcodec/qdm2.c b/libavcodec/qdm2.c
> >> index dd8b257..e3cc902 100644
> >> --- a/libavcodec/qdm2.c
> >> +++ b/libavcodec/qdm2.c
> >> @@ -537,7 +537,7 @@ static void fill_coding_method_array(sb_int8_array
> >> tone_level_idx,
> >>          /* This case is untested, no samples available */
> >>          avpriv_request_sample(NULL, "!superblocktype_2_3");
> >>          return;
> >> -        for (ch = 0; ch < nb_channels; ch++)
> >> +        for (ch = 0; ch < nb_channels; ch++) {
> >>              for (sb = 0; sb < 30; sb++) {
> >>                  for (j = 1; j < 63; j++) {  // The loop only iterates to
> >> 63 so the code doesn't overflow the buffer
> >>                      add1 = tone_level_idx[ch][sb][j] - 10;
> >> @@ -566,67 +566,68 @@ static void fill_coding_method_array(sb_int8_array
> >> tone_level_idx,
> >>                  }
> >>                  tone_level_idx_temp[ch][sb][0] =
> >> tone_level_idx_temp[ch][sb][1];
> >>              }
> >> -            acc = 0;
> >> -            for (ch = 0; ch < nb_channels; ch++)
> >> -                for (sb = 0; sb < 30; sb++)
> >> -                    for (j = 0; j < 64; j++)
> >> -                        acc += tone_level_idx_temp[ch][sb][j];
> >> -
> >> -            multres = 0x66666667LL * (acc * 10);
> >> -            esp_40 = (multres >> 32) / 8 + ((multres & 0xffffffff) >>
> >> 31);
> >> -            for (ch = 0;  ch < nb_channels; ch++)
> >> -                for (sb = 0; sb < 30; sb++)
> >> -                    for (j = 0; j < 64; j++) {
> >> -                        comp = tone_level_idx_temp[ch][sb][j]* esp_40 *
> >> 10;
> >> -                        if (comp < 0)
> >> -                            comp += 0xff;
> >> -                        comp /= 256; // signed shift
> >> -                        switch(sb) {
> >> -                            case 0:
> >> -                                if (comp < 30)
> >> -                                    comp = 30;
> >> -                                comp += 15;
> >> -                                break;
> >> -                            case 1:
> >> -                                if (comp < 24)
> >> -                                    comp = 24;
> >> -                                comp += 10;
> >> -                                break;
> >> -                            case 2:
> >> -                            case 3:
> >> -                            case 4:
> >> -                                if (comp < 16)
> >> -                                    comp = 16;
> >> -                        }
> >> -                        if (comp <= 5)
> >> -                            tmp = 0;
> >> -                        else if (comp <= 10)
> >> -                            tmp = 10;
> >> -                        else if (comp <= 16)
> >> -                            tmp = 16;
> >> -                        else if (comp <= 24)
> >> -                            tmp = -1;
> >> -                        else
> >> -                            tmp = 0;
> >> -                        coding_method[ch][sb][j] = ((tmp & 0xfffa) + 30
> >> )&
> >> 0xff;
> >> +        }
> >> +        acc = 0;
> >> +        for (ch = 0; ch < nb_channels; ch++)
> >> +            for (sb = 0; sb < 30; sb++)
> >> +                for (j = 0; j < 64; j++)
> >> +                    acc += tone_level_idx_temp[ch][sb][j];
> >> +
> >> +        multres = 0x66666667LL * (acc * 10);
> >> +        esp_40 = (multres >> 32) / 8 + ((multres & 0xffffffff) >> 31);
> >> +        for (ch = 0;  ch < nb_channels; ch++)
> >> +            for (sb = 0; sb < 30; sb++)
> >> +                for (j = 0; j < 64; j++) {
> >> +                    comp = tone_level_idx_temp[ch][sb][j]* esp_40 * 10;
> >> +                    if (comp < 0)
> >> +                        comp += 0xff;
> >> +                    comp /= 256; // signed shift
> >> +                    switch(sb) {
> >> +                        case 0:
> >> +                            if (comp < 30)
> >> +                                comp = 30;
> >> +                            comp += 15;
> >> +                            break;
> >> +                        case 1:
> >> +                            if (comp < 24)
> >> +                                comp = 24;
> >> +                            comp += 10;
> >> +                            break;
> >> +                        case 2:
> >> +                        case 3:
> >> +                        case 4:
> >> +                            if (comp < 16)
> >> +                                comp = 16;
> >>                      }
> >> +                    if (comp <= 5)
> >> +                        tmp = 0;
> >> +                    else if (comp <= 10)
> >> +                        tmp = 10;
> >> +                    else if (comp <= 16)
> >> +                        tmp = 16;
> >> +                    else if (comp <= 24)
> >> +                        tmp = -1;
> >> +                    else
> >> +                        tmp = 0;
> >> +                    coding_method[ch][sb][j] = ((tmp & 0xfffa) + 30 )&
> >> 0xff;
> >> +                }
> >> +        for (sb = 0; sb < 30; sb++)
> >> +            fix_coding_method_array(sb, nb_channels, coding_method);
> >> +        for (ch = 0; ch < nb_channels; ch++)
> >>              for (sb = 0; sb < 30; sb++)
> >> -                fix_coding_method_array(sb, nb_channels, coding_method);
> >> -            for (ch = 0; ch < nb_channels; ch++)
> >> -                for (sb = 0; sb < 30; sb++)
> >> -                    for (j = 0; j < 64; j++)
> >> -                        if (sb >= 10) {
> >> -                            if (coding_method[ch][sb][j] < 10)
> >> -                                coding_method[ch][sb][j] = 10;
> >> +                for (j = 0; j < 64; j++)
> >> +                    if (sb >= 10) {
> >> +                        if (coding_method[ch][sb][j] < 10)
> >> +                            coding_method[ch][sb][j] = 10;
> >> +                    } else {
> >> +                        if (sb >= 2) {
> >> +                            if (coding_method[ch][sb][j] < 16)
> >> +                                coding_method[ch][sb][j] = 16;
> >>                          } else {
> >> -                            if (sb >= 2) {
> >> -                                if (coding_method[ch][sb][j] < 16)
> >> -                                    coding_method[ch][sb][j] = 16;
> >> -                            } else {
> >> -                                if (coding_method[ch][sb][j] < 30)
> >> -                                    coding_method[ch][sb][j] = 30;
> >> -                            }
> >> +                            if (coding_method[ch][sb][j] < 30)
> >> +                                coding_method[ch][sb][j] = 30;
> >>                          }
> >> +                    }
> >>      } else { // superblocktype_2_3 != 0
> >>          for (ch = 0; ch < nb_channels; ch++)
> >>              for (sb = 0; sb < 30; sb++)
> >>
> >>
> > Could you send the patch as an attachment it is corrupted.
> >
> > --
> > Josh
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >

>  qdm2.c |  117 ++++++++++++++++++++++++++++++++---------------------------------
>  1 file changed, 59 insertions(+), 58 deletions(-)
> 4700738fba8d16d1b17c420801c48470174a550f  fix_warning_qdm2_540.patch
> From c80cdc96e59c695dfc977371791f9de282f71629 Mon Sep 17 00:00:00 2001
> From: Adriano Pallavicino <adriano.pallavicino at gmail.com>
> Date: Tue, 27 Sep 2016 20:22:20 +0200
> Subject: [PATCH] Fix warning error due to missing {} around for loop
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> libavcodec/qdm2.c:540:9: warning: this ‘for’ clause does not guard... [-Wmisleading-indentation]
>          for (ch = 0; ch < nb_channels; ch++)
> ---
>  libavcodec/qdm2.c | 117 +++++++++++++++++++++++++++---------------------------
>  1 file changed, 59 insertions(+), 58 deletions(-)
> 
> diff --git a/libavcodec/qdm2.c b/libavcodec/qdm2.c
> index dd8b257..e3cc902 100644
> --- a/libavcodec/qdm2.c
> +++ b/libavcodec/qdm2.c
> @@ -537,7 +537,7 @@ static void fill_coding_method_array(sb_int8_array tone_level_idx,
>          /* This case is untested, no samples available */
>          avpriv_request_sample(NULL, "!superblocktype_2_3");
>          return;

LGTM, the code below is unreachable and untested due to lack of a
sample

[...]

-- 
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160928/ae8b087c/attachment.sig>


More information about the ffmpeg-devel mailing list