From b9d28d11cdf8f8474dc4b482754cb45f4eeb2aa9 Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Fri, 25 Sep 2020 16:45:03 +0200 Subject: [PATCH 1/9] openvidu-server: composed quickstart recording stop bug fix --- .../ComposedQuickStartRecordingService.java | 4 +- .../service/ComposedRecordingService.java | 2 +- .../openvidu/server/utils/DockerManager.java | 46 ++++++++++++------- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedQuickStartRecordingService.java b/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedQuickStartRecordingService.java index f1262adb..5ab5588e 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedQuickStartRecordingService.java +++ b/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedQuickStartRecordingService.java @@ -78,7 +78,7 @@ public class ComposedQuickStartRecordingService extends ComposedRecordingService recordExecCommand += "export " + envs.get(i) + " "; } recordExecCommand += "&& ./composed_quick_start.sh --start-recording > /var/log/ffmpeg.log 2>&1 &"; - dockerManager.runCommandInContainer(containerId, recordExecCommand, 0); + dockerManager.runCommandInContainer(containerId, recordExecCommand); } catch (Exception e) { this.cleanRecordingMaps(recording); throw this.failStartRecording(session, recording, @@ -116,7 +116,7 @@ public class ComposedQuickStartRecordingService extends ComposedRecordingService } try { - dockerManager.runCommandInContainer(containerId, "./composed_quick_start.sh --stop-recording", 10); + dockerManager.runCommandInContainerSync(containerId, "./composed_quick_start.sh --stop-recording", 10); } catch (InterruptedException e1) { cleanRecordingMaps(recording); log.error("Error stopping recording for session id: {}", session.getSessionId()); diff --git a/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java b/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java index 19020abe..e91ef8b9 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java +++ b/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java @@ -396,7 +396,7 @@ public class ComposedRecordingService extends RecordingService { protected void stopAndRemoveRecordingContainer(Recording recording, String containerId, int secondsOfWait) { // Gracefully stop ffmpeg process try { - dockerManager.runCommandInContainer(containerId, "echo 'q' > stop", 0); + dockerManager.runCommandInContainer(containerId, "echo 'q' > stop"); } catch (InterruptedException e1) { e1.printStackTrace(); } diff --git a/openvidu-server/src/main/java/io/openvidu/server/utils/DockerManager.java b/openvidu-server/src/main/java/io/openvidu/server/utils/DockerManager.java index bb4a87d0..c616729b 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/utils/DockerManager.java +++ b/openvidu-server/src/main/java/io/openvidu/server/utils/DockerManager.java @@ -26,23 +26,29 @@ import java.util.concurrent.TimeUnit; import javax.ws.rs.ProcessingException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.github.dockerjava.api.DockerClient; -import com.github.dockerjava.api.async.ResultCallback; -import com.github.dockerjava.api.command.*; +import com.github.dockerjava.api.command.CreateContainerCmd; +import com.github.dockerjava.api.command.CreateContainerResponse; +import com.github.dockerjava.api.command.ExecCreateCmdResponse; +import com.github.dockerjava.api.command.InspectContainerResponse; +import com.github.dockerjava.api.command.InspectImageResponse; import com.github.dockerjava.api.exception.ConflictException; import com.github.dockerjava.api.exception.DockerClientException; import com.github.dockerjava.api.exception.InternalServerErrorException; import com.github.dockerjava.api.exception.NotFoundException; -import com.github.dockerjava.api.model.*; +import com.github.dockerjava.api.model.Bind; +import com.github.dockerjava.api.model.Container; +import com.github.dockerjava.api.model.HostConfig; +import com.github.dockerjava.api.model.Volume; import com.github.dockerjava.core.DefaultDockerClientConfig; import com.github.dockerjava.core.DockerClientBuilder; import com.github.dockerjava.core.DockerClientConfig; import com.github.dockerjava.core.command.ExecStartResultCallback; import com.github.dockerjava.core.command.PullImageResultCallback; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import io.openvidu.client.OpenViduException; import io.openvidu.client.OpenViduException.Code; import io.openvidu.server.recording.service.WaitForContainerStoppedCallback; @@ -63,7 +69,7 @@ public class DockerManager { // Pull image this.dockerClient.pullImageCmd(image).exec(new PullImageResultCallback()).awaitCompletion(secondsOfWait, TimeUnit.SECONDS); - + } catch (NotFoundException | InternalServerErrorException e) { if (dockerImageExistsLocally(image)) { log.info("Docker image '{}' exists locally", image); @@ -102,9 +108,8 @@ public class DockerManager { } public String runContainer(String container, String containerName, String user, List volumes, - List binds, String networkMode, List envs, List command, Long shmSize, boolean privileged, - Map labels) - throws Exception { + List binds, String networkMode, List envs, List command, Long shmSize, + boolean privileged, Map labels) throws Exception { CreateContainerCmd cmd = dockerClient.createContainerCmd(container).withEnv(envs); if (containerName != null) { @@ -167,21 +172,30 @@ public class DockerManager { } } - public String runCommandInContainer(String containerId, String command, int secondsOfWait) + public void runCommandInContainer(String containerId, String command) throws InterruptedException { + ExecCreateCmdResponse execCreateCmdResponse = dockerClient.execCreateCmd(containerId).withAttachStdout(true) + .withAttachStderr(true).withCmd("bash", "-c", command).exec(); + dockerClient.execStartCmd(execCreateCmdResponse.getId()).exec(new ExecStartResultCallback() { + }); + } + + public void runCommandInContainerSync(String containerId, String command, int secondsOfWait) throws InterruptedException { ExecCreateCmdResponse execCreateCmdResponse = dockerClient.execCreateCmd(containerId).withAttachStdout(true) .withAttachStderr(true).withCmd("bash", "-c", command).exec(); CountDownLatch latch = new CountDownLatch(1); - final String[] stringResponse = new String[1]; dockerClient.execStartCmd(execCreateCmdResponse.getId()).exec(new ExecStartResultCallback() { @Override - public void onNext(Frame item) { - stringResponse[0] = new String(item.getPayload()); + public void onComplete() { latch.countDown(); } }); - latch.await(secondsOfWait, TimeUnit.SECONDS); - return stringResponse[0]; + try { + latch.await(secondsOfWait, TimeUnit.SECONDS); + } catch (InterruptedException e) { + throw new InterruptedException("Container " + containerId + " did not return from executing command \"" + + command + "\" in " + secondsOfWait + " seconds"); + } } public void waitForContainerStopped(String containerId, int secondsOfWait) throws Exception { From eb7fb81995f42192bf6f221fa96c4fa51d747197 Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Fri, 25 Sep 2020 17:34:46 +0200 Subject: [PATCH 2/9] openvidu-test-e2e: improved quick start record test --- .../test/e2e/OpenViduTestAppE2eTest.java | 189 ++++++++++-------- 1 file changed, 102 insertions(+), 87 deletions(-) diff --git a/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java b/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java index 5f9986c1..6fa0317d 100644 --- a/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java +++ b/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java @@ -1245,96 +1245,111 @@ public class OpenViduTestAppE2eTest { setupBrowser("chrome"); log.info("Remote composed quick start record"); + + CountDownLatch initLatch = new CountDownLatch(1); + io.openvidu.test.browsers.utils.CustomWebhook.main(new String[0], initLatch); - final String sessionName = "COMPOSED_QUICK_START_RECORDED_SESSION"; + try { - // 1. MANUAL mode and recording explicitly stopped + if (!initLatch.await(30, TimeUnit.SECONDS)) { + Assert.fail("Timeout waiting for webhook springboot app to start"); + CustomWebhook.shutDown(); + return; + } - user.getDriver().findElement(By.id("add-user-btn")).click(); - user.getDriver().findElement(By.id("session-name-input-0")).clear(); - user.getDriver().findElement(By.id("session-name-input-0")).sendKeys(sessionName); - - user.getDriver().findElement(By.id("session-settings-btn-0")).click(); - Thread.sleep(1000); - user.getDriver().findElement(By.id("output-mode-select")).click(); - Thread.sleep(500); - user.getDriver().findElement(By.id("option-COMPOSED_QUICK_START")).click(); - Thread.sleep(500); - user.getDriver().findElement(By.id("save-btn")).click(); - Thread.sleep(1000); - - // Join the subscriber user to the session - user.getDriver().findElement(By.cssSelector("#openvidu-instance-0 .publish-checkbox")).click(); - user.getDriver().findElement(By.className("join-btn")).click(); - user.getEventManager().waitUntilEventReaches("connectionCreated", 1); - - // Check the recording container is up and running but no ongoing recordings - checkDockerContainerRunning(RECORDING_IMAGE, 1); - Assert.assertEquals("Wrong number of recordings found", 0, OV.listRecordings().size()); - - // Join the publisher user to the session - user.getDriver().findElement(By.id("add-user-btn")).click(); - user.getDriver().findElement(By.id("session-name-input-1")).clear(); - user.getDriver().findElement(By.id("session-name-input-1")).sendKeys(sessionName); - user.getDriver().findElement(By.cssSelector("#openvidu-instance-1 .join-btn")).click(); - - user.getEventManager().waitUntilEventReaches("connectionCreated", 4); - user.getEventManager().waitUntilEventReaches("accessAllowed", 1); - user.getEventManager().waitUntilEventReaches("streamCreated", 2); - user.getEventManager().waitUntilEventReaches("streamPlaying", 2); - - // Start recording - OV.fetch(); - String recId = OV.startRecording(sessionName).getId(); - user.getEventManager().waitUntilEventReaches("recordingStarted", 2); - checkDockerContainerRunning("openvidu/openvidu-recording", 1); - - Thread.sleep(1000); - - Assert.assertEquals("Wrong number of recordings found", 1, OV.listRecordings().size()); - OV.stopRecording(recId); - user.getEventManager().waitUntilEventReaches("recordingStopped", 2); - checkDockerContainerRunning("openvidu/openvidu-recording", 1); - - Assert.assertEquals("Wrong number of sessions", 1, OV.getActiveSessions().size()); - Session session = OV.getActiveSessions().get(0); - session.close(); - - checkDockerContainerRunning("openvidu/openvidu-recording", 0); - - // 2. ALWAYS mode and recording stopped by session close up - user.getDriver().findElement(By.id("remove-all-users-btn")).click(); - user.getDriver().findElement(By.id("add-user-btn")).click(); - user.getDriver().findElement(By.id("session-name-input-0")).clear(); - user.getDriver().findElement(By.id("session-name-input-0")).sendKeys(sessionName); - - user.getDriver().findElement(By.id("session-settings-btn-0")).click(); - Thread.sleep(1000); - user.getDriver().findElement(By.id("recording-mode-select")).click(); - Thread.sleep(500); - user.getDriver().findElement(By.id("option-ALWAYS")).click(); - Thread.sleep(500); - user.getDriver().findElement(By.id("output-mode-select")).click(); - Thread.sleep(500); - user.getDriver().findElement(By.id("option-COMPOSED_QUICK_START")).click(); - Thread.sleep(500); - user.getDriver().findElement(By.id("save-btn")).click(); - Thread.sleep(1000); - - user.getDriver().findElement(By.className("join-btn")).click(); - user.getEventManager().waitUntilEventReaches("connectionCreated", 5); - user.getEventManager().waitUntilEventReaches("accessAllowed", 2); - user.getEventManager().waitUntilEventReaches("streamCreated", 3); - user.getEventManager().waitUntilEventReaches("streamPlaying", 3); - user.getEventManager().waitUntilEventReaches("recordingStarted", 3); - - checkDockerContainerRunning("openvidu/openvidu-recording", 1); - - OV.fetch(); - session = OV.getActiveSessions().get(0); - session.close(); - - checkDockerContainerRunning("openvidu/openvidu-recording", 0); + final String sessionName = "COMPOSED_QUICK_START_RECORDED_SESSION"; + + // 1. MANUAL mode and recording explicitly stopped + + user.getDriver().findElement(By.id("add-user-btn")).click(); + user.getDriver().findElement(By.id("session-name-input-0")).clear(); + user.getDriver().findElement(By.id("session-name-input-0")).sendKeys(sessionName); + + user.getDriver().findElement(By.id("session-settings-btn-0")).click(); + Thread.sleep(1000); + user.getDriver().findElement(By.id("output-mode-select")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("option-COMPOSED_QUICK_START")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("save-btn")).click(); + Thread.sleep(1000); + + // Join the subscriber user to the session + user.getDriver().findElement(By.cssSelector("#openvidu-instance-0 .publish-checkbox")).click(); + user.getDriver().findElement(By.className("join-btn")).click(); + user.getEventManager().waitUntilEventReaches("connectionCreated", 1); + + // Check the recording container is up and running but no ongoing recordings + checkDockerContainerRunning(RECORDING_IMAGE, 1); + Assert.assertEquals("Wrong number of recordings found", 0, OV.listRecordings().size()); + + // Join the publisher user to the session + user.getDriver().findElement(By.id("add-user-btn")).click(); + user.getDriver().findElement(By.id("session-name-input-1")).clear(); + user.getDriver().findElement(By.id("session-name-input-1")).sendKeys(sessionName); + user.getDriver().findElement(By.cssSelector("#openvidu-instance-1 .join-btn")).click(); + + user.getEventManager().waitUntilEventReaches("connectionCreated", 4); + user.getEventManager().waitUntilEventReaches("accessAllowed", 1); + user.getEventManager().waitUntilEventReaches("streamCreated", 2); + user.getEventManager().waitUntilEventReaches("streamPlaying", 2); + + // Start recording + OV.fetch(); + String recId = OV.startRecording(sessionName).getId(); + user.getEventManager().waitUntilEventReaches("recordingStarted", 2); + CustomWebhook.waitForEvent("recordingStatusChanged", 1); + checkDockerContainerRunning("openvidu/openvidu-recording", 1); + + Thread.sleep(2000); + + Assert.assertEquals("Wrong number of recordings found", 1, OV.listRecordings().size()); + OV.stopRecording(recId); + user.getEventManager().waitUntilEventReaches("recordingStopped", 2); + checkDockerContainerRunning("openvidu/openvidu-recording", 1); + + Assert.assertEquals("Wrong number of sessions", 1, OV.getActiveSessions().size()); + Session session = OV.getActiveSessions().get(0); + session.close(); + + checkDockerContainerRunning("openvidu/openvidu-recording", 0); + + // 2. ALWAYS mode and recording stopped by session close up + user.getDriver().findElement(By.id("remove-all-users-btn")).click(); + user.getDriver().findElement(By.id("add-user-btn")).click(); + user.getDriver().findElement(By.id("session-name-input-0")).clear(); + user.getDriver().findElement(By.id("session-name-input-0")).sendKeys(sessionName); + + user.getDriver().findElement(By.id("session-settings-btn-0")).click(); + Thread.sleep(1000); + user.getDriver().findElement(By.id("recording-mode-select")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("option-ALWAYS")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("output-mode-select")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("option-COMPOSED_QUICK_START")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("save-btn")).click(); + Thread.sleep(1000); + + user.getDriver().findElement(By.className("join-btn")).click(); + user.getEventManager().waitUntilEventReaches("connectionCreated", 5); + user.getEventManager().waitUntilEventReaches("accessAllowed", 2); + user.getEventManager().waitUntilEventReaches("streamCreated", 3); + user.getEventManager().waitUntilEventReaches("streamPlaying", 3); + user.getEventManager().waitUntilEventReaches("recordingStarted", 3); + CustomWebhook.waitForEvent("recordingStatusChanged", 1); + checkDockerContainerRunning("openvidu/openvidu-recording", 1); + + OV.fetch(); + session = OV.getActiveSessions().get(0); + session.close(); + + checkDockerContainerRunning("openvidu/openvidu-recording", 0); + } finally { + CustomWebhook.shutDown(); + } } @Test From e294ac606b48722431d15718e6037e34ba7f7caf Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Fri, 25 Sep 2020 17:36:49 +0200 Subject: [PATCH 3/9] openvidu-server: typo fixed --- .../java/io/openvidu/server/recording/RecordingInfoUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openvidu-server/src/main/java/io/openvidu/server/recording/RecordingInfoUtils.java b/openvidu-server/src/main/java/io/openvidu/server/recording/RecordingInfoUtils.java index f4e2cddd..ac303add 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/recording/RecordingInfoUtils.java +++ b/openvidu-server/src/main/java/io/openvidu/server/recording/RecordingInfoUtils.java @@ -50,7 +50,7 @@ public class RecordingInfoUtils { throw new OpenViduException(Code.RECORDING_FILE_EMPTY_ERROR, "The recording file is corrupted"); } if (this.json.size() == 0) { - // Recording metadata from ffprobe is an emtpy JSON + // Recording metadata from ffprobe is an empty JSON throw new OpenViduException(Code.RECORDING_FILE_EMPTY_ERROR, "The recording file is empty"); } From 03503e586c4980c13432a717393f3a6d92b51eda Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Fri, 25 Sep 2020 23:53:21 +0200 Subject: [PATCH 4/9] openvidu-server: fixed wait for video file not empty in COMPOSED video recording --- .../service/ComposedRecordingService.java | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java b/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java index e91ef8b9..01a64264 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java +++ b/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java @@ -393,7 +393,7 @@ public class ComposedRecordingService extends RecordingService { return finalRecordingArray[0]; } - protected void stopAndRemoveRecordingContainer(Recording recording, String containerId, int secondsOfWait) { + private void stopAndRemoveRecordingContainer(Recording recording, String containerId, int secondsOfWait) { // Gracefully stop ffmpeg process try { dockerManager.runCommandInContainer(containerId, "echo 'q' > stop"); @@ -440,21 +440,26 @@ public class ComposedRecordingService extends RecordingService { } protected void waitForVideoFileNotEmpty(Recording recording) throws OpenViduException { - boolean isPresent = false; - int i = 1; - int timeout = 150; // Wait for 150*150 = 22500 = 22.5 seconds - while (!isPresent && timeout <= 150) { + + final String VIDEO_FILE = this.openviduConfig.getOpenViduRecordingPath() + recording.getId() + "/" + + recording.getName() + ".mp4"; + + int SECONDS_MAX_WAIT = 15; + int MILLISECONDS_INTERVAL_WAIT = 100; + int LIMIT = SECONDS_MAX_WAIT * 1000 / MILLISECONDS_INTERVAL_WAIT; + + int i = 0; + boolean arePresent = fileExistsAndHasBytes(VIDEO_FILE); + while (!arePresent && i < LIMIT) { try { - Thread.sleep(150); - timeout++; - File f = new File(this.openviduConfig.getOpenViduRecordingPath() + recording.getId() + "/" - + recording.getName() + ".mp4"); - isPresent = ((f.isFile()) && (f.length() > 0)); + Thread.sleep(MILLISECONDS_INTERVAL_WAIT); + arePresent = fileExistsAndHasBytes(VIDEO_FILE); + i++; } catch (InterruptedException e) { e.printStackTrace(); } } - if (i == timeout) { + if (!arePresent) { log.error("Recorder container failed generating video file (is empty) for session {}", recording.getSessionId()); throw new OpenViduException(Code.RECORDING_START_ERROR_CODE, @@ -462,7 +467,12 @@ public class ComposedRecordingService extends RecordingService { } } - protected void failRecordingCompletion(Recording recording, String containerId, OpenViduException e) + private boolean fileExistsAndHasBytes(String fileName) { + File f = new File(fileName); + return (f.exists() && f.isFile() && f.length() > 0); + } + + private void failRecordingCompletion(Recording recording, String containerId, OpenViduException e) throws OpenViduException { recording.setStatus(io.openvidu.java.client.Recording.Status.failed); dockerManager.removeDockerContainer(containerId, true); From 3a61ce0f8636e8a36083e9bb263064f114db15b0 Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Sat, 26 Sep 2020 01:12:25 +0200 Subject: [PATCH 5/9] openvidu-server: fix recording started client event --- .../server/core/SessionEventsHandler.java | 30 +++++++------------ .../recording/service/RecordingManager.java | 10 +++++-- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/openvidu-server/src/main/java/io/openvidu/server/core/SessionEventsHandler.java b/openvidu-server/src/main/java/io/openvidu/server/core/SessionEventsHandler.java index 982d96f5..bee845f2 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/core/SessionEventsHandler.java +++ b/openvidu-server/src/main/java/io/openvidu/server/core/SessionEventsHandler.java @@ -22,7 +22,6 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; import org.kurento.client.GenericMediaEvent; @@ -62,9 +61,7 @@ public class SessionEventsHandler { @Autowired protected OpenviduConfig openviduConfig; - Map recordingsStarted = new ConcurrentHashMap<>(); - - ReentrantLock lock = new ReentrantLock(); + private Map recordingsToSendClientEvents = new ConcurrentHashMap<>(); public void onSessionCreated(Session session) { CDR.recordSessionCreated(session); @@ -290,16 +287,10 @@ public class SessionEventsHandler { rpcNotificationService.sendResponse(participant.getParticipantPrivateId(), transactionId, result); if (ProtocolElements.RECORDER_PARTICIPANT_PUBLICID.equals(participant.getParticipantPublicId())) { - lock.lock(); - try { - Recording recording = this.recordingsStarted.remove(session.getSessionId()); - if (recording != null) { - // RECORDER participant is now receiving video from the first publisher - this.sendRecordingStartedNotification(session, recording); - } - } finally { - lock.unlock(); - } + recordingsToSendClientEvents.computeIfPresent(session.getSessionId(), (key, value) -> { + sendRecordingStartedNotification(session, value); + return null; + }); } } @@ -311,8 +302,9 @@ public class SessionEventsHandler { rpcNotificationService.sendResponse(participant.getParticipantPrivateId(), transactionId, new JsonObject()); } - public void onNetworkQualityChanged(Participant participant, JsonObject params ) { - rpcNotificationService.sendNotification(participant.getParticipantPrivateId(), ProtocolElements.NETWORKQUALITYCHANGED_METHOD, params); + public void onNetworkQualityChanged(Participant participant, JsonObject params) { + rpcNotificationService.sendNotification(participant.getParticipantPrivateId(), + ProtocolElements.NETWORKQUALITYCHANGED_METHOD, params); } public void onSendMessage(Participant participant, JsonObject message, Set participants, @@ -460,7 +452,7 @@ public class SessionEventsHandler { public void sendRecordingStoppedNotification(Session session, Recording recording, EndReason reason) { // Be sure to clean this map (this should return null) - this.recordingsStarted.remove(session.getSessionId()); + recordingsToSendClientEvents.remove(session.getSessionId()); // Filter participants by roles according to "OPENVIDU_RECORDING_NOTIFICATION" Set existingParticipants; @@ -570,8 +562,8 @@ public class SessionEventsHandler { this.rpcNotificationService.closeRpcSession(participantPrivateId); } - public void setRecordingStarted(String sessionId, Recording recording) { - this.recordingsStarted.put(sessionId, recording); + public void storeRecordingToSendClientEvent(Recording recording) { + recordingsToSendClientEvents.put(recording.getSessionId(), recording); } private Set filterParticipantsByRole(OpenViduRole[] roles, Set participants) { diff --git a/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java b/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java index f4666ddf..2a281104 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java +++ b/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java @@ -280,9 +280,12 @@ public class RecordingManager { this.cdr.recordRecordingStatusChanged(recording, null, recording.getCreatedAt(), Status.started); - if (!(OutputMode.COMPOSED.equals(properties.outputMode()) && properties.hasVideo())) { + if (!((properties.outputMode().equals(OutputMode.COMPOSED) + || properties.outputMode().equals(OutputMode.COMPOSED_QUICK_START)) + && properties.hasVideo())) { // Directly send recording started notification for all cases except for - // COMPOSED recordings with video (will be sent on first RECORDER subscriber) + // COMPOSED/COMPOSED_QUICK_START recordings with video (will be sent on first + // RECORDER subscriber) this.sessionHandler.sendRecordingStartedNotification(session, recording); } if (session.getActivePublishers() == 0) { @@ -835,6 +838,8 @@ public class RecordingManager { || (sessionsRecordingsStarting.putIfAbsent(recording.getSessionId(), recording) != null)) { log.error("Concurrent session recording initialization. Aborting this thread"); throw new RuntimeException("Concurrent initialization of recording " + recording.getId()); + } else { + this.sessionHandler.storeRecordingToSendClientEvent(recording); } } @@ -843,7 +848,6 @@ public class RecordingManager { * collection */ private void recordingFromStartingToStarted(Recording recording) { - this.sessionHandler.setRecordingStarted(recording.getSessionId(), recording); this.sessionsRecordings.put(recording.getSessionId(), recording); this.startingRecordings.remove(recording.getId()); this.sessionsRecordingsStarting.remove(recording.getSessionId()); From 0d00ad8fee1822affad597730bf8ef0b922e3c9b Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Sat, 26 Sep 2020 17:13:37 +0200 Subject: [PATCH 6/9] openvidu-server: fix close session with recording in starting status --- .../recording/service/RecordingManager.java | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java b/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java index 2a281104..a63533c4 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java +++ b/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java @@ -280,12 +280,10 @@ public class RecordingManager { this.cdr.recordRecordingStatusChanged(recording, null, recording.getCreatedAt(), Status.started); - if (!((properties.outputMode().equals(OutputMode.COMPOSED) - || properties.outputMode().equals(OutputMode.COMPOSED_QUICK_START)) - && properties.hasVideo())) { + if (!(OutputMode.COMPOSED.equals(properties.outputMode()) && properties.hasVideo())) { // Directly send recording started notification for all cases except for - // COMPOSED/COMPOSED_QUICK_START recordings with video (will be sent on first - // RECORDER subscriber) + // COMPOSED recordings with video (will be sent on first RECORDER subscriber) + // Both INDIVIDUAL and COMPOSED_QUICK_START should notify immediately this.sessionHandler.sendRecordingStartedNotification(session, recording); } if (session.getActivePublishers() == 0) { @@ -320,7 +318,19 @@ public class RecordingManager { recording = this.sessionsRecordings.get(session.getSessionId()); } - recording = ((RecordingService) singleStreamRecordingService).sealRecordingMetadataFileAsStopped(recording); + if (recording == null) { + recording = this.sessionsRecordingsStarting.get(session.getSessionId()); + if (recording == null) { + log.error("Cannot stop recording. Session {} is not being recorded", recordingId, + session.getSessionId()); + return null; + } else { + // Recording is still starting + log.warn("Recording {} is still in \"starting\" status", recording.getId()); + } + } + + ((RecordingService) singleStreamRecordingService).sealRecordingMetadataFileAsStopped(recording); final long timestamp = System.currentTimeMillis(); this.cdr.recordRecordingStatusChanged(recording, reason, timestamp, Status.stopped); @@ -412,8 +422,15 @@ public class RecordingManager { public void stopOneIndividualStreamRecording(KurentoSession session, String streamId, long kmsDisconnectionTime) { Recording recording = this.sessionsRecordings.get(session.getSessionId()); if (recording == null) { - log.error("Cannot stop recording of existing stream {}. Session {} is not being recorded", streamId, - session.getSessionId()); + recording = this.sessionsRecordingsStarting.get(session.getSessionId()); + if (recording == null) { + log.error("Cannot stop recording of existing stream {}. Session {} is not being recorded", streamId, + session.getSessionId()); + return; + } else { + // Recording is still starting + log.warn("Recording {} is still in \"starting\" status", recording.getId()); + } } if (OutputMode.INDIVIDUAL.equals(recording.getOutputMode())) { // Stop specific RecorderEndpoint for this stream From b5f83ea7cc9a6429c025d8e68216b431b059b64f Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Sat, 26 Sep 2020 17:14:15 +0200 Subject: [PATCH 7/9] openvidu-test-e2e: fixed composed recording tests --- .../test/browsers/utils/CustomWebhook.java | 4 + .../test/e2e/OpenViduTestAppE2eTest.java | 101 ++++++++++++++---- 2 files changed, 85 insertions(+), 20 deletions(-) diff --git a/openvidu-test-browsers/src/main/java/io/openvidu/test/browsers/utils/CustomWebhook.java b/openvidu-test-browsers/src/main/java/io/openvidu/test/browsers/utils/CustomWebhook.java index 2737785f..0ecf7aca 100644 --- a/openvidu-test-browsers/src/main/java/io/openvidu/test/browsers/utils/CustomWebhook.java +++ b/openvidu-test-browsers/src/main/java/io/openvidu/test/browsers/utils/CustomWebhook.java @@ -59,6 +59,10 @@ public class CustomWebhook { CustomWebhook.context.close(); } + public static void clean() { + CustomWebhook.events.clear(); + } + public synchronized static JsonObject waitForEvent(String eventName, int maxSecondsWait) throws Exception { if (events.get(eventName) == null) { events.put(eventName, new LinkedBlockingDeque<>()); diff --git a/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java b/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java index 6fa0317d..b787e0ce 100644 --- a/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java +++ b/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java @@ -1245,7 +1245,7 @@ public class OpenViduTestAppE2eTest { setupBrowser("chrome"); log.info("Remote composed quick start record"); - + CountDownLatch initLatch = new CountDownLatch(1); io.openvidu.test.browsers.utils.CustomWebhook.main(new String[0], initLatch); @@ -1258,13 +1258,14 @@ public class OpenViduTestAppE2eTest { } final String sessionName = "COMPOSED_QUICK_START_RECORDED_SESSION"; - + JsonObject event; + // 1. MANUAL mode and recording explicitly stopped - + user.getDriver().findElement(By.id("add-user-btn")).click(); user.getDriver().findElement(By.id("session-name-input-0")).clear(); user.getDriver().findElement(By.id("session-name-input-0")).sendKeys(sessionName); - + user.getDriver().findElement(By.id("session-settings-btn-0")).click(); Thread.sleep(1000); user.getDriver().findElement(By.id("output-mode-select")).click(); @@ -1273,53 +1274,57 @@ public class OpenViduTestAppE2eTest { Thread.sleep(500); user.getDriver().findElement(By.id("save-btn")).click(); Thread.sleep(1000); - + // Join the subscriber user to the session user.getDriver().findElement(By.cssSelector("#openvidu-instance-0 .publish-checkbox")).click(); user.getDriver().findElement(By.className("join-btn")).click(); user.getEventManager().waitUntilEventReaches("connectionCreated", 1); - + // Check the recording container is up and running but no ongoing recordings checkDockerContainerRunning(RECORDING_IMAGE, 1); Assert.assertEquals("Wrong number of recordings found", 0, OV.listRecordings().size()); - + // Join the publisher user to the session user.getDriver().findElement(By.id("add-user-btn")).click(); user.getDriver().findElement(By.id("session-name-input-1")).clear(); user.getDriver().findElement(By.id("session-name-input-1")).sendKeys(sessionName); user.getDriver().findElement(By.cssSelector("#openvidu-instance-1 .join-btn")).click(); - + user.getEventManager().waitUntilEventReaches("connectionCreated", 4); user.getEventManager().waitUntilEventReaches("accessAllowed", 1); user.getEventManager().waitUntilEventReaches("streamCreated", 2); user.getEventManager().waitUntilEventReaches("streamPlaying", 2); - + // Start recording OV.fetch(); String recId = OV.startRecording(sessionName).getId(); user.getEventManager().waitUntilEventReaches("recordingStarted", 2); CustomWebhook.waitForEvent("recordingStatusChanged", 1); checkDockerContainerRunning("openvidu/openvidu-recording", 1); - + Thread.sleep(2000); - + Assert.assertEquals("Wrong number of recordings found", 1, OV.listRecordings().size()); OV.stopRecording(recId); user.getEventManager().waitUntilEventReaches("recordingStopped", 2); checkDockerContainerRunning("openvidu/openvidu-recording", 1); - + Assert.assertEquals("Wrong number of sessions", 1, OV.getActiveSessions().size()); Session session = OV.getActiveSessions().get(0); session.close(); - + checkDockerContainerRunning("openvidu/openvidu-recording", 0); - + + Assert.assertEquals("Wrong recording status", Recording.Status.ready, + OV.getRecording(sessionName).getStatus()); + // 2. ALWAYS mode and recording stopped by session close up + CustomWebhook.clean(); user.getDriver().findElement(By.id("remove-all-users-btn")).click(); user.getDriver().findElement(By.id("add-user-btn")).click(); user.getDriver().findElement(By.id("session-name-input-0")).clear(); user.getDriver().findElement(By.id("session-name-input-0")).sendKeys(sessionName); - + user.getDriver().findElement(By.id("session-settings-btn-0")).click(); Thread.sleep(1000); user.getDriver().findElement(By.id("recording-mode-select")).click(); @@ -1332,21 +1337,76 @@ public class OpenViduTestAppE2eTest { Thread.sleep(500); user.getDriver().findElement(By.id("save-btn")).click(); Thread.sleep(1000); - + user.getDriver().findElement(By.className("join-btn")).click(); user.getEventManager().waitUntilEventReaches("connectionCreated", 5); user.getEventManager().waitUntilEventReaches("accessAllowed", 2); user.getEventManager().waitUntilEventReaches("streamCreated", 3); user.getEventManager().waitUntilEventReaches("streamPlaying", 3); user.getEventManager().waitUntilEventReaches("recordingStarted", 3); - CustomWebhook.waitForEvent("recordingStatusChanged", 1); + + event = CustomWebhook.waitForEvent("recordingStatusChanged", 1); // started + Assert.assertEquals("Wrong status in recordingStatusChanged event", "started", + event.get("status").getAsString()); + checkDockerContainerRunning("openvidu/openvidu-recording", 1); - + OV.fetch(); session = OV.getActiveSessions().get(0); session.close(); - + checkDockerContainerRunning("openvidu/openvidu-recording", 0); + + Assert.assertEquals("Wrong recording status", Recording.Status.ready, + OV.getRecording(sessionName + "-1").getStatus()); + + // 3. Session closed before recording started should trigger + CustomWebhook.clean(); + user.getDriver().findElement(By.id("remove-all-users-btn")).click(); + user.getDriver().findElement(By.id("add-user-btn")).click(); + user.getDriver().findElement(By.id("session-name-input-0")).clear(); + user.getDriver().findElement(By.id("session-name-input-0")).sendKeys(sessionName); + + user.getDriver().findElement(By.id("session-settings-btn-0")).click(); + Thread.sleep(1000); + user.getDriver().findElement(By.id("recording-mode-select")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("option-ALWAYS")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("output-mode-select")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("option-COMPOSED_QUICK_START")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("save-btn")).click(); + Thread.sleep(1000); + + user.getDriver().findElement(By.className("join-btn")).click(); + user.getEventManager().waitUntilEventReaches("connectionCreated", 6); + user.getEventManager().waitUntilEventReaches("accessAllowed", 3); + user.getEventManager().waitUntilEventReaches("streamCreated", 4); + user.getEventManager().waitUntilEventReaches("streamPlaying", 4); + checkDockerContainerRunning("openvidu/openvidu-recording", 1); + + OV.fetch(); + session = OV.getActiveSessions().get(0); + session.close(); + + // Recording hasn't had time to start. Should trigger stopped, started, failed + event = CustomWebhook.waitForEvent("recordingStatusChanged", 1); // stopped + Assert.assertEquals("Wrong status in recordingStatusChanged event", "stopped", + event.get("status").getAsString()); + event = CustomWebhook.waitForEvent("recordingStatusChanged", 1); // started + Assert.assertEquals("Wrong status in recordingStatusChanged event", "started", + event.get("status").getAsString()); + event = CustomWebhook.waitForEvent("recordingStatusChanged", 1); // failed + Assert.assertEquals("Wrong status in recordingStatusChanged event", "failed", + event.get("status").getAsString()); + + checkDockerContainerRunning("openvidu/openvidu-recording", 0); + + Assert.assertEquals("Wrong recording status", Recording.Status.failed, + OV.getRecording(sessionName + "-2").getStatus()); + } finally { CustomWebhook.shutDown(); } @@ -2381,9 +2441,10 @@ public class OpenViduTestAppE2eTest { .recordingLayout(RecordingLayout.BEST_FIT).resolution("1280x720").hasVideo(true).hasAudio(false) .name(customRecordingName).build(); + // Start recording method should block until video exists and size > 0 Recording recording2 = OV.startRecording(session.getSessionId(), recordingProperties); recording2 = OV.stopRecording(recording2.getId()); - Assert.assertEquals("Wrong recording status", Recording.Status.failed, recording2.getStatus()); + Assert.assertEquals("Wrong recording status", Recording.Status.ready, recording2.getStatus()); OV.deleteRecording(recording2.getId()); recording2 = OV.startRecording(session.getSessionId(), recordingProperties); From 90299411bb4f638d24df51d1ccb209b5b5e757d0 Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Sat, 26 Sep 2020 18:43:49 +0200 Subject: [PATCH 8/9] openvidu-test-e2e: remoteComposedQuickStartRecordTest fix --- .../java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java b/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java index b787e0ce..64d54b6e 100644 --- a/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java +++ b/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java @@ -1299,7 +1299,7 @@ public class OpenViduTestAppE2eTest { OV.fetch(); String recId = OV.startRecording(sessionName).getId(); user.getEventManager().waitUntilEventReaches("recordingStarted", 2); - CustomWebhook.waitForEvent("recordingStatusChanged", 1); + CustomWebhook.waitForEvent("recordingStatusChanged", 3); checkDockerContainerRunning("openvidu/openvidu-recording", 1); Thread.sleep(2000); @@ -1345,7 +1345,7 @@ public class OpenViduTestAppE2eTest { user.getEventManager().waitUntilEventReaches("streamPlaying", 3); user.getEventManager().waitUntilEventReaches("recordingStarted", 3); - event = CustomWebhook.waitForEvent("recordingStatusChanged", 1); // started + event = CustomWebhook.waitForEvent("recordingStatusChanged", 3); // started Assert.assertEquals("Wrong status in recordingStatusChanged event", "started", event.get("status").getAsString()); @@ -1395,7 +1395,7 @@ public class OpenViduTestAppE2eTest { event = CustomWebhook.waitForEvent("recordingStatusChanged", 1); // stopped Assert.assertEquals("Wrong status in recordingStatusChanged event", "stopped", event.get("status").getAsString()); - event = CustomWebhook.waitForEvent("recordingStatusChanged", 1); // started + event = CustomWebhook.waitForEvent("recordingStatusChanged", 3); // started Assert.assertEquals("Wrong status in recordingStatusChanged event", "started", event.get("status").getAsString()); event = CustomWebhook.waitForEvent("recordingStatusChanged", 1); // failed From ed7aaec1d5fc735ecbf36e1ffdd84dfbcb7df620 Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Sat, 26 Sep 2020 18:50:00 +0200 Subject: [PATCH 9/9] openvidu-test-e2e: longer recording started event timeouts --- .../java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java b/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java index 64d54b6e..f8de5a25 100644 --- a/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java +++ b/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java @@ -1299,7 +1299,7 @@ public class OpenViduTestAppE2eTest { OV.fetch(); String recId = OV.startRecording(sessionName).getId(); user.getEventManager().waitUntilEventReaches("recordingStarted", 2); - CustomWebhook.waitForEvent("recordingStatusChanged", 3); + CustomWebhook.waitForEvent("recordingStatusChanged", 5); checkDockerContainerRunning("openvidu/openvidu-recording", 1); Thread.sleep(2000); @@ -1345,7 +1345,7 @@ public class OpenViduTestAppE2eTest { user.getEventManager().waitUntilEventReaches("streamPlaying", 3); user.getEventManager().waitUntilEventReaches("recordingStarted", 3); - event = CustomWebhook.waitForEvent("recordingStatusChanged", 3); // started + event = CustomWebhook.waitForEvent("recordingStatusChanged", 5); // started Assert.assertEquals("Wrong status in recordingStatusChanged event", "started", event.get("status").getAsString()); @@ -1395,7 +1395,7 @@ public class OpenViduTestAppE2eTest { event = CustomWebhook.waitForEvent("recordingStatusChanged", 1); // stopped Assert.assertEquals("Wrong status in recordingStatusChanged event", "stopped", event.get("status").getAsString()); - event = CustomWebhook.waitForEvent("recordingStatusChanged", 3); // started + event = CustomWebhook.waitForEvent("recordingStatusChanged", 5); // started Assert.assertEquals("Wrong status in recordingStatusChanged event", "started", event.get("status").getAsString()); event = CustomWebhook.waitForEvent("recordingStatusChanged", 1); // failed