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

[Audio] Adds ICustomBufferAudioSource to implement custom audio sources (rebase) #2162

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

markdchurchill
Copy link

PR Details

Adds entry point for custom audio sources

This is a straight rebase of the work of @tebjan on an older PR #1313. I've reworked this because I just cannot figure out how to provide custom sound data deriving DynamicSoundSource - without use of internals (the exposed constructors seem a bit chicken/egg). (Am I missing something here?)

Description

The basic idea is to use a similar approach as the streaming sources that read files from the disc in blocks but have an endless callback for audio blocks. For this, the user can implement the interface ICustomBufferAudioSource and pass it to the constructor of StreamedBufferSound. This StreamedBuferSound instance will then behave like any Sound instance in Stride.

See 21-StrideRuntime.Tests\Stride.Audio.Tests.Windows\TestCustomBufferSoundSource.cs

Motivation and Context

Reupped this for discussion. My use case is roughly for various generated audio from tts and voice comms to be attached to audio emitters for positioning.

Getting this to build correctly has been an adventure while trying to keep my dev environment for other projects stable. I haven't tested this in the editor - particularly while using AudioEmitterComponent.AttachSound which was the original use case I couldn't get to work with DynamicSoundSource.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

tebjan and others added 4 commits February 20, 2024 17:05
@markdchurchill
Copy link
Author

@dotnet-policy-service agree

@tebjan
Copy link
Member

tebjan commented Feb 20, 2024

Looks very good at first sight, thanks for adding a test. Will test it soon and maybe add some thoughts.

@markdchurchill
Copy link
Author

Yeah I need to actually use this to get a complete opinion. On initial impression there's "too much code" around creating the separate ICustomSoundSource interface, and this should be solvable inheriting DynamicSoundSource (which again - really looks like a user should be able to do!). On the other hand in my adventures fighting all the internal coupling between these audio types this makes me really appreciate being able to do a clean and simple implementation that writes straight to an internal buffer.

If you do get the editor up, I'd appreciate a check that this plays nice with the AudioEmitterComponent :)

@tebjan
Copy link
Member

tebjan commented Feb 20, 2024

On the user side, it should just be this:

class UserAudioSource: CustomAudioSourceBase
    {
        // Callback from the audio engine
        public override bool ComputeAudioData(AudioData bufferToFill, out bool endOfStream)
        {
            // Create audio data
            WriteUserAudioDataTo(bufferToFill.Data);

            bufferToFill.CountDataBytes += BlockSizeInBytes;
            endOfStream = false;
            return true; // success
        }

...user code...

}

Which are not many lines to write. Or do you mean the amount of lines in this PR?

@markdchurchill
Copy link
Author

Yeah I meant the PR. User side for this is good either way.

I guess my expectation (before I saw this PR) was something like "inherit DynamicDataSource, override a method to fill a buffer.

The docs do suggest to inherit DDS - but from my playing around with it, it seems impossible. The public constructor requires an underlying sound instance, its seems to make this practically work would need a lot of the internal class coupling to be unpicked - which not being familiar with the code, and only having the occasional half hour block outside or work or waiting on builds is a bit beyond my capability.

@Eideren Eideren requested a review from tebjan April 21, 2024 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants