search

a2stuff / a2d

Disassembly of the Apple II Desktop - ProDOS GUI
https://a2desktop.com
261 stars 20 forks source link

CD Remote shouldn't play on track up/down unless it's already playing #768

Closed rb6502 closed 10 months ago

rb6502 commented 10 months ago

Describe the bug CD Remote currently starts playing each track when you press track forward/backward. This should only occur if the CD is currently playing, like real CD players and Apple's CD Player apps on the IIgs and classic Mac.

Moreover, if you press track up/down when the drive is stopped (for instance, after clicking the Stop button) playback starts but the playback position doesn't update and you can't click the stop button.

To Reproduce Steps to reproduce the behavior:

  1. Start DeskTop with an Apple CD-ROM drive and SCSI card or the upcoming MAME emulation and a CD with at least one audio track in the drive
  2. Open the Extras folder and double-click "CD Remote"
  3. Click Track Up, CD Remote will try to play the first track (even if it's a data track, but that's a separate issue).
  4. Playback will continue, but you will be unable to click Stop and the time display won't update unless you then click Play.

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

System Details (please complete the following information):

Additional context Add any other context about the problem here.

inexorabletash commented 10 months ago

None of the 3 SCSI CD-ROM drives I've purchased are functional, so I'm looking forward to the MAME release!

inexorabletash commented 10 months ago

Interesting... the code seems to be doing the same thing as the GS/OS 6.0.1 source when sending the AudioSearch command, i.e. setting the first byte of the buffer to signal whether to play or hold.

https://github.com/a2stuff/a2d/blob/5ea833706e7bc24a389a0a266257926cfa79314a/desk.acc/cd.remote.s#L1715

Possibilities are:

I'll be able to diagnose further once that release of MAME is out, but if you want to see if we're sending AudioSearch with the correct or incorrect params that'd help.

rb6502 commented 10 months ago

I don't actually handle the search command right now because it's the least understandable one :-) There's an explicit Play being sent somewhere down the line when you click the Track up/down buttons, is all I know. It'd be great if someone had a working drive and could try some things.

inexorabletash commented 10 months ago

Heh, okay. That helps, I'll stare at the code some more.

inexorabletash commented 10 months ago

Does the same thing happen when the N key is pressed? Under the hood, the GUI maps button presses to key presses, and dispatches into the key handling logic. I'm not spotting an issue yet but if the same thing happens with the N key then it at least slightly narrows things down.

apple2geek commented 10 months ago

I'd be most interested to know if the "original" full-screen P8 CD Remote starts playing if you choose "track up"/"next" from a stopped state. If it does, then the behavior lies in the original code to begin with. I would hope it does not, but I don't recall ever specifically seeing that behavior tested or demonstrated, one way or the other.

My suspicion is that if the playback is being erroneously started, it's probably happening somewhere in the "C20AudioSearch" (line 1690). I have to confess, that was one of the subroutines that I had the most trouble wrapping my head around. I thought I eventually had a pretty good handle on it, but even then I wasn't 100% sure of exactly how it behaves, or is intended to behave, and I never got enough of a compatible system set up to do the deep dive I needed to figure it out fully. The documentation that I was able to find for the AudioSearch SP Control call is absolutely inadequate at best.

That routine (C20AudioSearch) and some of the "Random" play behavior routines near the end were the ones that I ultimately had to just ... assume they worked as-is, and leave them exactly as they were (please note my blunt comments on lines 2308/2396/2399).

inexorabletash commented 10 months ago

Non-update: inspection of the IIgs 6.0.1 source calling #$20 and the original CDREMOTE and our new DA all seem to call it with the same buffer, so it does seem like it's some higher level flow control issue in the code, but I still haven't spotted it yet.

Now that we understand the calls and responses a bit better expanding the FAKE_HARDWARE mode to return dummy data is a possibility, but if we get the emulator soon that'll be easier. :)

inexorabletash commented 10 months ago

Playing with this now. The issue reproduces in the original code https://github.com/a2stuff/cdremote too.

The CD Remote (DA here and original) just send #$20 (AudioSearch) when next/prev track is selected, whether playing or not. They don't send a subsequent AudioPlay command - I can BRK right after the Nth search and the playing starts up in MAME. If I explicitly send an AudioStop (assuming we're not playing) then it behaves.

When you mention "I don't actually handle the search command right now because it's the least understandable one" is the code just falling through to an AudioPlay or something? The parameters (looking at ASeekTrack in the IIgs code spdriver.aii) seem to be:

It seems like the first byte (buffer+0) is being ignored and play is initiated regardless.

If it helps here are videos of someone using the original remote on real hardware:

https://www.youtube.com/watch?v=loXFF4MgP24

And here's our CD Remote. You can see the scenario here being exercised - with playback stopped, the Next Track button is hit and playback doesn't start automatically.

https://www.youtube.com/watch?v=m8VFkwpGVOM

EDIT: buffer addresses above seem to be equivalent to scsi_cmdbuf offsets -3 (so scsi_cmdbuf[5] is buffer+2, etc)

inexorabletash commented 10 months ago

Also, it seems on startup that the remote is detecting (via a status query) that the CD is actively playing. This can be worked around by sending an explicit stop but this is not necessary on real hardware, so far as I can tell.

inexorabletash commented 10 months ago

FYI the original CD Remote which gets more video air time: https://mirrors.apple2.org.za/ftp.apple.asimov.net/images/hardware/storage/disk/misc/Apple%20II%20CD%20Setup.dsk

inexorabletash commented 10 months ago

Hmmm, and confusingly - if CD audio is currently playing then $20 AudioSearch stops audio, regardless of the state of the first buffer byte.

inexorabletash commented 10 months ago

Oh hey @rb6502 - hopefully you're typing an update somewhere. :) I was starting to look into the MAME code but if you've got an update coming let me know!

inexorabletash commented 9 months ago

Checking in @rb6502 - I'm assuming you closed this because you agree this is an emulation issue, but if I'm wrong please let me know. If it is an emulation issue, is there a pending fix for MAME? Or do we need to do research, and if so how can we help?