-
Notifications
You must be signed in to change notification settings - Fork 349
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
feat: Enable transcription based on MUC config form #1135
Conversation
Read room_metadata from the MUC config form, and enable transcription if it is requested.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1135 +/- ##
============================================
+ Coverage 28.57% 28.60% +0.03%
- Complexity 463 466 +3
============================================
Files 128 129 +1
Lines 7742 7778 +36
Branches 1058 1061 +3
============================================
+ Hits 2212 2225 +13
- Misses 5263 5286 +23
Partials 267 267
Continue to review full report in Codecov by Sentry.
|
{ | ||
if (jigasiDetector == null) | ||
{ | ||
logger.warn("Transcription requested, but jigasiDetector is not configured."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note for the future: it would be useful to somehow propagate this all the way to the client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed
@Override | ||
public void transcriptionRequestedChanged(boolean transcriptionRequested) | ||
{ | ||
if (transcriptionRequested) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this check for the active flag too? Note that right now there will be 2 ways to trigger this and both will happen roughly at once:
- presence
- room metadata
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why the tasks are put on a single threaded executor. I can check the flag here, but I'll have to re-check it in the other thread too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I'm not familiar with the code so I thought I'd ask. SGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good question
Read room_metadata from the MUC config form, and enable transcription if
it is requested.