Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

machine/t10mmc: Fix MSF to LBA conversion for T10MMC_CMD_PLAY_AUDIO_MSF #12113

Merged
merged 2 commits into from
Mar 9, 2024

Conversation

987123879113
Copy link
Contributor

This fixes audio being noticeably out of sync in pumpit1. The current dump of pumpit1 was thought to be a bad dump previously due to this bug, but T10MMC_CMD_PLAY_AUDIO_MSF had its own MSF to LBA conversion code which did not adjust the LBA value as shown in the formula below which was causing all songs played via the MSF playback command to be 2 seconds off.

ref: MMC-3 draft
image

Comment on lines 973 to +974
uint32_t msf = to_msf(m_lba);
data[data_idx] = BIT(msf, 16, 8);
data[data_idx+1] = BIT(msf, 8, 8);
data[data_idx+2] = BIT(msf, 0, 8);
put_u24be(&data[data_idx], msf);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While auditing all usages of to_msf in t10mmc when I noticed this was the only one that didn't do lba+150. I checked a mode 2 CD image I had on hand and the first sector's timestamp starts at 00:02:00, so not having lba+150 here was a bug. The new to_msf adds the +150 so this should be fine now.

@987123879113 987123879113 marked this pull request as draft March 9, 2024 03:42
@987123879113 987123879113 marked this pull request as ready for review March 9, 2024 04:29
@ajrhacker ajrhacker merged commit 1c773ba into mamedev:master Mar 9, 2024
5 checks passed
@987123879113 987123879113 deleted the t10mmc_play_audio_msf branch April 12, 2024 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants