Program hangs when changing subtune, exSID

[yxkalle]

I'm using Windows and ftd2xx.dll (the same issue with libftdi1.dll, though). As the title says, the program hangs when changing subtunes or reaches the end of the current one. I have to unplug/replug the USB cable to be able to start the program again.

[drfiemost]

Confirmed, it sometimes hangs during Device setup in Linux too:

$ sidplayfp --exsid  $HVSC_BASE/MUSICIANS/T/Tel_Jeroen/Myth.sid 
(exSID_init) libexsid v2.1.0
(xSfw_dlopen) Using libftdi
(exSID_init) Trying exSID USB...
(exSID_init) Opened!
(exSID_init) Device setup

this is the backtrace

#0  0x00007ffff64b1f1e in ?? () from /usr/lib/libc.so.6
libsidplayfp/sidplayfp#1  0x00007ffff64a6cb8 in ?? () from /usr/lib/libc.so.6
libsidplayfp/sidplayfp#2  0x00007ffff64a6cfd in ?? () from /usr/lib/libc.so.6
libsidplayfp/sidplayfp#3  0x00007ffff651a9c6 in poll () from /usr/lib/libc.so.6
libsidplayfp/sidplayfp#4  0x00007ffff68d7ba8 in usbi_wait_for_events () from /usr/lib/libusb-1.0.so.0
libsidplayfp/sidplayfp#5  0x00007ffff68d4e3c in handle_events () from /usr/lib/libusb-1.0.so.0
libsidplayfp/sidplayfp#6  0x00007ffff68d64a8 in libusb_handle_events_timeout_completed () from /usr/lib/libusb-1.0.so.0
libsidplayfp/sidplayfp#7  0x00007ffff68d654a in libusb_handle_events_completed () from /usr/lib/libusb-1.0.so.0
libsidplayfp/sidplayfp#8  0x00007ffff68d7221 in sync_transfer_wait_for_completion () from /usr/lib/libusb-1.0.so.0
libsidplayfp/sidplayfp#9  0x00007ffff68d734e in do_sync_bulk_transfer () from /usr/lib/libusb-1.0.so.0
libsidplayfp/sidplayfp#10 0x00007ffff68d76ff in libusb_bulk_transfer () from /usr/lib/libusb-1.0.so.0
libsidplayfp/sidplayfp#11 0x00007ffff7fb3e63 in ftdi_read_data () from /usr/lib/../lib/libftdi1.so
libsidplayfp/sidplayfp#12 0x00007ffff6a3ced3 in exSID_init () from /usr/lib/libsidplayfp.so.6
libsidplayfp/sidplayfp#13 0x00007ffff6a3c8c5 in libsidplayfp::exSID::exSID (this=this@entry=0x555555638a20, builder=builder@entry=0x5555555b8bb0)
    at ../src/builders/exsid-builder/exsid-emu.cpp:56
libsidplayfp/sidplayfp#14 0x00007ffff6a3be28 in exSIDBuilder::create (this=0x5555555b8bb0, sids=1) at ../src/builders/exsid-builder/exsid-builder.cpp:68
libsidplayfp/libsidplayfp#66 0x000055555557004b in ?? ()
libsidplayfp/sidplayfp#16 0x0000555555570461 in ?? ()
libsidplayfp/sidplayfp#17 0x000055555555e268 in ?? ()
libsidplayfp/sidplayfp#18 0x00007ffff6441ce8 in ?? () from /usr/lib/libc.so.6
libsidplayfp/sidplayfp#19 0x00007ffff6441da5 in __libc_start_main () from /usr/lib/libc.so.6
libsidplayfp/sidplayfp#20 0x000055555555ee61 in ?? ()

Maybe an issue in the exSID driver, @f00b4r0 ?

exsid-driver/exSID.c

Lines 445 to 450 in 3749d6a

	// Wait for device ready by trying to read hw version and wait for the answer
	buf[0] = XS_AD_IOCTHV;
	buf[1] = XS_AD_IOCTFV; // ok as we have a 2-byte RX buffer on PIC
	xSwrite(xs, buf, 2);
	xSread(xs, buf, 2);
	xs->hwvers = buf[0] << 8 | buf[1]; // ensure proper order regardless of endianness

[f00b4r0]

