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

Provide memory buffer -> mixer Chunk conversion methods #970

Merged
merged 2 commits into from
Apr 2, 2020

Conversation

jstasiak
Copy link
Contributor

@jstasiak jstasiak commented Feb 6, 2020

This basically provides an API to access Mix_QuickLoad_WAV and
Mix_QuickLoad_RAW with one caveat – thie API takes ownership of the
buffers, while Mix_QuickLoad_* functions don't. Since Rust-SDL2 wraps
Mix_Chunks in its own structure with its own buffer ownership flag it's
easy to work around that and deallocate the buffer when Chunk is
dropped.

There are few design considerations here:

  • Taking ownership of the buffer. I couldn't come up with a way to
    get a Chunk based on a buffer while still keeping the code safe and
    making sure that as long as Chunk is alive the buffer is also alive
    and not making the change be terribly involved and disruptive
  • What's the best way to take an ownership of a buffer passed as a
    parameter? I came up with Box<[T]> as it seems universal enough,
    but I expect a more idiomatic Rust way may exist.

I modified the mixer demo to show how the API is used, a beep is
generated when no sound file is passed to the example.

@jstasiak jstasiak force-pushed the chunks-from-buffers branch from 6a2ec5b to 252b468 Compare February 6, 2020 23:03
This basically provides an API to access Mix_QuickLoad_WAV and
Mix_QuickLoad_RAW with one caveat – thie API takes ownership of the
buffers, while Mix_QuickLoad_* functions don't. Since Rust-SDL2 wraps
Mix_Chunks in its own structure with its own buffer ownership flag it's
easy to work around that and deallocate the buffer when Chunk is
dropped.

There are few design considerations here:

* Taking ownership of the buffer. I couldn't come up with a way to
  get a Chunk based on a buffer while still keeping the code safe and
  making sure that as long as Chunk is alive the buffer is also alive
  *and* not making the change be terribly involved and disruptive
* What's the best way to take an ownership of a buffer passed as a
  parameter? I came up with Box<[T]> as it seems universal enough,
  but I expect a more idiomatic Rust way may exist.

I modified the mixer demo to show how the API is used, a beep is
generated when no sound file is passed to the example.
@jstasiak jstasiak force-pushed the chunks-from-buffers branch from 252b468 to 9dde795 Compare February 6, 2020 23:39
@jstasiak
Copy link
Contributor Author

jstasiak commented Feb 7, 2020

Actually, I'm not sure if integrating Mix_QuickLoad_WAV is feasible – there's no way to pass buffer length to it and it performs blind buffer traversal (and reads buffer length from the buffer itself) on the assumption that the contents are correct. If they're not, we'll get a segmentation fault or even possibly something else. I'm considering dropping it, as loading from raw buffer seems much more useful to me anyway.

Mix_QuickLoad_WAV performs buffer traversal assuming we passed it enough
data. This is not necessarily true and protecting against it would
duplicate some of the Mix_QuickLoad_WAV logic here. As this is less
useful than from_raw_buffer() let's drop it for now.
@jstasiak
Copy link
Contributor Author

jstasiak commented Mar 5, 2020

I removed the Chunk::from_wav_buffer() method from this pull request following the earlier comment, it can be introduced at a later point if necessary/feasible.

@jstasiak
Copy link
Contributor Author

jstasiak commented Apr 2, 2020

Is there something I can do to get this merged? If this is inappropriate to be included in Rust-SDL2 just let me know.

@Cobrand
Copy link
Member

Cobrand commented Apr 2, 2020

Sorry I totally forgot about it, LGTM so I'll merge this.

@Cobrand Cobrand merged commit f9e6c2a into Rust-SDL2:master Apr 2, 2020
@jstasiak jstasiak deleted the chunks-from-buffers branch April 3, 2020 11:09
@jstasiak
Copy link
Contributor Author

jstasiak commented Apr 3, 2020

Cheers!

sypwex pushed a commit to sypwex/rust-sdl2 that referenced this pull request Jun 2, 2024
Provide memory buffer -> mixer Chunk conversion methods
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