From ac5700cd9555088a4fbe3392415c7285715a6876 Mon Sep 17 00:00:00 2001 From: Juan Navarro Date: Thu, 9 Jun 2022 15:29:23 +0200 Subject: [PATCH] Remove improper usages of SessionProperties.forcedVideoCodecResolved (#737) forcedVideoCodecResolved is a property that gets automatically assigned by the server and only used by it; clients don't need to know about its existence and don't need to use it. Similarly, SessionProperties itself should not serialize this field. --- .../main/java/io/openvidu/java/client/Session.java | 7 +------ .../io/openvidu/java/client/SessionProperties.java | 10 +++++++--- openvidu-node-client/src/Session.ts | 6 ------ openvidu-node-client/src/SessionProperties.ts | 11 ----------- .../java/io/openvidu/test/e2e/OpenViduTestE2e.java | 2 +- .../io/openvidu/test/e2e/OpenViduTestAppE2eTest.java | 6 +++--- 6 files changed, 12 insertions(+), 30 deletions(-) diff --git a/openvidu-java-client/src/main/java/io/openvidu/java/client/Session.java b/openvidu-java-client/src/main/java/io/openvidu/java/client/Session.java index bbf8085c..eb35b11d 100644 --- a/openvidu-java-client/src/main/java/io/openvidu/java/client/Session.java +++ b/openvidu-java-client/src/main/java/io/openvidu/java/client/Session.java @@ -665,15 +665,13 @@ public class Session { // Values that get filled by OpenVidu Server from its global or per-session // configuration VideoCodec forcedVideoCodec = VideoCodec.valueOf(responseJson.get("forcedVideoCodec").getAsString()); - VideoCodec forcedVideoCodecResolved = VideoCodec - .valueOf(responseJson.get("forcedVideoCodecResolved").getAsString()); Boolean allowTranscoding = responseJson.get("allowTranscoding").getAsBoolean(); SessionProperties responseProperties = new SessionProperties.Builder().mediaMode(properties.mediaMode()) .recordingMode(properties.recordingMode()) .defaultRecordingProperties(properties.defaultRecordingProperties()) .customSessionId(properties.customSessionId()).mediaNode(properties.mediaNode()) - .forcedVideoCodec(forcedVideoCodec).forcedVideoCodecResolved(forcedVideoCodecResolved) + .forcedVideoCodec(forcedVideoCodec) .allowTranscoding(allowTranscoding).build(); this.properties = responseProperties; @@ -727,9 +725,6 @@ public class Session { if (json.has("forcedVideoCodec")) { builder.forcedVideoCodec(VideoCodec.valueOf(json.get("forcedVideoCodec").getAsString())); } - if (json.has("forcedVideoCodecResolved")) { - builder.forcedVideoCodecResolved(VideoCodec.valueOf(json.get("forcedVideoCodecResolved").getAsString())); - } if (json.has("allowTranscoding")) { builder.allowTranscoding(json.get("allowTranscoding").getAsBoolean()); } diff --git a/openvidu-java-client/src/main/java/io/openvidu/java/client/SessionProperties.java b/openvidu-java-client/src/main/java/io/openvidu/java/client/SessionProperties.java index b57677d1..9321925f 100644 --- a/openvidu-java-client/src/main/java/io/openvidu/java/client/SessionProperties.java +++ b/openvidu-java-client/src/main/java/io/openvidu/java/client/SessionProperties.java @@ -249,6 +249,13 @@ public class SessionProperties { * Defines which video codec is being forced to be used in the browser/client. * This is the resolved value, for actual usage in the server. * + * This is a server-only property, and as such, it doesn't need to be transmitted + * over the wire between server and client. Thus it doesn't get serialized in + * the `toJson()` method. + * + * If more server-only properties start to appear here, maybe a good idea + * would be to refactor them all into a server-specific Properties class. + * * @hidden */ public VideoCodec forcedVideoCodecResolved() { @@ -277,9 +284,6 @@ public class SessionProperties { if (this.forcedVideoCodec != null) { json.addProperty("forcedVideoCodec", this.forcedVideoCodec.name()); } - if (this.forcedVideoCodecResolved != null) { - json.addProperty("forcedVideoCodecResolved", this.forcedVideoCodecResolved.name()); - } if (this.allowTranscoding != null) { json.addProperty("allowTranscoding", this.allowTranscoding); } diff --git a/openvidu-node-client/src/Session.ts b/openvidu-node-client/src/Session.ts index f70f127e..e840cc3f 100644 --- a/openvidu-node-client/src/Session.ts +++ b/openvidu-node-client/src/Session.ts @@ -491,7 +491,6 @@ export class Session { this.properties.defaultRecordingProperties = res.data.defaultRecordingProperties; this.properties.mediaNode = res.data.mediaNode; this.properties.forcedVideoCodec = res.data.forcedVideoCodec; - this.properties.forcedVideoCodecResolved = res.data.forcedVideoCodecResolved; this.properties.allowTranscoding = res.data.allowTranscoding; this.sanitizeDefaultSessionProperties(this.properties); resolve(this.sessionId); @@ -524,7 +523,6 @@ export class Session { recordingMode: json.recordingMode, defaultRecordingProperties: json.defaultRecordingProperties, forcedVideoCodec: json.forcedVideoCodec, - forcedVideoCodecResolved: json.forcedVideoCodecResolved, allowTranscoding: json.allowTranscoding }; this.sanitizeDefaultSessionProperties(this.properties); @@ -540,9 +538,6 @@ export class Session { if (json.forcedVideoCodec == null) { delete this.properties.forcedVideoCodec; } - if (json.forcedVideoCodecResolved == null) { - delete this.properties.forcedVideoCodecResolved; - } if (json.allowTranscoding == null) { delete this.properties.allowTranscoding; } @@ -645,7 +640,6 @@ export class Session { // Remove null values: either set, or undefined props.mediaNode = props.mediaNode ?? undefined; props.forcedVideoCodec = props.forcedVideoCodec ?? undefined; - props.forcedVideoCodecResolved = props.forcedVideoCodecResolved ?? undefined; props.allowTranscoding = props.allowTranscoding ?? undefined; if (!props.defaultRecordingProperties) { diff --git a/openvidu-node-client/src/SessionProperties.ts b/openvidu-node-client/src/SessionProperties.ts index c94239e2..1708d0a0 100644 --- a/openvidu-node-client/src/SessionProperties.ts +++ b/openvidu-node-client/src/SessionProperties.ts @@ -82,17 +82,6 @@ export interface SessionProperties { */ forcedVideoCodec?: VideoCodec; - /** - * Actual video codec that will be forcibly used for this session. - * This is the same as forcedVideoCodec, except when its value - * is [[VideoCodec.MEDIA_SERVER_PREFERRED]]: in that case, OpenVidu Server - * will fill this property with a resolved value, depending on what is the - * configured media server. - * - * @hidden - */ - forcedVideoCodecResolved?: VideoCodec; - /** * It defines if you want to allow transcoding in the media server or not * when [[forcedVideoCodec]] is not compatible with the browser/client. diff --git a/openvidu-test-e2e/src/main/java/io/openvidu/test/e2e/OpenViduTestE2e.java b/openvidu-test-e2e/src/main/java/io/openvidu/test/e2e/OpenViduTestE2e.java index c7356c6f..702494bc 100644 --- a/openvidu-test-e2e/src/main/java/io/openvidu/test/e2e/OpenViduTestE2e.java +++ b/openvidu-test-e2e/src/main/java/io/openvidu/test/e2e/OpenViduTestE2e.java @@ -67,7 +67,7 @@ public class OpenViduTestE2e { final protected static String MEDIASOUP_IMAGE = "openvidu/mediasoup-controller"; protected static String MEDIA_SERVER_IMAGE = KURENTO_IMAGE + ":6.16.0"; - final protected String DEFAULT_JSON_SESSION = "{'id':'STR','object':'session','sessionId':'STR','createdAt':0,'mediaMode':'STR','recordingMode':'STR','defaultRecordingProperties':{'hasVideo':true,'frameRate':25,'hasAudio':true,'shmSize':536870912,'name':'','outputMode':'COMPOSED','resolution':'1280x720','recordingLayout':'BEST_FIT'},'customSessionId':'STR','connections':{'numberOfElements':0,'content':[]},'recording':false,'forcedVideoCodec':'STR','forcedVideoCodecResolved':'STR','allowTranscoding':false}"; + final protected String DEFAULT_JSON_SESSION = "{'id':'STR','object':'session','sessionId':'STR','createdAt':0,'mediaMode':'STR','recordingMode':'STR','defaultRecordingProperties':{'hasVideo':true,'frameRate':25,'hasAudio':true,'shmSize':536870912,'name':'','outputMode':'COMPOSED','resolution':'1280x720','recordingLayout':'BEST_FIT'},'customSessionId':'STR','connections':{'numberOfElements':0,'content':[]},'recording':false,'forcedVideoCodec':'STR','allowTranscoding':false}"; final protected String DEFAULT_JSON_PENDING_CONNECTION = "{'id':'STR','object':'connection','type':'WEBRTC','status':'pending','connectionId':'STR','sessionId':'STR','createdAt':0,'activeAt':null,'location':null,'ip':null,'platform':null,'token':'STR','serverData':'STR','record':true,'role':'STR','kurentoOptions':null,'rtspUri':null,'adaptativeBitrate':null,'onlyPlayWithSubscribers':null,'networkCache':null,'clientData':null,'publishers':null,'subscribers':null, 'customIceServers':[]}"; final protected String DEFAULT_JSON_ACTIVE_CONNECTION = "{'id':'STR','object':'connection','type':'WEBRTC','status':'active','connectionId':'STR','sessionId':'STR','createdAt':0,'activeAt':0,'location':'STR','ip':'STR','platform':'STR','token':'STR','serverData':'STR','record':true,'role':'STR','kurentoOptions':null,'rtspUri':null,'adaptativeBitrate':null,'onlyPlayWithSubscribers':null,'networkCache':null,'clientData':'STR','publishers':[],'subscribers':[], 'customIceServers':[]}"; final protected String DEFAULT_JSON_IPCAM_CONNECTION = "{'id':'STR','object':'connection','type':'IPCAM','status':'active','connectionId':'STR','sessionId':'STR','createdAt':0,'activeAt':0,'location':'STR','ip':'STR','platform':'IPCAM','token':null,'serverData':'STR','record':true,'role':null,'kurentoOptions':null,'rtspUri':'STR','adaptativeBitrate':true,'onlyPlayWithSubscribers':true,'networkCache':2000,'clientData':null,'publishers':[],'subscribers':[], 'customIceServers':[]}"; 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 84e2fdf9..c893f7cb 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 @@ -3304,7 +3304,7 @@ public class OpenViduTestAppE2eTest extends AbstractOpenViduTestappE2eTest { + "'videoDimensions':'STR','filter':{}}}],'subscribers':[{'createdAt':0,'streamId':'STR','publisher':'STR'}]},{'connectionId':'STR','createdAt':0,'location':'STR','ip':'STR'," + "'platform':'STR','token':'STR','role':'STR','serverData':'STR','clientData':'STR','publishers':[{'createdAt':0,'streamId':'STR','mediaOptions':{'hasAudio':false," + "'audioActive':false,'hasVideo':false,'videoActive':false,'typeOfVideo':'STR','frameRate':0,'videoDimensions':'STR','filter':{}}}],'subscribers':[{'createdAt':0,'streamId':'STR','publisher':'STR'}]}]}," - + "'recording':false,'forcedVideoCodec':'STR','forcedVideoCodecResolved':'STR','allowTranscoding':false}"); + + "'recording':false,'forcedVideoCodec':'STR','allowTranscoding':false}"); String streamId = res.get("connections").getAsJsonObject().get("content").getAsJsonArray().get(0) .getAsJsonObject().get("publishers").getAsJsonArray().get(0).getAsJsonObject().get("streamId") .getAsString(); @@ -4901,7 +4901,7 @@ public class OpenViduTestAppE2eTest extends AbstractOpenViduTestappE2eTest { /** * Test to force specified codec and allowTranscoding - * + * * @param codec codec to force. If null, default value in openvidu * config will be used. * @param allowTranscoding If true, allow transcoding. If null, default value in @@ -5010,7 +5010,7 @@ public class OpenViduTestAppE2eTest extends AbstractOpenViduTestappE2eTest { /** * Force codec not allowed by opened browser - * + * * @throws Exception */ private void forceNotSupportedCodec(OpenViduTestappUser user, VideoCodec codec, boolean allowTranscoding)