jacquesh / foo_openlyrics

An open-source lyric display panel for foobar2000
MIT License
401 stars 24 forks source link

Crash playing a missing track on playlist #360

Closed regorxxx closed 2 weeks ago

regorxxx commented 2 months ago

Steps to reproduce

  1. Add a track into a playlist
  2. Modify the filename
  3. Play the track (which should now point to a non existent file)
  4. Foobar2000 crashes.

Expected behavior

Track skipped without errors.

Versions

Debug logs

failure_00000166.dmp failure_00000166.txt failure_00000167.dmp failure_00000167.txt failure_00000168.dmp failure_00000168.txt

Additional information

This thread points to the problem at code https://hydrogenaud.io/index.php/topic,125885.new.html#info_1043892

jacquesh commented 2 months ago

Thanks for reporting. For my own future reference: I gather that we should try-catch around https://github.com/jacquesh/foo_openlyrics/blob/master/src/tag_util.cpp#L85 (I guess just log a warning and return the empty result if it fails).

I see query_v2 does not have docs calling out that it can throw, and your report is for fb2k<2.0 so I can only assume for now that it will indeed not throw and we don't need the try{} block for the fb2kv1.x version

regorxxx commented 3 weeks ago

In case someone more has this problem and can't wait for a fix, if you are also using dead items daily. Here it is an updated version:

This one uses 1.8.1 version number https://github.com/regorxxx/foo_openlyrics/releases/tag/c And this one maintains the 1.8 version number https://github.com/regorxxx/foo_openlyrics/releases/tag/b

@jacquesh This is the commit in case you want to reuse it https://github.com/regorxxx/foo_openlyrics/commit/ef29b5dd106a1b82ae52e8d6550aae047d90ebfe

I did not include any kind of log warning since foobar2000 already warns about dead items, I see no utility on having another component warning about it in this case.

jacquesh commented 2 weeks ago

Sorry I should really be faster at fixing crashes specifically. Even before I read your message, when I saw the notification for this issue my first thought was "yeah I should probably fix that and put a new version out" >_<

New version coming soon. Thanks!

regorxxx commented 2 weeks ago

Don't worry, in my case I was just having this crash everyday multiple times (since I use foobar in a way dead items are frequent), so I really wanted to fix it asap. Thanks!