Possible bug in scalefactor calculation

Hi all,

I've received a few crash reports involving LAME through Windows Error 
Reporting (WER). This has never been actively reported by any user and I 
have no way to reproduce it, only the stack trace from the WER log:

0: _wassert in mingw-w64-crt/misc/wassert.c:54
1: assert in mingw-w64-crt/misc/wassert.c:31
2: long_block_constrain in vbrquantize.c:869
3: VBR_encode_frame in vbrquantize.c:1306
4: VBR_new_iteration_loop in quantize.c:1685

Turns out I have asserts enabled in my Windows release builds, otherwise 
this would not have come up. An assert is triggered in 
long_block_constrain, checking that vbrsf[n] is larger or equal to 
vbrsfmin[n].

This is the same issue as reported in 
https://sourceforge.net/p/lame/bugs/490/. Only difference is that there 
it was triggered in short_block_constrain.

The scalefactors are computed in block_sf(), so for this assert to 
trigger, block_sf must somehow put a higher value in vbrsfmin[] than in 
vbrsf[] for one slot. Looking at the code in block_sf(), I find one 
possible explanation:

- At least 2 iterations of the while loop are necessary:
     - 1st iteration:
         - vbrsfmin[0] = m1 = 128
         - if (sfb < psymax && w > 2) -> yes
             - if (energy_above_cutoff[sfb]) -> yes
                 - vbrsf[0] = m2 = 128
                 - m_o = 128
     - 2nd iteration:
         - vbrsfmin[1] = m1 = 129
         - if (sfb < psymax && w > 2) -> yes
             - if (energy_above_cutoff[sfb]) -> no
             - vbrsf[1] = m2 = 255
- After the while loop, m_o is checked
     - if (m_o > -1) -> yes
         - if (vbrsf[0] == 255) -> no
         - if (vbrsf[1] == 255) -> yes
             - vbrsf[1] = m_o = 128

This would leave vbrsf[1] as 128 while vbrsfmin[1] is 129, triggering 
the assert in long_block_constrain.

I'm not a codec guy, so I'm not sure if any of the above makes sense. 
Maybe this situation is supposed to be impossible to occur for other 
reasons. Looking at the code, I cannot find another way the assert might 
be triggered, but maybe someone with more codec experience can find a 
better explanation.

Attached is a proposed patch, but again, I'm not sure if this is the 
correct way to fix this. Maybe this issue goes deeper and a more 
elaborate fix is required.

Anyway, would be great if this could be investigated/fixed.

Cheers,
Robert

-- 
----
Robert Kausch
[email protected]

_______________________________________________
Lame-dev mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/lame-dev

diff -Naur lame-3.100/libmp3lame/vbrquantize.c lame-3.100-patched/libmp3lame/vbrquantize.c
--- lame-3.100/libmp3lame/vbrquantize.c	2012-02-07 12:36:35 +0000
+++ lame-3.100-patched/libmp3lame/vbrquantize.c	2020-07-06 20:32:38 +0000
@@ -478,6 +478,9 @@
         for (sfb = 0; sfb < SFBMAX; ++sfb) {
             if (vbrsf[sfb] == 255) {
                 vbrsf[sfb] = m_o;
+                if (vbrsfmin[sfb] > m_o) {
+                    vbrsfmin[sfb] = m_o;
+                }
             }
         }
     }