search

frno7 / psgplay

PSG play is a music player and emulator for the Atari ST Programmable Sound Generator (PSG) YM2149.
24 stars 4 forks source link

fix for tag scanner not working with SNDHs using an rts (or similar) … #17

Closed tin-nl closed 2 years ago

tin-nl commented 2 years ago

…as one of the entry points (see e.g. http://sndh.atari.org/sndh/sndh_lf/Whittaker_David/Verminator.sndh)

(Heads up: this makes the assumption, that a bound of "0" is not wanted for a SNDH in general; not sure if a bound of 0 is/was needed because of severly broken SNDHs?)

frno7 commented 2 years ago

Thanks, @tin-nl! I’ve made a fix in commit 12ba67d92b6e667c6a1fedc1deb8d3a171e3f145 on the rts-fix branch. Does it work as you’d expect? If so, I’ll merge to the master branch. Would you like to have a Reported-By: tag with your real name and email in the commit?

tin-nl commented 2 years ago

Will check - are you sure that the size fix in tag_bound line 313++ and in branch_address of my patch shouldn't be introduced, too? This part looks to me like there's a mixup between size of the given area to check and the ending offset of the area to check. Line 280 for example wants to say "I can't deduce an offset from an opcode smaller than 2 bytes" but it says "if position is two and the size is 0 everything is OK" instead. I might have misunderstood this code's intention ofc - see the comments I introduced, did I get those parts wrong?

frno7 commented 2 years ago

Oh, you’re quite right, there’s been a confusion. Does the fix in commit 558624beb21c234206c26b7cb9d14a1d843c09ba make sense? Both offset and size are intended to refer to data. Would you like to have credit for these findings in the commits?

frno7 commented 2 years ago

Thanks for your contributions! I’ve merged the fixes mentioned so far. Refactoring with memory ranges per https://github.com/frno7/psgplay/commit/558624beb21c234206c26b7cb9d14a1d843c09ba#commitcomment-83302690 needs some more thought, as mentioned.