Unify hidden domain usage for recording and transcription #1737
Conversation
|
Any progress here? Looking forward for this to be merged |
| @@ -170,6 +172,11 @@ org.jitsi.jigasi.transcription.SAVE_TXT=true | |||
| org.jitsi.jigasi.transcription.SEND_TXT={{ .Env.JIGASI_TRANSCRIBER_SEND_TXT | default "false"}} | |||
| org.jitsi.jigasi.transcription.RECORD_AUDIO={{ .Env.JIGASI_TRANSCRIBER_RECORD_AUDIO | default "false"}} | |||
| org.jitsi.jigasi.transcription.RECORD_AUDIO_FORMAT=wav | |||
| # non-anonymous authentication is required for transcriber | |||
There was a problem hiding this comment.
These properties are already defined at line ~145: what's the difference between JIGASI_TRANSCRIBER_USER and JIGASI_XMPP_USER?
There was a problem hiding this comment.
I don't know. This may be a question to the architects of jigasi, whether a separate authentication for the transcriber is desired. I found preparations for a dedicated authentication in the code and just wanted to make the feature work.
Maybe it has to do with other jigasi features (SIP) that a separate authentication is desired?
There was a problem hiding this comment.
We don't need an extra user. JIgasi can be configured either as a trascriber or a SIP audio bridge, but not both at the same time. Thus, JIGASI_XMPP_USER (and the other ones) should be good.
There was a problem hiding this comment.
I plan to work shortly on transcribers as a first-class docker-jitsi-meet component, separated from the jigasi SIP component. I should have PRs open by next week, in case this helps.
| @@ -170,6 +172,11 @@ org.jitsi.jigasi.transcription.SAVE_TXT=true | |||
| org.jitsi.jigasi.transcription.SEND_TXT={{ .Env.JIGASI_TRANSCRIBER_SEND_TXT | default "false"}} | |||
| org.jitsi.jigasi.transcription.RECORD_AUDIO={{ .Env.JIGASI_TRANSCRIBER_RECORD_AUDIO | default "false"}} | |||
| org.jitsi.jigasi.transcription.RECORD_AUDIO_FORMAT=wav | |||
| # non-anonymous authentication is required for transcriber | |||
There was a problem hiding this comment.
We don't need an extra user. JIgasi can be configured either as a trascriber or a SIP audio bridge, but not both at the same time. Thus, JIGASI_XMPP_USER (and the other ones) should be good.
|
I would like to test this PR on my development environment, however as it is now this patch doesn't apply cleanly on the master branch due to recent commits (notably 360361e). |
|
I will try to update the patch versus master in the near future. In the meantime, please check release 9220, it should apply cleanly against this one. |
|
I can confirm this is working nicely on top of stable-9220, i am using this with some small changes (unrelated to this PR, just to get Vosk transcription instead of the default Google Cloud service), thanks. |
…single usage anymore
…iptions are enabled
…fault value hidden.meet.jitsi (as suggested by @saghul)
…harm even if both is disabled
zobadaniel
force-pushed
the
unify_hidden_domain_usage_for_recording_and_transcription
branch
from
07365a3
to
9fca6ad
Compare
|
The requested changes have been implemented by @M4GNV5 and the PR rebased. Please re-check and merge if accepted. |
| @@ -4,7 +4,6 @@ | |||
| {{ $ENABLE_SUBDOMAINS := .Env.ENABLE_SUBDOMAINS | default "true" | toBool -}} | |||
| {{ $ENABLE_XMPP_WEBSOCKET := .Env.ENABLE_XMPP_WEBSOCKET | default "1" | toBool -}} | |||
| {{ $JIBRI_RECORDER_USER := .Env.JIBRI_RECORDER_USER | default "recorder" -}} | |||
| {{ $JIGASI_TRANSCRIBER_USER := .Env.JIGASI_TRANSCRIBER_USER | default "transcriber" -}} | |||
There was a problem hiding this comment.
The transcriber user was combined with the JIGASI_XMPP_USER after the previous review comments here: #1737 (comment)
There was a problem hiding this comment.
Jigasi has two connections. First is the the XMPP command/control connection to the brewery MUC where jicofo can dispatch it to a conference. This first connection uses the JIGASI_XMPP_USER, which is a service account never seen by end users.
Second is the actual conference connection, where it authenticates as the transcriber user so as to be part of the hidden domain and not appear as a participant, as well as not being thwarted by lobby/passcode, etc. Both accounts are required to make transcriber work.
…s a seperate file)
it does not hurt to have them, so the conditions simply increase config complexity
M4GNV5
force-pushed
the
unify_hidden_domain_usage_for_recording_and_transcription
branch
from
5c940d5
to
48e66eb
Compare
|
@saghul hope everything is ok now. I cannot mark your comments as resolved, but I fixed the things you requested. |
|
I see the branch has conflicts, can you rebase? |
More generic approach to proper transcriber inclusion into docker.
I made a new pull request, based on the info & feedback from here: #1663
The patch is bigger because I renamed XMPP_RECORDER_DOMAIN. The name would be confusing if it is not the sole purpose of that domain.
I'm still not 100% sure this is the right way to do it. Please let me know your comments.
Patch has been tested vs release 9220.