Skip to content

Conversation

@enoch85
Copy link
Member

@enoch85 enoch85 commented Jul 19, 2023

Fix #2519

cc @szaimen

enoch85 added 5 commits July 19, 2023 18:35
Signed-off-by: Daniel Hansson <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Daniel Hansson <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Daniel Hansson <[email protected]>
enoch85 added 2 commits July 19, 2023 19:16
Signed-off-by: Daniel Hansson <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
enoch85 and others added 2 commits July 19, 2023 19:34
Signed-off-by: Daniel Hansson <[email protected]>
Co-authored-by: Simon L. <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
enoch85 added 2 commits July 19, 2023 19:40
Signed-off-by: Daniel Hansson <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
@enoch85
Copy link
Member Author

enoch85 commented Jul 19, 2023

@szaimen What do you think? Did I understand you correctly or is something wrong here?

Not tested.

@szaimen
Copy link
Collaborator

szaimen commented Jul 19, 2023

@szaimen What do you think? Did I understand you correctly or is something wrong here?

Not tested.

lgtm in general. However If you compare the talk-hpb with https://github.com/nextcloud/all-in-one/blob/main/Containers/talk/start.sh, the turn section can be removed

enoch85 and others added 3 commits July 19, 2023 21:10
Co-authored-by: Simon L. <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
Co-authored-by: Simon L. <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
Co-authored-by: Simon L. <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
@enoch85
Copy link
Member Author

enoch85 commented Jul 19, 2023

However If you compare the talk-hpb with https://github.com/nextcloud/all-in-one/blob/main/Containers/talk/start.sh, the turn section can be removed

Hmm, not so sure about that. I've seen it in an enterprise setup as well. As you already know, we use @morph027 setup for Signaling, and the JANUS_KEY is there, not added by me, and it works as is. This is just an addition.

I suppose you mean this: https://github.com/nextcloud/vm/blob/master/apps/talk.sh#L341-L343

@szaimen
Copy link
Collaborator

szaimen commented Jul 19, 2023

@enoch85
Copy link
Member Author

enoch85 commented Jul 19, 2023

I suppose you mean this: https://github.com/nextcloud/vm/blob/master/apps/talk.sh#L341-L343

yes

I'll leave it, don't know the side effects if I remove it.

@szaimen
Copy link
Collaborator

szaimen commented Jul 19, 2023

I'll leave it, don't know the side effects if I remove it.

The side-effect of leaving it could be that the recording server does not work.

Signed-off-by: Daniel Hansson <[email protected]>
@enoch85
Copy link
Member Author

enoch85 commented Jul 19, 2023

I'll leave it, don't know the side effects if I remove it.

The side-effect of leaving it could be that the recording server does not work.

Actually, we have that in another setup (not your container, and my colleges installation), and it's working there. Do you mean that your container doesn't handle it, or what's the reason for it stop working?

Also, it should only be removed if talk recording is turned on?

@szaimen
Copy link
Collaborator

szaimen commented Jul 19, 2023

Actually, we have that in another setup (not your container, and my colleges installation), and it's working there. Do you mean that your container doesn't handle it, or what's the reason for it stop working?

Also, it should only be removed if talk recording is turned on?

The config is simply wrong as far as I know. The talk-hpb does not need a turn server afaik. Turn is only used by the android and ios apps directly.

@enoch85
Copy link
Member Author

enoch85 commented Jul 20, 2023

The config is simply wrong as far as I know.

Can you give your take on this?

I will merge this now for easier testing. We can continue the discussion in this PR though.

@enoch85 enoch85 merged commit 86e8266 into master Jul 20, 2023
@delete-merged-branch delete-merged-branch bot deleted the talk-recording branch July 20, 2023 07:07
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.

Implement Talk recording

3 participants