drowe67 / freedv-gui

GUI Application for FreeDV – open source digital voice for HF radio
https://freedv.org/
GNU Lesser General Public License v2.1
206 stars 52 forks source link

Double click on waterfall or spectrum graph causes crash in freedv-gui #155

Closed Tyrbiter closed 3 years ago

Tyrbiter commented 3 years ago

Double left-click on either spectrum or waterfall graph in the freedv-gui window causes the program to crash

Steps to reproduce the behavior. Include code if applicable.

  1. Run freedv
  2. Start modem with start button
  3. Double left click in waterfall or spectrum graph window

Expected behavior No crashes with reasonable user behaviour.

Actual behavior I was experimenting with the left-click-to-tune tooltip which appears with the cursor over waterfall and spectrum graph windows. I could not get anything to happen with a left-click, this might mean this is a separate bug ;-) I double-clicked because my single click seemed to do nothing and I was a bit fast with the 2nd click. Immediate crash.

Attaching a gdb report with libasan installed: gdb_output_freedv.txt

Desktop (please complete the following information):

OS: Fedora Linux OS Version 34 PortAudio version: portaudio-19-36 (rebuilt from Fedora 35 src rpm, uses pa_stable_v190700_20210406.tgz, spec file edited to use pipewire-jack-audio-connection-kit-devel instead of jack-audio-connection-kit-devel which cannot be installed with pipewire-jack-audio-connection-kit providing JACK support) If Windows or Linux, which Host API (e.g. WASAPI): unsure, but pipewire 0.3.33 with JACK, alsa and pulse support.

Additional context Previously seen output in gdb from PA about illegal sample rates no longer appears with the April 2021 portaudio tarball, the current released Fedora packages are based on January 2014 portaudio with a few patches at build time to fix Fedora build-system and pipewire/JACK type bugs.

Copied from portaudio github, where I posted it in error.

Tyrbiter commented 3 years ago

Well, this is odd. I realised that the version of the freedv binary I was using had libasan.so linked even though I didn't want it to have, so I rebuilt, checked no libasan.so using ldd and re-ran under gdb

No crash!

Left click still does nothing, but double click gives this set of outputs in the gdb output:

g_TxFreqOffsetHz: 256.653931 g_RxFreqOffsetHz: -256.653931 g_TxFreqOffsetHz: -22.813721 g_RxFreqOffsetHz: 22.813721 g_TxFreqOffsetHz: -91.254761 g_RxFreqOffsetHz: 91.254761 indent! g_TxFreqOffsetHz: 0.000000 g_RxFreqOffsetHz: 0.000000 g_TxFreqOffsetHz: -496.197754 g_RxFreqOffsetHz: 496.197754 g_TxFreqOffsetHz: 11.406860 g_RxFreqOffsetHz: -11.406860 g_TxFreqOffsetHz: 11.406860 g_RxFreqOffsetHz: -11.406860 indent! g_TxFreqOffsetHz: 0.000000 g_RxFreqOffsetHz: 0.000000 g_TxFreqOffsetHz: -496.197754 g_RxFreqOffsetHz: 496.197754 g_TxFreqOffsetHz: -507.604553 g_RxFreqOffsetHz: 507.604553 indent! g_TxFreqOffsetHz: 0.000000 g_RxFreqOffsetHz: 0.000000

Not sure what the indent! lines mean.

It looks like this may not be a bug, but something to do with libasan.so slowing things down somehow, it does mention this in the gcc docs, minimum slowdown is about 2x.

tmiw commented 3 years ago

@Tyrbiter, can you try changing line 13 of util.cpp to the following:

extern int            g_split;

(note the removal of * from the above definition)

g_split is defined elsewhere as just a standard int (not pointer), so I think libasan was right to emit some sort of message--just that it's the wrong one.

Tyrbiter commented 3 years ago

Ah, just understood what g_split does, I presume it is either 1 or 0, so a pointer makes no sense.

I have also realised where the indent! message comes from. I assume it means you can recentre if you hit 1500Hz within 10Hz.

Tyrbiter commented 3 years ago

May I suggest that the "Left click to tune" tooltip in the waterfall and spectrum windows is replaced with "Double-click to tune"

I think this works if people have swapped the mouse to left-handed without needing to specify "primary mouse button" as the primary button is normally used for this sort of action.

I think this change is only needed in main.cpp

Makes it more obvious that a deliberate double click is needed to tune the modem centre frequency offset.

Including the change suggested to g_split in util.cpp this seems to have fixed this crash for me.

issue155_patch.txt

tmiw commented 3 years ago

Thanks for the patch! I created a PR to incorporate the changes. 👍