Add support for music streaming

[ACrazyTown]

Been testing this internally and it's been working well so I thought it'd be time to do a PR.

In short:

• Adds the FLX_STREAM_SOUND define, which is used to check when sound streaming is supported
• AssetsFrontEnd
  • Adds streamSoundUnsafe(), streamSound(), streamSoundAddExt(), canStreamSound()
• FlxSound: adds the FlxSound.loadStreamed() method which uses audio streaming under the hood. Very useful for large audio tracks as it loads chunks of data while it plays which results in small memory usage.
  • Also adds FlxG.sound.loadStreamed() which is a helper for this!

Audio streaming is currently only supported when targeting native and using OGG files, this is due to a limitation with Lime. I've been poking around trying to get streaming to work on HTML5 as well. I have a proof of concept, but it might require changes to OpenFL, so it'll be implemented in a later PR once that's all figured out.

Further considerations:

• Should FlxG.sound.playMusic() use streaming?
• Should we add a helper method akin to FlxG.sound.load() but for streamed sounds?

[Geokureli]

What exactly is "streaming"? I assume it's playing a sound without having to put the entire song in memory, first. How would I test this? Which targets does this work on, and do I need to prevent the asset from being preloaded via the project.xml, or something?

[ACrazyTown]

What exactly is "streaming"? I assume it's playing a sound without having to put the entire song in memory, first.

Yes. By default, sounds need to have their entire bytes loaded before they can be played. This is fine for shorter sounds, like sound effects, but is less optimal for music tracks as it leads to a longer load time and higher memory usage. Streaming loads and unloads chunks of audio data as it plays, so memory usage is much lower.

How would I test this?

It's a bit tricky to determine, as it's a backend change, but there's a couple ways:

By checking the audio buffer properties:

var sound = new FlxSound().loadStreamed("assets/music/somemusic.ogg");
FlxG.sound.list.add(sound);
sound.play();

@:privateAccess
{
    var buffer = sound._channel.__audioSource.buffer;
    trace(buffer.data);            // Should be null when streaming as the bytes are not loaded!
    trace(buffer.__srcVorbisFile); // Should NOT be null when streaming as this is used to stream!
}

By checking the memory usage:

When streaming, the memory usage graph fluctuates as data is constantly being loaded and unloaded.

When NOT streaming, the memory usage graph flat lines because no allocations are happening after the initial load.

On Windows specifically, holding the window's title bar as if you're going to drag it freezes the game. After a short while, streamed audio will cut out because the game can't feed data to the audio engine while it's frozen. This doesn't happen with regular audio.

Which targets does this work on, and do I need to prevent the asset from being preloaded via the project.xml, or something?

Currently it's limited to native targets and OGG/Vorbis sound files. Not sure about preloading. I think that assets on native are loaded on demand by default, so there's no need to do anything in Project.xml

[Geokureli]

I think if we're gonna do this we need to be really clear about when this can be done and what it's actually doing. I'm thinking we wrap these methods in #if FLX_STREAM_MUSIC, log an error if the sound isn't compatible, and make sure the doc explains all of this and the benefits.

[ACrazyTown]

How about just wrapping FlxSound.loadStreamed() in a conditional? The MUSIC asset type and related methods are already established in OpenFL, so not sure why we'd block those off. As for everything else, sounds good.

[ACrazyTown]

Let me know if these changes are good. I've also added a helper similar to FlxG.sound.load() but for streamed sounds

[Geokureli]

I'm actually having some doubts about this now. I think lime's way of differentiating "loading" vs "streaming" with "sound" vs "music" is confusing. I don't think people will see these methods and intuit this difference, and rather than copying lime's wording I say we come up with something that better highlights the differences.

My immediate thoughts:

• People may want to stream sounds and/or load music, or may not expect any differences between the two
• FlxG.assets.getFoo methods are always syncronous, they return the usable asset, where FlxG.assets.loadFoo methods are async, they return a Future<T> to use while it fetches the asset. Streaming is definitely closer to a "get" but perhaps we should add a third option, or an optional arg to get methods.
• It's possible I wanted these methods wrapped in #if only because of this lack of clarity. Another thing to consider is always having them and warning when it's not available, like you did in some cases here. Though, if we can tell devs - at compile time -that streaming is definitely not possible, that's also helpful
• FlxG.assets.canBeStreamed(asset, type) seems helpful to everyone trying to add streaming

[ACrazyTown]

People may want to stream sounds and/or load music, or may not expect any differences between the two

I'd understand loading music, but there's really never a need to stream sounds. Doing so has more overhead and no gains. I do agree that the sound/music naming convention can be confusing though.

FlxG.assets.getFoo methods are always syncronous, they return the usable asset, where FlxG.assets.loadFoo methods are async, they return a Future<T> to use while it fetches the asset. Streaming is definitely closer to a "get" but perhaps we should add a third option, or an optional arg to get methods.

I think this is a bit of a weird case because streaming is really only applicable to audio files. I don't think it would make sense to add a new method/argument to all of the methods if they'll be useless in a lot of cases. It also technically is synchronous. The sound is ready to be used immediately. Sure, it loads data behind the scenes, but to the user it's as if it was immediately fully loaded.

Flixel's asset front end seems to be modeled pretty closely after OpenFL's assets, so I'm not sure how we'd squeeze in a new API that's not present there. Would a FlxG.assets.streamSound() or whatever method bypass OpenFL's Assets.getMusic()?

It's possible I wanted these methods wrapped in #if only because of this lack of clarity. Another thing to consider is always having them and warning when it's not available, like you did in some cases here. Though, if we can tell devs - at compile time -that streaming is definitely not possible, that's also helpful

