MM-56540 - Live-captions support #18

cpoile · 2024-01-25T23:36:18Z

Summary

At the moment this is pretty WIP, making a PR to get feedback on the general approach.
I am keeping the debug statements (starting with <><>) which give a lot of useful insight into what's happening in the transcriber.
I've left Todos to show what I'm still planning to do, and liberal comments to help explain why things are the way they are.
I'm open to changing anything and everything, so please suggest anything that comes to mind.

Ticket Link

https://mattermost.atlassian.net/browse/MM-56540

cpoile · 2024-01-25T23:37:38Z

~~Looks like the lint is failing on the C types -- I'm wondering if there's a way to make this work on both linux and darwin?~~

~~Edit: working on the fix, now lints, I'll get it green soon.~~

Fixed.

streamer45

Looks pretty good 💪 . Gave it a very quick pass to potentially unblock further improvements. Still need to go through the meat (processLiveCaptionsForTrack).

My only "blocking" thought would be the need for a config setting to control whether we do live captions. This way we can have a related setting on the plugin side to globally enable/disable this functionality.

cmd/transcriber/call/transcriber.go

cmd/transcriber/call/live_captions.go

cmd/transcriber/call/tracks.go

cmd/transcriber/call/live_captions.go

… logs

jupenur · 2024-03-14T13:18:05Z

@cpoile there was nothing particularly risky here. I can do another quick pass some time next week -- it shouldn't take me more than a few hours tops. Just tag me again once all the changes are in.

cpoile · 2024-03-19T18:32:40Z

Changes are in, @jupenur. Thank you for taking the time.

jupenur

Still looking pretty solid from my PoV. Thanks @cpoile!

streamer45

Still a lot of complexity in here but if it works it works I suppose :)

Left some candid feedback, nothing truly blocking though.

cmd/transcriber/call/transcriber.go

streamer45 · 2024-03-21T16:35:31Z

cmd/transcriber/call/transcriber.go

+			slog.Int("LiveCaptionsNumThreadsPerTranscriber", t.cfg.LiveCaptionsNumThreadsPerTranscriber))
+		go t.startTranscriberPool()
+	} else {
+		slog.Info("LiveCaptionsOn is false; not starting the transcriber pool")


Is this really needed?

I guess not totally needed -- was using it during development. Will remove.

Yeah, we can infer if they are disabled. It's also a less interesting case.

cmd/transcriber/config/config.go

cmd/transcriber/call/live_captions.go

cmd/transcriber/call/tracks.go

cmd/transcriber/call/live_captions.go

streamer45

I promise these are the last comments :)

cmd/transcriber/call/transcriber.go

go.mod

streamer45 · 2024-03-27T16:21:56Z

cmd/transcriber/call/transcriber.go

