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 audio device selector to transcribe + take a stab at Delete/Retry models #54

Merged
merged 5 commits into from
Mar 8, 2024

Conversation

cgfarmer4
Copy link
Contributor

@cgfarmer4 cgfarmer4 commented Mar 7, 2024

Addresses #13 + adds audio device view in both sections.

  1. New delete model button
Screenshot 2024-03-06 at 9 32 03 PM
  1. Restart download of model if connection lost
Screenshot 2024-03-06 at 9 49 04 PM
  1. Move audio input selection between the buttons on transcribe.
Screenshot 2024-03-06 at 9 49 17 PM

@@ -779,13 +812,35 @@ struct ContentView: View {
try await whisperKit.loadModels()

await MainActor.run {
if !localModels.contains(model) {
Copy link
Contributor Author

@cgfarmer4 cgfarmer4 Mar 7, 2024

Choose a reason for hiding this comment

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

local models doesnt refresh if you redownload a model after deleting without this

@cgfarmer4 cgfarmer4 marked this pull request as ready for review March 7, 2024 05:53
@ZachNagengast ZachNagengast self-requested a review March 7, 2024 16:34
@ZachNagengast
Copy link
Contributor

Nice idea with the delete and retry! This is probably a good time to handle canceling the previous download task in resetState() because it's a static method. As-is, if you are in the middle of a download and tap the restart button, the previous download will continue and a new one will start. It may require something like this:

        let downloadTask = Task {
            folder = try await WhisperKit.download(variant: model, from: repoName, progressCallback: { progress in
                DispatchQueue.main.async {
                    loadingProgressValue = Float(progress.fractionCompleted) * specializationProgressRatio
                    modelState = .downloading
                }

                // Check for cancellation here
                try Task.checkCancellation()
            })
        }

or something similar to how the progressbar task gets cancelled:

                let progressBarTask = Task {
                    await updateProgressBar(targetProgress: 0.9, maxTime: 240)
                }

                // Prewarm models
                do {
                    try await whisperKit.prewarmModels()
                    progressBarTask.cancel()
                } catch {
                    print("Error prewarming models, retrying: \(error.localizedDescription)")
                    progressBarTask.cancel()
                    if !redownload {
                        loadModel(model, redownload: true)
                        return
                    } else {
                        // Redownloading failed, error out
                        modelState = .unloaded
                        return
                    }
                }

What do you think? This would also solve a few edge cases with the model selection, like if you select a new model while the previous one is downloading.

@ZachNagengast
Copy link
Contributor

Also played around with the mic selector placement, what do you think of this setup? Added a PR to your repo: cgfarmer4#1

image image

@cgfarmer4
Copy link
Contributor Author

Beautiful, merged your PR and will take a look at cancelling the download tonight.

@cgfarmer4
Copy link
Contributor Author

cgfarmer4 commented Mar 8, 2024

I removed the retry, getting downloads to cancel is going to require a larger refactor of this function.

let downloadTask = Task {
            folder = try await WhisperKit.download(variant: model, from: repoName, progressCallback: { progress in
                DispatchQueue.main.async {
                    loadingProgressValue = Float(progress.fractionCompleted) * specializationProgressRatio
                    modelState = .downloading
                }

                // Check for cancellation here
                try Task.checkCancellation()
            })
        }

Going for this path, you get an error that Mutation of captured var 'folder' in concurrently-executing code.

Then I tried this which fixes the warning: Mutation of captured var 'folder' in concurrently-executing code; this is an error in Swift 6 but doesnt properly cancel the download.

DispatchQueue.main.async {
    folder = downloadFolder
}

Next approach I tried was assigning the main Task similar to how transcriptionTask is assigned but that requires another refactor around the do/catch statements.

How about I leave in the delete + your update and call it on this one? The download cancellation needs a rethink or Im totally missing something.

@ZachNagengast
Copy link
Contributor

No problem! We can get to that later on, but this is a good addition as-is 👍

@ZachNagengast ZachNagengast merged commit ccdd77d into argmaxinc:main Mar 8, 2024
2 checks passed
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.

Feature request: In example, provide delete and re-download options
2 participants