It's not immediately obvious to me what could be going on.

Considering that the code has been unchanged for the last 5y or so and worked flawlessly (or so it seemed) until now, and considering that sidplayfp shows up pretty deep into the call stack, I wouldn't necessarily throw the code under the bus just yet.

It's quite possible something changed in libusb which now triggers this issue.

Having a full debug symbol backtrace (including the topmost libc6 frames) would be helpful to get a better sense of what is actually going on, however I have unfortunately zero time to dedicate to this right now 😔

[f00b4r0]

@yxkalle Also is this a random problem or does it always happen?

[yxkalle]

@yxkalle Also is this a random problem or does it always happen?

It always happens, but I'm not sure about my build chain?

[yxkalle]

@f00b4r0 ?

This is exactly where it hangs for me too. Commenting out these lines let me change tunes, but the SID gets quiet 9/10 times.. :)

[f00b4r0]

As I said, this code hasn't changed in years so something elsewhere must be the cause. I don't understand how reading the device version could hang the software so I'm not sure there anything wrong with the exsid driver code here. Maybe @drfiemost can provide a more complete trace so we can figure this out?

[yxkalle]

I have made a fix for the issue. I cross-referenced (word?) jsidplay2 and saw this:
https://sourceforge.net/p/jsidplay2/code/HEAD/tree/trunk/jsidplay2/src/main/java/builder/jexsid/ExSIDEmu.java
...a much longer delay than is used in this project. (50ms vs 100µs).
I also borrowed some FTDI code for buffer purging. I don't think it hurts...

My fork is here: libsidplayfp

I let You check it out before I (if greenlit) make a PR.

[drfiemost]

Cool! I will give it a try this weekend, thanks.

Moving the issue to the exsid repo.

[f00b4r0]

@yxkalle thanks can you open a PR against this repo please so I can review and comment on your changes?

However from a very quick glance at your repo I can already tell you I'm seeing too many unrelated changes for this fix so please keep the changes to the absolute minimum required (in particular do not change the API, I don't want to have to deal with an ABI update) and keep the commits atomic.

[drfiemost]

I can confirm it's working. 👍🏻
Just had an error while compiling:

libtool: compile:  gcc -DHAVE_CONFIG_H -I. -Wall -fstrict-aliasing -I/usr/include/libftdi1 -I/usr/include/libusb-1.0 -pthread -DNDEBUG -I/home/drfiemost/prova/include -march=native -O2 -finline-functions -fomit-frame-pointer -floop-interchange -floop-strip-mine -floop-block -floop-parallelize-all -ftree-vectorize -ftracer -pipe -MT exSID.lo -MD -MP -MF .deps/exSID.Tpo -c exSID.c  -fPIC -DPIC -o .libs/exSID.o
exSID.c: In function 'exSID_reset':
exSID.c:584:42: warning: missing terminating ' character
  584 |         _exSID_write(xs, 0x18, volume, 50'000);
      |                                          ^
exSID.c:584:42: error: missing terminating ' character
  584 |         _exSID_write(xs, 0x18, volume, 50'000);
      |                                          ^~~~~~
exSID.c:584:42: error: expected ')' before 'xs'
  584 |         _exSID_write(xs, 0x18, volume, 50'000);
      |                     ~                    ^
      |                                          )
  585 |         xs->clkdrift = 0;
      |         ~~                                
exSID.c:587:18: error: expected ';' before '}' token
  587 |         return 0;
      |                  ^
      |                  ;
  588 | }
      | ~                 
exSID.c:588:1: warning: control reaches end of non-void function [-Wreturn-type]
  588 | }
      | ^
make[1]: *** [Makefile:536: exSID.lo] Error 1

Anyway I agree with Thibaut it's better to maintain backward compatibility, if really necessary a new API can be added but the old should stay untouched.

[yxkalle]

@drfiemost

I can confirm it's working. 👍🏻 Just had an error while compiling:

Guess your compiler didn't like single quotes as thousands-delimiter in the source files. I read that it requires C++ 14. Maybe that's too blame. Doesn't matter, this code will be discarded.

I agree too. It's too invasive. I was just playing around in the code to try to learn stuff, I guess. Bad habit.
I will submit a minimal PR soon. You could do it too, if you're eager! I think you know what to do. ;)