+			slog.Int("LiveCaptionsNumThreadsPerTranscriber", t.cfg.LiveCaptionsNumThreadsPerTranscriber))
+		go t.startTranscriberPool()
+	} else {
+		slog.Info("LiveCaptionsOn is false; not starting the transcriber pool")


Yeah, we can infer if they are disabled. It's also a less interesting case.

cmd/transcriber/call/live_captions.go

initial live-captions support; wip

3d76133

cpoile added 2: Dev Review Requires review by a core committer Do Not Merge Should not be merged until this label is removed labels Jan 25, 2024

cpoile requested a review from streamer45 January 25, 2024 23:36

cpoile added 3 commits January 26, 2024 09:11

upgrade silero-vad-go

9ea5bbf

linting

c668947

tests

82a5f37

streamer45 reviewed Jan 26, 2024

View reviewed changes

cpoile added 9 commits January 30, 2024 17:01

update rtcd

72a2fa1

add config options for LiveCaptions, esp. LiveCaptionsOn; tests

dd28a95

fix test

1d59d9f

fix tests

102936d

close captionDoneCh with other doneCh closers

c3d911a

make transcription loop more async

8d8ba6a

remove unused field

985c9f8

send NewAudioLenMs (a measure of load) in the caption ws event

783d5e8

be explicit that NewAudioLenMs is converted to float64 by marshaling

17ed4f5

cpoile requested a review from jupenur February 6, 2024 20:13

cpoile added the 3: Security Review label Feb 6, 2024

cpoile added 9 commits February 7, 2024 11:45

improvements: don't cut off old voice before it's transcribed; better…

b866ba9

… logs

param tuning; better defaults: 2 transcribers x 2 threads

08da042

fix tests

3f96d0a

return blank transcription if transcription error

5db394f

tweak debug statement

7dd28f2

add pressure valve to prevent death spiral on overloaded machines

df3cd10

tweak pressure valve

00ea909

move structs to calls public; send release valve metrics over

9f697d1

implement backpressure for the transcriber pool

de1da40

streamer45 self-requested a review February 9, 2024 15:30

cpoile added 4 commits March 14, 2024 14:26

update rtcd for moving type to JobStateClient

ae229f9

complicated algorithm but with clearer code; using normal vad

8d199b1

improve quality of transcription with background noise

7f63e74

lint

d855144

jupenur previously approved these changes Mar 19, 2024

View reviewed changes

streamer45 added this to the v0.2.0 milestone Mar 19, 2024

streamer45 reviewed Mar 21, 2024

View reviewed changes

cmd/transcriber/call/live_captions.go Outdated Show resolved Hide resolved

tweaking vad and minSpeechLength settings; PR comments

bd6e7dd

cpoile dismissed jupenur’s stale review via bd6e7dd March 22, 2024 20:14

cpoile added 4 commits March 22, 2024 16:39

close off last segment

e57bae6

fix useless for select

1c37d35

add LiveCaptionsLanguage

e206786

add LiveCaptionsLanguage debug statement

04ad5b5

streamer45 self-requested a review March 26, 2024 15:38

streamer45 reviewed Mar 27, 2024

View reviewed changes

cpoile added 2 commits March 27, 2024 13:55

PR comments

302d2e4

no need for loop label

efce4ad

streamer45 previously approved these changes Mar 27, 2024

View reviewed changes

streamer45 added 3: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Mar 27, 2024

upgrade tagged dependencies

1d0ce6a

cpoile dismissed streamer45’s stale review via 1d0ce6a March 27, 2024 18:41

SendWs -> SendWS; go mod tidy

b19051a

cpoile requested a review from streamer45 March 27, 2024 19:00

streamer45 approved these changes Mar 27, 2024

View reviewed changes

cpoile merged commit d5cfa96 into master Mar 27, 2024
2 checks passed

cpoile deleted the MM-56540-live-captions branch March 27, 2024 19:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MM-56540 - Live-captions support #18

MM-56540 - Live-captions support #18

cpoile commented Jan 25, 2024

cpoile commented Jan 25, 2024 •

edited

Loading

streamer45 left a comment •

edited

Loading

jupenur commented Mar 14, 2024

cpoile commented Mar 19, 2024

jupenur left a comment

streamer45 left a comment

streamer45 Mar 21, 2024

cpoile Mar 22, 2024

streamer45 Mar 27, 2024

streamer45 left a comment

streamer45 Mar 27, 2024

MM-56540 - Live-captions support #18

MM-56540 - Live-captions support #18

Conversation

cpoile commented Jan 25, 2024

Summary

Ticket Link

cpoile commented Jan 25, 2024 • edited Loading

streamer45 left a comment • edited Loading

Choose a reason for hiding this comment

jupenur commented Mar 14, 2024

cpoile commented Mar 19, 2024

jupenur left a comment

Choose a reason for hiding this comment

streamer45 left a comment

Choose a reason for hiding this comment

streamer45 Mar 21, 2024

Choose a reason for hiding this comment

cpoile Mar 22, 2024

Choose a reason for hiding this comment

streamer45 Mar 27, 2024

Choose a reason for hiding this comment

streamer45 left a comment

Choose a reason for hiding this comment

streamer45 Mar 27, 2024

Choose a reason for hiding this comment

cpoile commented Jan 25, 2024 •

edited

Loading

streamer45 left a comment •

edited

Loading