I left the FlxG.assets methods as is because that's how OpenFL does it as well. If streaming is not possible it will just fall back to getSound(). There's no hard rule that streaming only works in ultra-specific cases, it's just that Lime's audio streaming implementation is not ideal right now. Hopefully this is something that gets addressed soon

FlxG.assets.canBeStreamed(asset, type) seems helpful to everyone trying to add streaming

Same as I mentioned above, I don't understand why we'd pass in a type here since streaming is only applicable to audio.

[Geokureli]

I'd understand loading music, but there's really never a need to stream sounds.

This is correct, for nearly everyone's use case. I just meant, people won't intuitively connect "music" to streaming and "sounds" to fully loading. But to be pedantic, voice acted lines may be non-musical and long enough to warrant streaming

I think this is a bit of a weird case because streaming is really only applicable to audio files. I don't think it would make sense to add a new method/argument to all of the methods if they'll be useless in a lot of cases.

Additionally, adding an optional stream arg to getAssetUnsafe is a breaking change. Notably for people using hot-reloading or a custom asset manager.

FlxG.assets.canBeStreamed(asset, type)

I don't understand why we'd pass in a type here since streaming is only applicable to audio.

I agree, here is my plan:

• Keep canBeStreamed as is
• Remove FlxAssetType.MUSIC, and all uses of it, including FlxG.assets.getMusic and the like
• Make getAssetUnsafe() and getOpenflAssetUnsafe() behave the way it did before (how dev behaves, now)
• Add FlxG.assets.streamSound(soundId, useCache), have it call Assets.getMusic and bypass getAssetUnsafe. Make it dynamic, so it can be customized like everything else in AssetsFrontEnd
• Have a holly jolly Christmas

P.S.: Fwiw, if we add video or gif assets it may be possible to stream them, text and binary streams exist, but for now we don't need to worry about that.

[ACrazyTown]

How about this for a little more consistency with the other methods?

public dynamic function streamSoundUnsafe(id:String):Sound; // calls Assets.getMusic(id)

public function streamSound(id:String, ?logStyle:LogStyle):Sound; // calls streamSoundUnsafe(addSoundExtIf(id)), logs if missing

public function streamSoundAddExt(id:String, ?logStyle:LogStyle):Sound; // calls streamSoundUnsafe(addSoundExt(id)), logs if missing

I've removed the useCache parameter as due to the nature of streamed sounds, caching them can very easily lead to issues (sounds cutting out, etc.). Streamed audio was experimented with in Funkin', but their method cached it which lead to said unexpected issues:

Bugs, like a lot of them. From my testing, using too much streamed audio can sometimes cause the game to remove every audio, making the game silent, which I hope is an incredibly obvious problem for a rhythm game.

canStreamSound() is also private and not used anywhere since I've stripped out the MUSIC asset type. Should that be made public?

[Geokureli]

How about this for a little more consistency with the other methods?

public dynamic function streamSoundUnsafe(id:String):Sound; // calls Assets.getMusic(id)

public function streamSound(id:String, ?logStyle:LogStyle):Sound; // calls streamSoundUnsafe(addSoundExtIf(id)), logs if missing

public function streamSoundAddExt(id:String, ?logStyle:LogStyle):Sound; // calls streamSoundUnsafe(addSoundExt(id)), logs if missing

Excellent, I forgot about those methods

I've removed the useCache parameter as due to the nature of streamed sounds, caching them can very easily lead to issues (sounds cutting out, etc.). Streamed audio was experimented with in Funkin', but their method cached it which lead to said unexpected issues:

Bugs, like a lot of them. From my testing, using too much streamed audio can sometimes cause the game to remove every audio, making the game silent, which I hope is an incredibly obvious problem for a rhythm game.

Maybe we should include the useCache arg, default to false, and add documentation explaining these issues. Maybe putting the spotlight on these issues will lead to a fix for lime. Also if caching works for streaming a single music file, that use case is common enough to justify it.

canStreamSound() is also private and not used anywhere since I've stripped out the MUSIC asset type. Should that be made public?

Yes, please

[ACrazyTown]

Maybe we should include the useCache arg, default to false, and add documentation explaining these issues. Maybe putting the spotlight on these issues will lead to a fix for lime. Also if caching works for streaming a single music file, that use case is common enough to justify it.

I don't think it's a bug, it's just something that you should not do. A VorbisFile is really just a file pointer like you'd get by calling sys.File.read() or whatever. There's no value in caching the actual file pointer as opening it is near instant, and it also doesn't store any data by itself. The loading/unloading of audio buffers is handled by Lime's AudioSource during playback

[Geokureli]

I don't think it's a bug, it's just something that you should not do. A VorbisFile is really just a file pointer like you'd get by calling sys.File.read() or whatever. There's no value in caching the actual file pointer as opening it is near instant, and it also doesn't store any data by itself. The loading/unloading of audio buffers is handled by Lime's AudioSource during playback

Thanks for clarifying, lets scrap the arg, then

[Geokureli]

Some change notes

• I check lime_vorbis in canBeStreamed, in case we ever add streaming in some other way
• streamSound now check canBeStreamed
• Changed and added docs
• Made streamSoundUnsafe call addSoundExtIf just like getSoundUnsafe
• Made streamSoundAddExt call streamSound for simplicity

Let me know if this looks right to you @ACrazyTown. Apparently I can't request you to review your own PR

[ACrazyTown]

It won't let me request changes (I can only comment), but overall this looks good!

[Geokureli]

I'm getting a strange result in a test. will continue this tonight

Edit: The issue was unrelated

[Geokureli]

Thanks!

[ACrazyTown]

Now we can have a holly jolly Christmas :)