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

SUBSYSTEM: MIDI / OPL-Emulation #5321

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Raffaello
Copy link

If running an OPL2 track on a DOS_BOX OPL3 emulator, The buffer will be divided by 2 like if it is stereo, but it will generate a mono audio stream, as opl3Active flag is disabled, so it will act as an OPL2.
The error results in wrong sound produced.
This fix the issue when OPL3 is used as an OPL2 and therefore as a mono audio source.

The current error should be easily reproducible if using OPL3 with .ADL Westwood's files.

This bug it might have never been noticed because those files, in kyra engine, are using always OPL2 (kAdlib) is used.

If running an OPL2 track on a DOS_BOX OPL3 emulator,
The buffer will be divided by 2 like if it is stereo,
but it will generate a mono audio stream, as opl3Active
flag is disabled, so it will act as an OPL2.
The error results in wrong sound produced.
This fix the issue when OPL3 is used as an OPL2 and therefore
as a mono audio source.

The current error should be easily reproducible if
using OPL3 with .ADL Westwood's files.

This bug it might have never been noticed because those files,
in kyra engine, are using always OPL2 (kAdlib) is used.
@Raffaello
Copy link
Author

Raffaello commented Sep 21, 2023

actullally it should be something like this:
(i am not updating the PR as it might just never been merged as directly on ScummVM is not noticeable the small error and also further improvement are in development and i might not be aware of)

if(isStereo())
{
    while (length_ > 0)
    {
        const unsigned int readSamples = std::min<unsigned int>(length_, bufferLength);
        const unsigned int readSamples2 = (readSamples << 1);
        if (m_emulator->opl3Active)
        {
            m_emulator->GenerateBlock3(readSamples, tempBuffer.data());
            for (unsigned int i = 0; i < readSamples2; ++i)
                buffer[i] = static_cast<int16_t>(tempBuffer[i]);
        }
        else
        {
            m_emulator->GenerateBlock2(readSamples, tempBuffer.data());
            for (unsigned int i = 0, j =0; i < readSamples; ++i, j+=2)
            {
                buffer[j] = buffer[j+1] = static_cast<int16_t>(tempBuffer[i]);
            }
        }

        buffer += static_cast<int16_t>(readSamples2);
        length_ -= readSamples;
    }
}
else {
....
}

the main branch if checks if it is isStereo() this is required due to the mixer is initialized as stereo if not an OPL2.

within that branch if OPL3 is working as OPL2, must call the GenerateBlock2 for OPL2 compatiblity and interleave the Left and Right channels generated by the mono OPL2 to keep it sounding right.
This is required i think due to some DosBox internal channel masks as well if going looking deeper within the generating sounds.

It looks silly and overcompiclated but an OPL3 in this way can be used to run OPL2 sounds (otherwise must be initialized a DosBox OPL2 with OPL2 drivers)

anyway.
I thought it might be eventually interesting, but in ScummVM as the right OPL version is initialized for each game this small patch won't be required at the very end.
And if not using DosBox OPL emulator, for instance if using Nuked, there is no issue internally as it is fully opl2 compatible, only dosbox is kind of segmented among OPL2 and OPL3.

@antoniou79
Copy link
Contributor

antoniou79 commented Sep 22, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants