-
Notifications
You must be signed in to change notification settings - Fork 86
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 ability to set asset init options #71
Add ability to set asset init options #71
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job 👍 Added a few comments, have a look at them when you got time 🚀.
SwiftAudio/Classes/AudioItem.swift
Outdated
@@ -36,6 +36,11 @@ public protocol InitialTiming { | |||
func getInitialTime() -> TimeInterval | |||
} | |||
|
|||
/// Make your `AudioItem`-subclass conform to this protocol to control enable the ability to set initialization options for the asset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make your AudioItem
-subclass conform to this protocol to control enable the ability to set initialization options for the asset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also link to these docs to make it clear what options are to be provided.
SwiftAudio/Classes/AudioItem.swift
Outdated
@@ -125,3 +130,24 @@ public class DefaultAudioItemInitialTime: DefaultAudioItem, InitialTiming { | |||
} | |||
|
|||
} | |||
|
|||
/// An AudioItem that also conforms to the `AssetOptionsProviding`-protocol | |||
public class DefaultAudioItemAuthorizing: DefaultAudioItem, AssetOptionsProviding { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the naming of the class to DefaultAudioItemAssetOptionsProviding
.
SwiftAudio/Classes/AudioItem.swift
Outdated
@@ -36,6 +36,11 @@ public protocol InitialTiming { | |||
func getInitialTime() -> TimeInterval | |||
} | |||
|
|||
/// Make your `AudioItem`-subclass conform to this protocol to control enable the ability to set initialization options for the asset. | |||
public protocol AssetOptionsProviding { | |||
func getAssetOptions() -> [String : Any] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[String: Any] for consistency.
SwiftAudio/Classes/AudioItem.swift
Outdated
super.init(audioUrl: audioUrl, artist: artist, title: title, albumTitle: albumTitle, sourceType: sourceType, artwork: artwork) | ||
} | ||
|
||
public func getAssetOptions() -> [String : Any] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[String: Any] for consistency.
@@ -19,7 +19,6 @@ public enum PlaybackEndedReason: String { | |||
} | |||
|
|||
class AVPlayerWrapper: AVPlayerWrapperProtocol { | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not meant to remove this newline.
Fixed the issues @jorgenhenrichsen :) obviously I made this PR a bit too fast. Should all be good now! |
Closes #68