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

Add higher-level finishCallback closure #46

Merged
merged 2 commits into from
Apr 7, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 41 additions & 4 deletions Sources/PlaydateKit/Core/Sound.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
public import CPlaydate

Check warning on line 1 in Sources/PlaydateKit/Core/Sound.swift

View workflow job for this annotation

GitHub Actions / Build

public import of 'CPlaydate' was not used in public declarations or inlinable code

/// Functions related to audio playback.
///
Expand All @@ -6,22 +6,28 @@
/// longer files (uncompressed, MP3, and ADPCM formats), and a synthesis library for generating "computer-y" sounds.
/// Sound sources are grouped into channels, which can be panned separately, and various effects may be applied to the channels.
/// Additionally, signals can automate various parameters of the sound objects..
public enum Sound {

Check warning on line 9 in Sources/PlaydateKit/Core/Sound.swift

View workflow job for this annotation

GitHub Actions / Lint

Remove leading blank line at the start of a scope. (blankLinesAtStartOfScope)
// MARK: Public

Check warning on line 10 in Sources/PlaydateKit/Core/Sound.swift

View workflow job for this annotation

GitHub Actions / Lint

Organize declarations within class, struct, enum, actor, and extension bodies. (organizeDeclarations)

Check warning on line 11 in Sources/PlaydateKit/Core/Sound.swift

View workflow job for this annotation

GitHub Actions / Lint

Organize declarations within class, struct, enum, actor, and extension bodies. (organizeDeclarations)
/// The fileplayer class is used for streaming audio from a file on disk. This requires less memory than keeping all of the
/// file’s data in memory (as with the sampleplayer), but can increase overhead at run time.
public class FilePlayer {

Check warning on line 14 in Sources/PlaydateKit/Core/Sound.swift

View workflow job for this annotation

GitHub Actions / Lint

Remove leading blank line at the start of a scope. (blankLinesAtStartOfScope)

Check warning on line 14 in Sources/PlaydateKit/Core/Sound.swift

View workflow job for this annotation

GitHub Actions / Lint

Organize declarations within class, struct, enum, actor, and extension bodies. (organizeDeclarations)
// MARK: Lifecycle

Check warning on line 15 in Sources/PlaydateKit/Core/Sound.swift

View workflow job for this annotation

GitHub Actions / Lint

Organize declarations within class, struct, enum, actor, and extension bodies. (organizeDeclarations)

Check warning on line 16 in Sources/PlaydateKit/Core/Sound.swift

View workflow job for this annotation

GitHub Actions / Lint

Organize declarations within class, struct, enum, actor, and extension bodies. (organizeDeclarations)
/// Creates a new FilePlayer.
public init() {
pointer = fileplayer.newPlayer.unsafelyUnwrapped().unsafelyUnwrapped
}

deinit { fileplayer.freePlayer.unsafelyUnwrapped(pointer) }
deinit {
fileplayer.freePlayer.unsafelyUnwrapped(pointer)
if let callbackData = _callbackData {
callbackData.deinitialize(count: 1)
callbackData.deallocate()
}
}

// MARK: Public

Check warning on line 30 in Sources/PlaydateKit/Core/Sound.swift

View workflow job for this annotation

GitHub Actions / Lint

Organize declarations within class, struct, enum, actor, and extension bodies. (organizeDeclarations)

/// Returns true if player is playing
public var isPlaying: Bool {
Expand All @@ -30,12 +36,12 @@

/// Prepares player to stream the file at path. Returns `true` if the file exists, otherwise `false`.
@discardableResult public func load(path: StaticString) -> Bool {
fileplayer.loadIntoPlayer(pointer, path.utf8Start) == 0
fileplayer.loadIntoPlayer(pointer, path.utf8Start) == 1
}

/// Prepares player to stream the file at path. Returns `true` if the file exists, otherwise `false`.
@discardableResult public func load(path: UnsafePointer<CChar>) -> Bool {
fileplayer.loadIntoPlayer(pointer, path) == 0
fileplayer.loadIntoPlayer(pointer, path) == 1
}
Comment on lines 38 to 45
Copy link
Owner

Choose a reason for hiding this comment

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

I think it would be nice to eventually make these throw an error if the file doesn't exist instead of returning false, but this is a good fix for now


/// Starts playing the file player. If repeat is greater than one, it loops the given number of times.
Expand All @@ -43,10 +49,41 @@
/// Returns 1 on success, 0 if buffer allocation failed.
@discardableResult public func play(repeat: Int32 = 1) -> Int32 {
fileplayer.play.unsafelyUnwrapped(pointer, `repeat`)
}

Check warning on line 52 in Sources/PlaydateKit/Core/Sound.swift

View workflow job for this annotation

GitHub Actions / Lint

Insert blank line before class, struct, enum, extension, protocol or function declarations. (blankLinesBetweenScopes)

Check warning on line 52 in Sources/PlaydateKit/Core/Sound.swift

View workflow job for this annotation

GitHub Actions / Lint

Organize declarations within class, struct, enum, actor, and extension bodies. (organizeDeclarations)

// `CallbackData` wraps the callback closure and is stored in a dynamically allocated block, which
// is passed as `userData` to `setFinishCallback` below.
private struct CallbackData {
var callback: (() -> Void)?
}
private var _callbackData: UnsafeMutablePointer<CallbackData>?

/// Installs a closure that will be called when playback has completed.
public var finishCallback: (() -> Void)? {
get {
_callbackData?.pointee.callback
}
set {
if let callbackData = _callbackData {
callbackData.pointee.callback = newValue
} else if let newValue {
let callbackData: UnsafeMutablePointer<CallbackData> = .allocate(capacity: 1)
callbackData.initialize(to: .init(callback: newValue))
_callbackData = callbackData
setFinishCallback(
callback: { sourceSource, userdata in
if let callback = userdata?.assumingMemoryBound(to: CallbackData.self).pointee.callback {
callback()
}
},
soundUserdata: callbackData
)
}
}
}
Comment on lines +62 to +83
Copy link
Owner

Choose a reason for hiding this comment

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

This seems like a nice approach for this. Alternatives would be to mark FilePlayer as open, set the userdata to a pointer to the file player, and have it call an open didFinish() method that people can override (similar to what I'm doing in #32). Another option might be to allow setting a delegate on the FilePlayer, which would align with iOS APIs.

Don't really have much of a preference here, just thinking out loud.

Copy link
Contributor Author

@pcbeard pcbeard Apr 7, 2024

Choose a reason for hiding this comment

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

Another approach might pass the FilePlayer as a parameter to the closure. This may be preferable, to avoid creating cycles, since weak is unavailable in embedded Swift (which is a shame). The only tricky bit is that the FilePlayer pointer would have to be stored as well in the callback, as you mentioned above. And sub-classing seems less flexible to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One other tiny change might be to reorder the parameters of setFinishCallback() so a trailing closure can be used.


/// Sets a function to be called when playback has completed.
public func setFinishCallback(
func setFinishCallback(
callback: @convention(c) (
_ soundSource: OpaquePointer?,
_ userdata: UnsafeMutableRawPointer?
Expand Down
Loading