Skip to content

Can't extend AudioStreamGeneratorPlayback #1129

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

Open
djcsdy opened this issue Apr 20, 2025 · 5 comments
Open

Can't extend AudioStreamGeneratorPlayback #1129

djcsdy opened this issue Apr 20, 2025 · 5 comments
Labels
bug c: engine Godot classes (nodes, resources, ...)
Milestone

Comments

@djcsdy
Copy link

djcsdy commented Apr 20, 2025

godot-rust version: 0.2.4

Consider this minimal class extending AudioStreamGeneratorPlayback:

use godot::classes::native::AudioFrame;
use godot::classes::IAudioStreamGeneratorPlayback;
use godot::prelude::*;

#[derive(GodotClass)]
#[class(init, base=AudioStreamGeneratorPlayback)]
struct Foo {}

#[godot_api]
impl IAudioStreamGeneratorPlayback for Foo {
    unsafe fn mix_resampled(&mut self, _dst_buffer: *mut AudioFrame, frame_count: i32) -> i32 {
        frame_count
    }

    fn get_stream_sampling_rate(&self) -> f32 {
        8000.0
    }

    unsafe fn mix(&mut self, _buffer: *mut AudioFrame, _rate_scale: f32, frames: i32) -> i32 {
        frames
    }
}

When this code is loaded into Godot, Godot reports:

  ERROR: Extension class 'Foo' cannot extend native abstract class 'AudioStreamGeneratorPlayback'.
  ERROR: C:\Users\djc\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\godot-core-0.2.4\src\registry\class.rs:590 - Failed to register class `Foo`; check preceding Godot stderr messages.

My understanding is that it's not supposed to be possible to extend this class, but I may be mistaken. In this case, the above code should be a compile error in godot-rust.

Even if it is supposed to be possible to extend AudioStreamGeneratorPlayback, IAudioStreamGeneratorPlayback appears to be incorrect. To implement this trait the programmer must implement the mix_resampled and mix functions, but both of these have default implementations in Godot.

It's especially illogical to require the programmer to implement mix because the whole point of extending AudioStreamGeneratorPlayback (as opposed to AudioStreamPlayback) is that mix calls mix_resampled and resamples the audio for you.

I'm compiling this code with the experimental-threads feature enabled because if this code were working, mix would be called by Godot from a non-main thread. However as a matter of practice Godot never gets as far as calling this function, so experimental-threads is not strictly required to reproduce the above error.

I would like to use AudioStreamGeneratorPlayback in my project but as a workaround I am implementing AudioStreamPlayback instead and providing my own implementation of resampling.

@Bromeon Bromeon added bug c: engine Godot classes (nodes, resources, ...) labels Apr 20, 2025
@Bromeon
Copy link
Member

Bromeon commented Apr 20, 2025

Good catch, thanks for reporting!

Maybe a bit of background, we generate the entire Godot API (everything in godot::classes) from an API spec file extension_api.json. That file contains the declarations of all classes, methods, parameters, and so on.

To my knowledge, there is no property that allows us to reliably identify whether a class is inheritable (there is an is_abstract field for this purpose, but it's not set for AudioStreamGeneratorPlayback). So we would need to manually hardcode classes like this.

Are you aware of other such classes?

@djcsdy
Copy link
Author

djcsdy commented Apr 20, 2025

I am aware that the API is generated and I'm aware that I'm venturing into the more rarely used areas of the API surface, so I'm not too surprised to be encountering issues like this. Totally understood :-).

I'm not aware of any other classes that have this problem but I'll report them if I find them. Thanks!

@djcsdy
Copy link
Author

djcsdy commented Apr 20, 2025

I would like to use AudioStreamGeneratorPlayback in my project but as a workaround I am implementing AudioStreamPlayback instead and providing my own implementation of resampling.

For the benefit of anyone reading this, I should have been extending AudioStreamPlaybackResampled, which has its own problems (#1133) but does work as expected with minor workarounds.

The rest of the bug report is correct though :-).

@Bromeon Bromeon added this to the 0.3 milestone Apr 20, 2025
@djcsdy
Copy link
Author

djcsdy commented Apr 29, 2025

The following classes also have this problem:

  • AudioStreamPlaybackInteractive
  • AudioStreamPlaybackPlaylist
  • AudioStreamPlaybackPolyphonic
  • AudioStreamGeneratorPlayback
  • AudioStreamPlaybackSynchronized

It seems Godot doesn't allow extending any of these classes. In all cases attempting to do so gives an error similar to:

ERROR: Extension class 'Investigate1136Playback' cannot extend native abstract class 'AudioStreamPlaybackSynchronized'.

@djcsdy
Copy link
Author

djcsdy commented Apr 29, 2025

Note that this means it is illegal to extend any AudioStreamPlayback class except for AudioStreamPlayback itself, AudioStreamPlaybackResampled and, oddly, AudioStreamPlaybackOggVorbis.

I think it might be a bug in Godot that it is legal to extend AudioStreamPlaybackOggVorbis, because my brief investigation suggests that it is not possible to do so safely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: engine Godot classes (nodes, resources, ...)
Projects
None yet
Development

No branches or pull requests

2 participants