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 plugin #63

Merged
merged 8 commits into from
Feb 25, 2024
Merged

Audio plugin #63

merged 8 commits into from
Feb 25, 2024

Conversation

MiranDMC
Copy link
Collaborator

@MiranDMC MiranDMC commented Feb 23, 2024

New Audio plugin

  • audio related opcodes moved from CLEO core into separated plugin
  • CLEO's audio now obey game's volume settings
  • implemented support for Doppler Effects (fast moving sound sources)
  • implemented support for game speed changes
  • new opcode is_audio_stream_playing
  • new opcode get_audiostream_length_fine
  • new opcode get_audio_stream_speed
  • new opcode set_audio_stream_speed
  • new opcode set_audio_stream_volume_with_transition
  • new opcode set_audio_stream_speed_with_transition
  • new opcode set_audio_stream_source_size
  • new opcode set_audio_stream_progress
  • new opcode get_audio_stream_progress

Unit test scripts added to release pack

@MiranDMC MiranDMC marked this pull request as ready for review February 24, 2024 18:13
@MiranDMC MiranDMC requested a review from x87 February 24, 2024 18:13
auto stream = (CAudioStream*)OPCODE_READ_PARAM_PTR(); VALIDATE_STREAM();

auto length = stream->GetLength();
length /= max(stream->GetSpeed(), 0.000001f); // speed corrected
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this to avoid division by 0?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there should be a way to get unscaled stream length. If you watch a video on 2x speed the player does not change the total length. Maybe this opcode should return length as-is, and new 2501 get_audiostream_length_scaled would return it scaled to the speed?

Copy link
Collaborator Author

@MiranDMC MiranDMC Feb 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To get unscaled length just do not change the default speed (1.0). It is only affected by user set speed, not by game speed multiplier.
General idea was to keep it consistent with already existing behaviour as previously there was no way to change speed. It seems to make sense to ask "how long will this track play". Then use it as wait opcode argument.

Updated to handle 0.0 case separately.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to make sense to ask "how long will this track play".

this what new 2501 opcode is for. I don't see why we need two opcodes doing same

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original opcode was returning length as seconds in form of floored integer.
Updated to keep the old opcode behaviour unchanged.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Last thing is the 2501 name. "Fine" is ambiguous word and can mean different things. It's probably unclear what this command does without reading the description.

Let's call it GET_AUDIO_STREAM_LENGTH_SCALED_TO_SPEED. A little verbose, but has clear meaning.

Alternatives: GET_AUDIO_STREAM_LENGTH_SCALED
GET_AUDIO_STREAM_DURATION

auto stream = (CAudioStream*)OPCODE_READ_PARAM_PTR(); VALIDATE_STREAM();
auto speed = OPCODE_READ_PARAM_FLOAT();

stream->SetSpeed(speed);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably should disallow speed of 0. one should pause stream instead

Copy link
Collaborator Author

@MiranDMC MiranDMC Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intended. There is opcode allowing to perform smooth speed transition. So transitioning to 0.0 is useful to stop the audio with "tape scratch" effect.
Yes, speed 0 changes stream's state to paused, but still the stream speed property should be 0 after that operation. State and speed are two separated properties.


void CAudioStream::SetVolume(float val, float transitionTime)
{
if (val < 0.0f) // just update
Copy link

@x87 x87 Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets make a separate method UpdateVolume() instead of calling SetVolume(-1.0f). it makes the code easier to understand

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if (state != playing && !force) return; // done

SetSpeed(-1.0f);
SetVolume(-1.0f);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UpdateSpeed();
UpdateVolume();

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


void C3DAudioStream::Set3dSize(float radius)
{
BASS_ChannelSet3DAttributes(streamInternal, BASS_3DMODE_NORMAL, radius, 1E+12f, -1, -1, -1.0f);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1E+12f ?? 5000.0 covers the entire map

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BASS internally uses some weird decay curve calculations. I don't know what they used for this parameter as default but it seems to be insufficient. This parameter only stops volume decay after that distance is reached.

}

//2506=2,set_audio_stream_source_size %1d% radius %2d%
static OpcodeResult __stdcall opcode_2506(CScriptThread* thread)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is a property of 3D stream the opcode should be

set_3d_audio_stream_source_size

and belong to class AudioStream3D

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also consider alternative name: set_3d_audio_stream_distance

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used to set 3d sound source sphere radius. "distance" would suggest this is related to max hearing distance.

@x87
Copy link

x87 commented Feb 24, 2024

the rest looks good to me, thanks

@MiranDMC MiranDMC requested a review from x87 February 25, 2024 03:27
@MiranDMC MiranDMC merged commit 9228040 into master Feb 25, 2024
@MiranDMC MiranDMC deleted the audio_plugin branch February 27, 2024 01:54
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