DRAFT: IAGSAudioPlayer plugin api

[ivan-mogilko]

Adds a IAGSAudioPlayer api for plugins, which wraps our OpenALSource object, and lets render any arbitrary sound data using engine's audio output.

This is immediately for the sake of the "Sprite Video" plugin, which cannot play sounds at the moment, having no audio output of its own. But of course this may have any other potential future uses, such as playing custom sounds from plugins.

I referred to the conversation in #1981 when organizing api.

The practical use of this may be found in this experimental branch of the "Sprite Video" plugin:
https://github.com/ivan-mogilko/ags-spritevideo/tree/experiment--agsaudioplayer

New API:

#define AGS_AUDIOPLAY_FLD_VOLUME        0x0001

#define AGS_AUDIOFRAME_FLD_DATA         0x0001
#define AGS_AUDIOFRAME_FLD_TIMESTAMP    0x0002

// use sdl2 format for test, but maybe switch to our own custom values just in case?
#define AGS_AUDIOFORMAT_F32             0x8120

struct AGSAudioFormat
{
  // Set which fields are valid, see AGS_AUDIOFORMAT_FLD_* flags
  uint32_t Fields;
  int Format; // s16, s32, f16, f32, etc
  int Channels;
  int Freq;
};

struct AGSAudioPlayConfig
{
  // Set which fields are valid, see AGS_AUDIOPLAY_FLD_* flags
  uint32_t Fields;
  float Volume;
};

struct AGSAudioFrame
{
  // Set which fields are valid, see AGS_AUDIOFRAME_FLD_* flags
  uint32_t Fields;
  void *Data;
  size_t DataSize;
  int64_t Timestamp;
};

// Something that plays the sound data
class IAGSAudioPlayer {
public:
  // Tells which version of the plugin API this object corresponds to;
  // this lets users know which of the following methods are valid to use.
  virtual int    GetVersion() = 0;
  virtual void   SetConfig(AGSAudioPlayConfig *config) = 0;
  // Flushes and closes the AudioPlayer.
  // After calling this the IAGSAudioPlayer pointer becomes INVALID.
  virtual void   Close() = 0;
  virtual void   Pause() = 0;
  virtual void   Resume() = 0;
  // *copies* data on call, does not hold the frame (??? need different rule?)
  // returns amount of data copied, or 0 if data cannot be accepted at the moment.
  virtual size_t PutData(AGSAudioFrame *frame) = 0;

protected:
  IAGSAudioPlayer() = default;
  ~IAGSAudioPlayer() = default;
};

and...

class IAGSEngine {
  <...>

  // open audio player, telling the format of the sound data that will be SENT into this player by user.
  // optionally pass AGSAudioPlayConfig setting playback parameters (these may be reset later).
  AGSIFUNC(IAGSAudioPlayer*) OpenAudioPlayer(AGSAudioFormat *in_format, AGSAudioPlayConfig *config);
}

[ivan-mogilko]

Although formally working, there are few things that I do not like in this approach. Primarily, my mistake is that engine implements this interface as a specialized wrapper around existing "player" (OpenALSource).
The proper approach would be instead for the engine to use the "audioplayer" interface itself right in the audioslot, and implement this interface in OpenALSource class. Then similarly make exposable interface for the audio decoder and implement it in SDLDecoder.
This way we would both be able to export these to plugins, and be potentially ready to use custom implementations from plugins, if that will be wanted in a future.

This will also be in line with existing interfaces such as FontRenderer, which is used by the engine directly.

[ericoporto]

Uhm, the FontRenderer is an external thing that renders text and the Engine gets to use but this AudioPlayer is an external thing that (possibly decodes audio and) puts the audio data for the Engine to play. So the FontRenderer is for you to implement a font renderer and the audio player is for you to implement an audio ??? - a decoder?

What I mean is they look like different things. This interface is for you to implement a new in-game audio "source" - which in the example usage, comes from the video plugin.

[ivan-mogilko]

the FontRenderer is an external thing that renders text and the Engine gets to use

Not necessarily external, engine has two internal font renderers: WFNFontRenderer and TTFFontRenderer. Both implement IAGSFontRenderer interface, identical to the plugin's. This allows the engine to use a font renderer similarly regardless of what it is (with some small exceptions).

What I mean is they look like different things. This interface is for you to implement a new in-game audio "source" - which in the example usage, comes from the video plugin.

