Bidirectional Streaming iOS Example #776
Conversation
glbrntt
left a comment
glbrntt
left a comment
There was a problem hiding this comment.
Thanks for doing this Jake, this is really cool! I've left some notes in line.
Jake-Prickett
force-pushed
the
cocoapods-bidirectional-streaming-example
branch
from
7de8c57 to
aba7190
Compare
Jake-Prickett
force-pushed
the
cocoapods-bidirectional-streaming-example
branch
from
aba7190 to
8c44608
Compare
MrMage
left a comment
MrMage
left a comment
There was a problem hiding this comment.
Great example! I have just added a few nits that reflect my personal style preference; feel free to ignore them. I must also admit that I have not looked at George's comments...
If you have an extra 5-10 minutes to spare, I would love to see a video of this app in action without having to compile it myself. For example, just connect your device to your Mac, and record the device screen with QuickTime player while using the app. (Or just do an on-device screen recording, that would probably be even easier.) Again, completely optional!
| } | ||
|
|
||
| class AudioStreamManager { | ||
|
|
There was a problem hiding this comment.
my preference is not to have a newline at the beginning of a class, but up to you (same goes for the method implementations below)
| static var shared = AudioStreamManager() | ||
|
|
||
| private let bus1: AudioUnitElement = 1 | ||
| private var oneFlag: UInt32 = 1 |
There was a problem hiding this comment.
could that variable be local to the method where it is used? If not, document this variable and maybe the other ones as well? (Same goes for bus1 above.)
|
|
||
| private func configureAudioSession() { | ||
| let session = AVAudioSession.sharedInstance() | ||
| try? session.setCategory(.record) |
There was a problem hiding this comment.
should we make this method throw instead?
| // Send audio data | ||
| call.sendMessage(streamAudioDataRequest, promise: nil) | ||
|
|
||
| case .streaming(let stream): |
There was a problem hiding this comment.
this is called call above; unify the two variable names?
|
Sorry @Jake-Prickett; I missed the notification with your updates from ~a week a go. I'm happy with this, just wanted to check that @MrMage is happy with this too before merging? |
|
👌 from me |
|
No worries! Thanks for the merge! |
4 participants
Recently wrote a blog post about gRPC Swift using SwiftNIO, created this example app to help highlight some capabilities.
After publishing it, noticed this recently closed PR: #516, which implemented the same example app with a suggestion to use the NIO Implementation. Implemented a few other suggestions as well from the PR.
Partial Credit: @timburks