Re: [FFmpeg-devel] [PATCH] avcodec/aacenc_tns: remove unreachable code branch

[Yotam Ofek <yotam.ofek@gmail.com>]

Nope :)
Last commit that touched this was 9 years ago, seems like it's an oversight
that was never noticed/fixed.
The native AAC encoder is stable, even if this code was supposed to behave
differently, it's probably good enough as is, and I think that having this
redundant conditional is bad for readability and maintenance (definitely
confused me...)


On Sun, May 12, 2024 at 2:19 PM Timo Rothenpieler <timo@rothenpieler.org>
wrote:

> On 12.05.2024 10:53, Yotam Ofek wrote:
> > the condition being tested was the same as the stop condition for the
> containing loop,
> > so inside the loop it would always test positive
> > ---
> >   libavcodec/aacenc_tns.c | 7 +++----
> >   1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavcodec/aacenc_tns.c b/libavcodec/aacenc_tns.c
> > index 60888fece7..fa3cd2af39 100644
> > --- a/libavcodec/aacenc_tns.c
> > +++ b/libavcodec/aacenc_tns.c
> > @@ -181,7 +181,7 @@ void ff_aac_search_for_tns(AACEncContext *s,
> SingleChannelElement *sce)
> >
> >       for (w = 0; w < sce->ics.num_windows; w++) {
> >           float en[2] = {0.0f, 0.0f};
> > -        int oc_start = 0, os_start = 0;
> > +        int oc_start = 0;
> >           int coef_start = sce->ics.swb_offset[sfb_start];
> >
> >           for (g = sfb_start; g < sce->ics.num_swb && g <= sfb_end; g++)
> {
> > @@ -202,12 +202,11 @@ void ff_aac_search_for_tns(AACEncContext *s,
> SingleChannelElement *sce)
> >           tns->n_filt[w] = is8 ? 1 : order != TNS_MAX_ORDER ? 2 : 3;
> >           for (g = 0; g < tns->n_filt[w]; g++) {
> >               tns->direction[w][g] = slant != 2 ? slant : en[g] < en[!g];
> > -            tns->order[w][g] = g < tns->n_filt[w] ?
> order/tns->n_filt[w] : order - oc_start;
> > -            tns->length[w][g] = g < tns->n_filt[w] ?
> sfb_len/tns->n_filt[w] : sfb_len - os_start;
> > +            tns->order[w][g] = order/tns->n_filt[w];
> > +            tns->length[w][g] = sfb_len/tns->n_filt[w];
>
> Are you sure this isn't an indication that this code was meant to do
> something else, and there is actually a different issue than just dead
> code here?
>
> >               quantize_coefs(&coefs[oc_start], tns->coef_idx[w][g],
> tns->coef[w][g],
> >                               tns->order[w][g], c_bits);
> >               oc_start += tns->order[w][g];
> > -            os_start += tns->length[w][g];
> >           }
> >           count++;
> >       }
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".