Whisper live transcription #491

rpurdel · 2023-08-14T07:36:01Z

No description provided.

bgrozev

I added some comments for things that I noticed, but I didn't read thoroughly (I leave that to @damencho). Notably, I think there are some thread safety issues (unless I'm missing a reason why thread safety is not needed).

src/main/java/org/jitsi/jigasi/transcription/AbstractTranscriptionService.java

src/main/java/org/jitsi/jigasi/health/SipHealthPeriodicChecker.java

src/main/java/org/jitsi/jigasi/transcription/TranscribingAudioMixerMediaDevice.java

src/main/java/org/jitsi/jigasi/transcription/WhisperConnectionPool.java

src/main/java/org/jitsi/jigasi/transcription/WhisperWebsocket.java

src/main/java/org/jitsi/jigasi/TranscriptionGatewaySession.java

src/main/java/org/jitsi/jigasi/transcription/WhisperTsAudioSilenceCaptureDevice.java

damencho · 2023-09-08T14:19:16Z

Have you ran the check style? It should complain for all those { that are not on new line.

damencho · 2023-09-08T14:22:12Z

How are you building the project? mvn install runs the checkstyle:

[INFO] --- maven-checkstyle-plugin:3.1.2:check (default) @ jigasi ---
[INFO] You have 0 Checkstyle violations.

src/main/java/org/jitsi/jigasi/transcription/Transcriber.java

src/main/java/org/jitsi/jigasi/transcription/WhisperAudioSilenceCaptureDevice.java

src/main/java/org/jitsi/jigasi/transcription/WhisperAudioSilenceMediaDevice.java

src/main/java/org/jitsi/jigasi/transcription/WhisperConnectionPool.java

src/main/java/org/jitsi/jigasi/transcription/WhisperTranscriptionService.java

src/main/java/org/jitsi/jigasi/transcription/WhisperWebsocket.java

…e from debugName, remove unused prop

When connect slows down then the joined muc event is slowed down and the session-initiate (jicofo invite) arrives first and the chatRoom is null at that time.

codecov · 2023-09-11T17:30:21Z

Codecov Report

Merging #491 (6a5467b) into master (156938a) will decrease coverage by 1.10%.
Report is 1 commits behind head on master.
The diff coverage is 1.11%.

❗ Current head 6a5467b differs from pull request most recent head 127ff7d. Consider uploading reports for the commit 127ff7d to get more accurate results

@@             Coverage Diff              @@
##             master     #491      +/-   ##
============================================
- Coverage     22.06%   20.96%   -1.10%     
- Complexity      285      288       +3     
============================================
  Files            68       74       +6     
  Lines          5827     6166     +339     
  Branches        788      827      +39     
============================================
+ Hits           1286     1293       +7     
- Misses         4318     4651     +333     
+ Partials        223      222       -1

Files Changed	Coverage
src/main/java/org/jitsi/jigasi/JvbConference.java	`0.00%`
.../main/java/org/jitsi/jigasi/SipGatewaySession.java	`0.00%`
...in/java/org/jitsi/jigasi/TranscriptionGateway.java	`0.00%`
.../org/jitsi/jigasi/TranscriptionGatewaySession.java	`0.00%`
.../jitsi/jigasi/health/SipHealthPeriodicChecker.java	`ø`
...asi/transcription/AbstractTranscriptPublisher.java	`0.00%`
...si/transcription/AbstractTranscriptionService.java	`0.00%`
...transcription/GoogleCloudTranscriptionService.java	`ø`
...ranscription/LibreTranslateTranslationService.java	`ø`
...va/org/jitsi/jigasi/transcription/Participant.java	`ø`
... and 10 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b15bde3...127ff7d. Read the comment docs.

damencho · 2023-09-13T13:03:38Z

src/main/java/org/jitsi/jigasi/TranscriptionGateway.java

+import org.osgi.framework.BundleContext;
+
+import java.io.*;
+import java.net.*;



Can you fix the imports.

damencho · 2023-09-13T13:03:58Z

src/main/java/org/jitsi/jigasi/TranscriptionGateway.java

+                }
+                else
+                {
+                    logger.warn("+++++++++ Got HTTP status code " + responseCode);


Can you drop the debug logging.

damencho · 2023-09-13T13:04:01Z

src/main/java/org/jitsi/jigasi/TranscriptionGateway.java

            {
-                logger.warn("Could not find '" + tenant + "' tenant in custom transcription service JSON property.");
-                return null;
+                logger.error("+++++++++++++ Could not retrieve transcriber from remote URL." + ex);


Can you drop the debug logging.

damencho · 2023-09-13T13:05:12Z

src/main/java/org/jitsi/jigasi/TranscriptionGateway.java

+                        responseBody.append(inputLine);
+                    }
+                    inputStream.close();
+                    logger.info("+++++++++ Received json " + responseBody);


damencho · 2023-09-13T13:05:18Z

src/main/java/org/jitsi/jigasi/TranscriptionGateway.java

+                    logger.info("+++++++++ Received json " + responseBody);
+                    JSONObject obj = new JSONObject(responseBody.toString());
+                    transcriberClass = obj.getString("transcriber");
+                    logger.info("+++++++++ Using " + transcriberClass + " as the transcriber class.");


rpurdel requested review from damencho, bgrozev and oanaianc August 14, 2023 07:36

bgrozev reviewed Aug 14, 2023

View reviewed changes

damencho reviewed Aug 16, 2023

View reviewed changes

src/main/java/org/jitsi/jigasi/TranscriptionGatewaySession.java Outdated Show resolved Hide resolved

src/main/java/org/jitsi/jigasi/transcription/WhisperTsAudioSilenceCaptureDevice.java Outdated Show resolved Hide resolved

rpurdel requested review from bgrozev and damencho September 8, 2023 07:35