Fix FreeBoy CPU time bug

[messmerd]

Fixes #6649

The bug occurs because all Gb_Apu_Buffer objects, which are each stored in a NotePlayHandle's m_pluginData member, share the same m_time CPU time value from FreeBoyInstrument. So if there are two different NotePlayHandle objects in use at the same time by the same FreeBoyInstrument, such as when transitioning to a new note, FreeBoyInstrument::playNote(...) called with NotePlayHandle A may reset m_time to zero prior to calling papu->end_frame(FRAME_LENGTH). Then when FreeBoyInstrument::playNote(...) is called again, this time with NotePlayHandle B, it may call a Gb_Apu method which still remembers the Gb_Apu::last_time value as whatever m_time was before the call with NotePlayHandle A set it to zero. This last_time value is now invalid, which causes the assertion in Gb_Apu::run_until(...) to fail. For non-debug builds, this would be a silent error.

My solution to this problem is to have each Gb_Apu_Buffer store their own m_time variable rather than sharing a single one. This makes sense because each Gb_Apu_Buffer is a separate instance of the Game Boy APU, so they should be keeping track of their own CPU time values.

I also fixed the coding style in all the methods I edited and renamed Gb_Apu_Buffer to GbApuWrapper since that better reflects its purpose and follows the LMMS naming convention.

[superpaik]

This is a comment for "Refactor wrapper class" commit. Sorry, I don't quite understand how git works yet.
Style. Functions should not have spaces between brackets and variables. Like in
papu->setSampleRate( samplerate, CLOCK_RATE );
I'm not sure what's the coding convention for pointers and references, like in
long Gb_Apu_Buffer::readSamples(blip_sample_t* out, long count)
I'd say it should be
long Gb_Apu_Buffer::readSamples(blip_sample_t * out, long count)

[superpaik]

LGTM

[PhysSong]

I'm not sure if it's better to match the original code style of the submoduled upstream library. @tresf Any ideas, since you wrote this class?

[tresf]

Good question. Since the wrapper code is our own, probably our own style, but doing so can make it harder to read. Perhaps we just PR upstream with a patch that exposes the buffer, etc as a long term solution?

[messmerd]

Even with upstream changes, we'd still need a wrapper class for handling the CPU time. I don't think it's really worth it or should be within the scope of this simple bug fix PR. But after this PR is complete, I can look into upgrading to the latest version of the library and making significant structural changes like that.

[tresf]

Thanks... Yeah, my intentions aren't to delay anything. I'm no longer an active contributor to LMMS, so I'm mostly brainstorming here. The code style guidelines are a wider project decision and since the codebase has recently received a huge effort to standardize all of the codebase, it's only natural that these types of exceptions need to be decided upon.

@JohannesLorenz / @PhysSong if the decision is to maintain the submodule style, is there a quick way make this process easier (or at least more straight forward) for contributors?

For example, the switch to the submodule deliberately changed the plugin name to match our internal conventions, however some wrapper code just doesn't benefit from forced styling due to divergence from upstream.

The project should have guidelines for this so that contributions aren't being held up by missing policy.

[JohannesLorenz]

I would say: our files -> our style. Their files -> their style. If we start changing style in their submodule, we risk ugly merge errors when we update from their upstream. If we use their style in our files, we risk issues with our code check/format tools and also difficulties amongst our developers. Of course, this can mean that some of our files use mixed styles in the same source file, but we have this already with STL or Qt. Though, it's just my opinion and I am open to other suggestions.

[Veratil]

Nothing else stands out to me, and renders shared in Discord sound much better and without clicks and static.