Skip to content

Conversation

@timburks
Copy link
Member

No description provided.

@timburks timburks mentioned this pull request Jul 21, 2019
4 tasks
@MrMage
Copy link
Collaborator

MrMage commented Jul 22, 2019

This is example is for the non-NIO implementation; do we really want to add new samples for that? (Also, using bidi APIs should be much more straightforward with the NIO version.)

@timburks
Copy link
Member Author

@MrMage I got some questions about how to make streaming calls with the original APIs. I plan to add a NIO version when we have Cocoapods support for that.

@MrMage MrMage closed this Jul 23, 2019
@MrMage MrMage reopened this Jul 23, 2019
Copy link
Collaborator

@MrMage MrMage left a comment

Choose a reason for hiding this comment

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

A few stylistic suggestions. If it were framework code, I'd probably request some more edits, but as an example it is sufficient I guess.

static var sharedInstance = AudioController()

deinit {
AudioComponentInstanceDispose(remoteIOUnit!);
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove the semicolons (also below)

}

func prepare(specifiedSampleRate: Int) -> OSStatus {

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: extra newline

return status
}

let bus1 : AudioUnitElement = 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: please remove the space before the colon (also below)

}

func recordingCallback(
inRefCon:UnsafeMutableRawPointer,
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: please add spaces after the colons

inBusNumber:UInt32,
inNumberFrames:UInt32,
ioData:UnsafeMutablePointer<AudioBufferList>?) -> OSStatus {

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: extra newlines


static let sharedInstance = SpeechRecognitionService()

func streamAudioData(_ audioData: NSData, completion: @escaping SpeechRecognitionCompletionHandler) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use Data instead of NSData?

print("update send error > \(error)")
}
}
// self.signal(UIColor.green);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this?

self.button.setTitle("LISTENING (tap to stop)", for: .normal)
let audioSession = AVAudioSession.sharedInstance()
do {
try audioSession.setCategory(AVAudioSession.Category.record)
Copy link
Collaborator

Choose a reason for hiding this comment

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

try! instead?

@IBOutlet weak var textView: UITextView!
@IBOutlet weak var button: UIButton!

var audioData: NSMutableData!
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use Data?


if (audioData.length > chunkSize) {
SpeechRecognitionService.sharedInstance.streamAudioData(audioData,
completion:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove completion: and just pass the closure as a trailing closure instead?

Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Really cool example Tim!


## Quickstart
- Clone this repo and `cd` into this directory.
- Run `./COMPILE-PROTOS.sh` to generate the Protocol Buffer and gRPC support files. Note that this requires protoc and the protoc-gen-swift and protoc-gen-swiftgrpc plugins. You can get the plugins by running `swift build` in the root of the grpc-swift repository.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: add protoc to the prerequisites

Copy link
Collaborator

@glbrntt glbrntt Jul 25, 2019

Choose a reason for hiding this comment

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

Do the plugins need to be statically linked? i.e. make plugins instead of swift build?


var window: UIWindow?

func application
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: formatting looks off here

func application(
  _ application: UIApplication,
  didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]? = nil
) -> Bool {
  return true
}

import Foundation
import SwiftGRPC

let API_KEY = "YOUR_API_KEY"
Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW: you can add a compiler warning (/error) here so that it's easier to find: #warning("You need to set your API Key")

@glbrntt
Copy link
Collaborator

glbrntt commented Jul 25, 2019

@MrMage I got some questions about how to make streaming calls with the original APIs. I plan to add a NIO version when we have Cocoapods support for that.

Do we need to do the NIO example with Cocoapods? The Xcode 11 support for SwiftPM would make this easier. Also I'd be happy to put that example together.

@MrMage
Copy link
Collaborator

MrMage commented Jul 26, 2019

Do we need to do the NIO example with Cocoapods? The Xcode 11 support for SwiftPM would make this easier. Also I'd be happy to put that example together.

I guess we could be fine without it :-)

@timburks
Copy link
Member Author

@glbrntt We don't need this to use Cocoapods - I think it would be fine/great to replace all Cocoapods examples with SwiftPM/Xcode 11 ones.

return
}
self.button.setTitle("LISTENING (tap to stop)", for: .normal)
let audioSession = AVAudioSession.sharedInstance()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these two lines necessary, they're also done in AudioController.prepare?

}

class AudioController {
var remoteIOUnit: AudioComponentInstance? // optional to allow it to be an inout argument
Copy link
Collaborator

Choose a reason for hiding this comment

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

As far as I understand, this is the microphone, right? microphoneIOUnit would be clearer if so.

AudioComponentInstanceDispose(remoteIOUnit!)
}

func prepare(specifiedSampleRate: Int) -> OSStatus {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this is an example and the following isn't really the subject of the example but it would be useful to provide more of a description as to what each part of this is doing. Breaking it up into a couple of functions would also help readability!

// Set format for mic input (bus 1) on RemoteIO's output scope
var asbd = AudioStreamBasicDescription()
asbd.mSampleRate = sampleRate
asbd.mFormatID = kAudioFormatLinearPCM
Copy link
Collaborator

Choose a reason for hiding this comment

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

Worth adding a comment saying that the initial request config is linked to this config.

@MrMage MrMage changed the base branch from master to cgrpc March 19, 2020 13:05
@MrMage
Copy link
Collaborator

MrMage commented Apr 23, 2020

Closing this due to inactivity for now; please let us know if you would like to pick it up again.

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.

3 participants