I mean that internal audio player (currently - OpenALSource) could potentially be also switched by something given by plugin.
(This is not achievable with the current PR, but something that may potentially be supported in future.)
Same if I add IAGSAudioDecoder (and other parts mentioned in #1981), in ideal way engine should be able to replace internal decoders, players, and any other parts of the video and audio pipeline with the ones implemented by plugin. This may be achieved only if engine uses these exact interfaces inside its video/audio playback logic.

EDIT:
In simple words, I'm looking for a solution where every part of the audio and video processing may be potentially replaceable, and come either from engine or plugin.

[ericoporto]

I mean that internal audio player (currently - OpenALSource) could potentially be also switched by something given by plugin

This is more sort of what I meant, the OpenALSource is the audio player but the interface named AudioPlayer is not for it, but for someone implementing their own AudioClip+Decoder of sorts - you read the audio file from the video in the plugin, decode and push the results here.

So when you compare with FontRenderer it's confusing because it's meant for you to implement your own FontRenderer.

I'm looking for a solution where every part of the audio and video processing may be potentially replaceable

I think we would have to break down this pieces in an issue, the only catch is in Audio we have the decoding and mixing separately, so we can today have these split, but if later these are replaced by SDL_mixer, which does both in the same package we would have to do some adjustments - although still could work if ones decoder fed pcm chunks or similar which I think SDL_mixer supports, so perhaps this point is not an issue at all.

[ivan-mogilko]

This is more sort of what I meant, the OpenALSource is the audio player but the interface named AudioPlayer is not for it, but for someone implementing their own AudioClip+Decoder of sorts - you read the audio file from the video in the plugin, decode and push the results here.

I'm not sure, but maybe the misunderstanding is because I imply also hypothetical replacement of these parts inside the engine. If somebody implements their own IAudioPlayer, then in order to use it inside the engine, then engine would also need to use this interface IAudioPlayer, and not OpenALSource directly.

Same of decoder, if engine would need to use a decoder implemented from an interface by someone else, they would need to be using that interface, and not class SDLDecoder directly.

In other words, instead of using a chain SDLDecoder -> OpenALSource, like it does now, engine would need to have a IAudioDecoder -> IAudioPlayer chain (or, going further, IAudioDecoder -> IAudioFilter -> IAudioPlayer, or similar), where IAudioDecoder is SDLDecoder by default,and IAudioPlayer is OpenALSource by default.

So when you compare with FontRenderer it's confusing because it's meant for you to implement your own FontRenderer.

Engine uses IAGSFontRenderer interface, which may have either internal (WFN, TTF) implementation, or a plugin's implementation.

[ivan-mogilko]

I'd like to repost my notes on a problem of "disposing" the objects involved in plugin api, which initially were posted in the comments to already closed stream api PR: #2227 (comment)
for the record, as they are also be related to this pr, and practically to any future similar api, where some object may be provided from engine to plugins and other way, and that object's ownership is passed along. In other words, when the receiver must delete the object after use.

Specifics of dynamically loaded plugin is that object allocated in plugin must be deallocated in plugin as well, same goes for the object allocated in the engine (it must be deallocated in the engine). The reason for this is that in theory engine and plugin may be compiled using different compilers, and even have different standard lib linked to them. This is why these objects cannot be deleted with "delete" outside of the module that created them.
This leads to a requirement that there has to be factory methods for creating and destroying each object type. If an object is created in the engine, then engine api must have "create"/"destroy" functions. If an object is created by plugin, then plugin should also provide "create"/"destroy" functions for this object type.
(This refers only to dynamically created objects which ownership is passed along with them. Anything that engine or plugin passes but does not give ownership for do not need this.)

While "create" function is always a part of the factory interface, there are two approaches to "destroy" function:

1. Have it in a factory interface as well, with the object passed as an argument,
2. Have it in an object interface itself, so called from that object.

Having "destroy" function in an object's interface has a advantage of easy access, because all you need is to have the object pointer itself. But if we want to pass internal engine's objects out to plugins, then we also would have to implement this "destroy" method in them. In other words, our own engine class should have a method that deletes object itself. This may be contradicting the class design, and not look pretty. This may also be not safe if the object of such class can be created as a local variable on stack, because calling that function then would corrupt the stack.

Possible solutions here are:

• Enforce that this object cannot be created on stack. (This may be done by hiding its constructor and providing factory method returning the pointer.) This will change how the class is used in the program. Still the "destroy" method has to be commented with a warning that you should use either delete or this method, but not both. Of course if the object in some particular place in the engine may theoretically come from plugin, then only "destroy" method must be used.
• Hide this "destroy" method itself by making it private in the class. It will be public in the interface, but private in the implementation. This means that if you have an object as a interface pointer, then you may use the "destroy" method to dispose it. And at the same time you may create a subclass object on stack, and the "destroy" method won't be accessible.
• Write a wrapper over the engine's object, which replicates the base class methods, but adds just this extra "destroy" function. The disadvantage of wrappers is that they may make code more cumbersome, and likely less efficient by adding another layer (although the actual impact may vary alot between use cases). The situation becomes more annoying if plugin may pass interface pointer back to the engine for some purpose. Since engine does not know if this interface has implementation by engine or plugin, it cannot safely cast it to its internal class. This would mean that engine would have to use the "wrapper" interface with "destroy" function in places where it would instead use original class object.

In any case, it's important to note that whenever we have a place where the object may come both from engine and plugin, we must use the interface pointer and not any specific implementation. In such case also we must dispose such object using "destroy" method, and not delete, as we don't know where it came from.
This may be automated using smart pointers, as they support custom deleters. So we store the interface in std::unique_ptr or std::shared_ptr, with custom deleter that calls "destroy" method for the object instead of deleting it regular way.

Having "destroy" function in factory interface, besides the "create" function. This is easy at first, but if the object interface may be implemented by plugins too, there appear a new issue: you cannot tell which "destroy" function to use, as the object's interface itself has no means to tell this.

It means that we have to somehow pair object's interface with a pointer to its "destroy" function anyway. This may be done by passing them together in any api function that accepts this interface. Either as 2 arguments, or grouped into a struct.

To give an example of such struct:

template <typename TInterface>
struct DisposableInterface
{
    TInterface *Object;
    void (*Dispose)(TInterface *obj);
};

So when any disposable object is passed from plugin to engine, it's done using this struct, for example:

typedef DisposableInterface<IAGSStream> DisposableStream;

IAGSAudioDecoder *OpenAudioDecoder(DisposableStream *stream);

The struct itself is not owned, and should not be deleted by the engine, of course. We simply take values out from it.
Similar to the first solution, the received object ptr may be stored in a unique_ptr or shared_ptr with custom deleter, except that custom deleter would be using this provided Dispose function pointer.