Safemobile/mediasoup
21
0
Fork 0
Code Issues Pull Requests 1 Projects Releases Wiki Activity

LINXD-2209-black-screen-when-2-video-calls-are-answered-simultaneously #3

Merged
sergiu merged 31 commits from LINXD-2209-black-screen-when-2-video-calls-are-answered-simultaneously into master 2022-09-15 14:49:55 +00:00
Conversation 0 Commits 31 Files Changed 1 +93 -80
sergiu commented 2022-09-13 19:59:55 +00:00
Member
Copy Link
No description provided.
sergiu added 21 commits 2022-09-13 19:59:55 +00:00
sergiu requested review from cristi 2022-09-13 19:59:59 +00:00
sergiu changed title from LINXD-2209-black-screen-when-2-video-calls-are-answered-simultaneously to WIP: LINXD-2209-black-screen-when-2-video-calls-are-answered-simultaneously 2022-09-13 20:00:33 +00:00
sergiu added 1 commit 2022-09-15 06:19:55 +00:00
sergiu added 1 commit 2022-09-15 06:33:27 +00:00
sergiu added 1 commit 2022-09-15 06:34:09 +00:00
sergiu added 1 commit 2022-09-15 06:36:51 +00:00
sergiu added 1 commit 2022-09-15 06:40:31 +00:00
sergiu added 1 commit 2022-09-15 06:42:03 +00:00
sergiu added 1 commit 2022-09-15 06:44:41 +00:00
sergiu added 1 commit 2022-09-15 06:57:12 +00:00
sergiu added 1 commit 2022-09-15 06:58:42 +00:00
sergiu changed title from WIP: LINXD-2209-black-screen-when-2-video-calls-are-answered-simultaneously to LINXD-2209-black-screen-when-2-video-calls-are-answered-simultaneously 2022-09-15 07:01:53 +00:00
cristi approved these changes 2022-09-15 10:44:17 +00:00
cristi left a comment
Owner
Copy Link

I think we can use it the way it is and refactor afterwards. Please see comments.

Considering that all the event handlers for a connection are scoped inside the peers.on('connection', async socket => {, I think that we can have all variables declared and used in here without conflict, without the need for videoCalls and socketDetails hashes.

peers.on('connection', async socket => { 

  let callId 		
  let router = {}				/**
  let producerTransport				 * videoCalls
  let consumerTransport				 * |-> Router
  let producer				 * |-> Producer
  let consumer
  ...
  socket.on (...

This is not possible as the router need to be shared between producer and consumer.

I think we can use it the way it is and refactor afterwards. Please see comments. Considering that all the event handlers for a connection are scoped inside the `peers.on('connection', async socket => {`, I think that we can have all variables declared and used in here without conflict, without the need for `videoCalls` and `socketDetails` hashes. ```javascript peers.on('connection', async socket => { let callId let router = {} /** let producerTransport * videoCalls let consumerTransport * |-> Router let producer * |-> Producer let consumer ... socket.on (... ``` --- This is not possible as the router need to be shared between producer and consumer.
app.js Outdated
@ -101,3 +110,2 @@
socket.emit('connection-success', {
socketId: socket.id,
existsProducer: producer ? true : false,
socketId: socket.id
cristi commented 2022-09-15 10:06:38 +00:00
Owner
Copy Link

not important: we don't need socket.id on clients

not important: we don't need socket.id on clients
app.js Outdated
@ -141,2 +138,3 @@
videoCalls[callId].producerTransport = await createWebRtcTransportLayer(callId, callback)
else
consumerTransport = await createWebRtcTransportLayer(callId, callback)
videoCalls[callId].consumerTransport = await createWebRtcTransportLayer(callId, callback)
cristi commented 2022-09-15 10:38:51 +00:00
Owner
Copy Link

For refactor: why do we need to differentiate between producerTransport and consumerTransport? Why don't we have a transport variable (it is only one transport for each client regardless if they are producer or consumer)?

For refactor: why do we need to differentiate between producerTransport and consumerTransport? Why don't we have a `transport` variable (it is only one transport for each client regardless if they are producer or consumer)?
cristi commented 2022-09-15 13:58:38 +00:00
Owner
Copy Link

We need producer and consumer transports for each call.

We need producer and consumer transports for each call.
cristi marked this conversation as resolved
app.js Outdated
@ -211,0 +214,4 @@
videoCalls[callId].router.close()
videoCalls[callId].producer.close()
videoCalls[callId].consumer.close()
delete videoCalls[callId].router
cristi commented 2022-09-15 10:39:42 +00:00
Owner
Copy Link

Function to clearVideoCall and socket.

Function to clearVideoCall and socket.
sergiu added 1 commit 2022-09-15 14:08:35 +00:00
sergiu merged commit 4a98a79630 into master 2022-09-15 14:49:55 +00:00
sergiu deleted branch LINXD-2209-black-screen-when-2-video-calls-are-answered-simultaneously 2022-09-15 14:50:01 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
cristi
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Safemobile/mediasoup#3
No description provided.
Delete Branch "LINXD-2209-black-screen-when-2-video-calls-are-answered-simultaneously"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?