Add "-e rawout" to "play" a file to an OPL2/3 .RAW file

[arodland]

I'm not really a C++ dev, and I'm sure that this could really use some refactoring, but it works.

The idea is to allow someone to do adplay -e rawout -d song.raw song.rad to convert a song in some module format to the RDOS RAW format using adplug's "disk OPL" class. gamemus can then convert the RAW to formats like DRO, IMF, or VGM. -e rawout implies -O null -o since those are the options you probably want for conversion.

As part of the change, the null output driver now parses the input (calls p->update()) instead of ignoring the input and going to a non-playing state immediately.

The stuff I did with dynamic_cast is probably a bad idea, but I didn't see a better way without changing the interface of Copl, which I didn't want to take on.

[Malvineous]

Looks good but I'd rather NullOutput be left unchanged and a second class be created instead, see inline comments for details. Also looks like one change can be avoided by fixing what looks like a CDiskopl bug in the core AdPlug library.

[Malvineous]

I'm not so keen on changing NullOutput to no longer be null, as it's used elsewhere for things like getting the length of the song. If it starts processing output, then operations like retrieving the length of songs will become slower and CPU-intensive.

Would it be possible to clone NullOutput to something like DiscardOutput and apply your changes there instead?

[arodland]

Sure, that's easily done.

[mywave82]

Last commit addresses this @Malvineous

[Malvineous]

I think this is due to a bug in AdPlug. CDiskopl::update() only contains one parameter, but it should contain two in order to correctly override Copl::update().

For this I would suggest adding the second parameter to CDiskopl::update() so that this bit of the patch is no longer required.

If it works, please submit a PR to the main AdPlug library for this part.

Thanks for sending in the PR, I hope you don't mind my comments!

[arodland]

It's not just that, though, because the param it does take is a CPlayer, which is incompatible with what Copl::update does. I'm sure there's a nicer way to do things, but I'm not sure what it is.

[arodland]

(change the signature of update to require all three things, and ignore various subsets of them in different OPLs? That has a broad impact though.)

[mywave82]

CDiskopl->update is on purpose not overriding the virtual update from the Copl base class, since they are for very different things. They should had different names, but a bit late to change that now.

virtual Copl->update(short *buf, int samples) is only used when the audio chain involves an PCM audio stream:
ALSA/OSS/ESD/libao talks with CPlayer to push time and retrieve delays, and pulls audio-data from the Copl->update(buf,length) as needed.

CDiskopl has its own dance. It wants the I/O data written to Copl, but it also wants the delay information that CPlayer provides which does not fit the model that audio-stream uses.
CDiskopl->update() is the common code that the top-level classes like EmuPlayer in AdPlug/Unix would have contained.

I also agree to hijacking NullOutput is a bit dirty, and a more clean implementation would be to add it as its own Player class "DiskPlayer" that always and only is used when emulator type is Emu_Rawout.

[arodland]

Yeah, I'll happily admit that this isn't done "right". I didn't quite follow your class hierarchy and C++ isn't a language I touch very often. I'd be grateful for any amount of surgery if it gets this mergeable.

[mywave82]

They are a bit messy, and update() name is used multiple locations

[mywave82]

I am going to rebase this onto current master, and fix the indentation problems