On 12/05/2024 14:24, Yotam Ofek wrote:
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 <t...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Thanks, pushed.
I have no idea what this was meant to do, but whatever it is, it's not
correct. To begin with, you barely have 4 bits for the length and 6/4
for the order, much less than the #SFBs. USAC even reduced the order
down to 5/3 bits.
The code should do proper RDO here using the dequantized coeffs to
figure out if TNS can help, as TNS settings themselves consume around 60
bits in total for general inputs and only make sense when you have
actual coefficients in a band after dequant, rather than a quantized
mess full of spectral holes.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
