VOC_EXTENDED divide by zero crash

[noctrun]

VOC_EXTENDED divide by zero crash

[Wohlstand]

Okay, just now I made a fix, it should work fine 🦊 (at my end it now works 🤔)
@sezero , can you verify this? Is this a good fix?
e8198d6

[sezero]

If spec->channels is zero without this (didn't follow the code much), then the fix seems correct.

[Wohlstand]

After a small digging of the code, the spec begins at here:
https://github.com/WohlSoft/SDL-Mixer-X/blob/master/src/mixer.c#L715
And it seems uninitialized.

Next time it gets be initialized with zeros:
https://github.com/WohlSoft/SDL-Mixer-X/blob/master/src/codecs/load_voc.c#L390

Then I see an attempt to initialize it:
https://github.com/WohlSoft/SDL-Mixer-X/blob/master/src/codecs/load_voc.c#L401-L402

However, as we can see, when VOC file does use of VOC_EXTENDED, it gives a zero value, lemme debug that to confirm...

[Wohlstand]

Aha! I see https://github.com/WohlSoft/SDL-Mixer-X/blob/master/src/codecs/load_voc.c#L392-L393
The reason why is zero - because VOC_EXTENDED gets be read before spec->channels will be initialized.
However, the channels number should be here "uc + 1" regaring to this spec:
https://wiki.multimedia.cx/index.php?title=Creative_Voice#Block_type_0x08:_Extra_info

[Wohlstand]

@sezero, I did a new fix, it should be better now.

[sezero]

@sezero, I did a new fix, it should be better now.

It is overdoing it: are there really voc files with more than 2 chans?
(I don't think so.) The first fix seems more simple / straightforward,
it is what https://bugzilla.libsdl.org/show_bug.cgi?id=4604 recommends
too.

[sezero]

I plan on doing something limke the following in mainstream

diff --git a/src/codecs/load_voc.c b/src/codecs/load_voc.c
--- a/src/codecs/load_voc.c
+++ b/src/codecs/load_voc.c
@@ -289,6 +289,7 @@ static int voc_get_block(SDL_RWops *src,
 
                 if (uc)
                     spec->channels = 2;  /* Stereo */
+                else spec->channels = 1; /* VOC_EXTENDED may be read before spec->channels is initialized */
                 /* Needed number of channels before finishing
                    compute for rate */
                 spec->freq = (256000000L/(65536L - v->rate))/spec->channels;

[Wohlstand]

It is overdoing it: are there really voc files with more than 2 chans?

Idk, however, the specification of VOC allows that. Just for a case, I would like to follow the specification to don't return this question if some multichannel VOC files will be found and the "multichannel VOC files playing incorrectly" issue will be reported in the future. However, a small tune and making of necessary comments would be good 🦊

[Wohlstand]

btw, VLC does the same as me: https://github.com/videolan/vlc/blob/master/modules/demux/voc.c#L313
however, it does invalidating of VOC files when channels count is more than 32.

[sezero]

From the spec, top paragraph:
The VOC file format was created by Creative and is generally associated with their 8-bit line of cards (Sound Blaster, Sound Blaster Pro). While initially limited to unsigned 8-bit PCM and ADPCM data, the VOC format was eventually expanded to handle 16-bit formats with the introduction of Creative's 16-bit cards, as well as a-law and u-law companded formats.

Obviously, multichannel voc files should not have been in mind at the time. So, I am not comfortable with your second solution.

btw, VLC does the same as me: https://github.com/videolan/vlc/blob/master/modules/demux/voc.c#L313
however, it does invalidating of VOC files when channels count is more than 32.

Can it be that 32 is the max number of channels vlc allows globally for anything?? (don't know vlc internals.)

[Wohlstand]

Can it be that 32 is the max number of channels vlc allows globally for anything?? (don't know vlc internals.)

By the first look, it can output 6/8 channels maximum, but depend on the output API. I guess if that globally, the macro or global variable should be referred, however, here is a local magic number.

Obviously, multichannel voc files should not have been in mind at the time.

However, yeah if that. The first solution is fine.

[sezero]

Fixed in mainstream by http://hg.libsdl.org/SDL_mixer/rev/037c22bc945d