ProducerOptions: Rename keyFrameWaitTime option to `keyFrameRequest…

…Delay` and make it work as expected.
versatica · Feb 17, 2020 · 43c5d12 · 43c5d12
1 parent 89b478f
commit 43c5d12
Show file tree

Hide file tree

Showing 12 changed files with 190 additions and 69 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,11 @@
 # Changelog
 
 
+### 3.4.10 (WIP)
+
+* `ProducerOptions`: Rename `keyFrameWaitTime` option to `keyFrameRequestDelay` and make it work as expected.
+
+
 ### 3.4.9
 
 * Add `Utils::Json::IsPositiveInteger()` to not rely on `is_number_unsigned()` of json lib, which is unreliable due to its design.

diff --git a/lib/Producer.d.ts b/lib/Producer.d.ts
@@ -20,10 +20,9 @@ export interface ProducerOptions {
     paused?: boolean;
     /**
      * Just for video. Time (in ms) before asking the sender for a new key frame
-     * after having asked a previous one. Default 1000. Minimum valid value is
-     * 500.
+     * after having asked a previous one. Default 0.
      */
-    keyFrameWaitTime?: number;
+    keyFrameRequestDelay?: number;
     /**
      * Custom application data.
      */

diff --git a/lib/Producer.d.ts.map b/lib/Producer.d.ts.map
diff --git a/lib/Transport.d.ts b/lib/Transport.d.ts
@@ -149,7 +149,7 @@ export declare class Transport extends EnhancedEventEmitter {
     /**
      * Create a Producer.
      */
-    produce({ id, kind, rtpParameters, paused, keyFrameWaitTime, appData }: ProducerOptions): Promise<Producer>;
+    produce({ id, kind, rtpParameters, paused, keyFrameRequestDelay, appData }: ProducerOptions): Promise<Producer>;
     /**
      * Create a Consumer.
      *

diff --git a/lib/Transport.d.ts.map b/lib/Transport.d.ts.map
diff --git a/lib/Transport.js b/lib/Transport.js
@@ -227,7 +227,7 @@ class Transport extends EnhancedEventEmitter_1.EnhancedEventEmitter {
     /**
      * Create a Producer.
      */
-    produce({ id = undefined, kind, rtpParameters, paused = false, keyFrameWaitTime, appData = {} }) {
+    produce({ id = undefined, kind, rtpParameters, paused = false, keyFrameRequestDelay, appData = {} }) {
         return __awaiter(this, void 0, void 0, function* () {
             logger.debug('produce()');
             if (id && this._producers.has(id))
@@ -267,7 +267,7 @@ class Transport extends EnhancedEventEmitter_1.EnhancedEventEmitter {
             // This may throw.
             const consumableRtpParameters = ortc.getConsumableRtpParameters(kind, rtpParameters, routerRtpCapabilities, rtpMapping);
             const internal = Object.assign(Object.assign({}, this._internal), { producerId: id || v4_1.default() });
-            const reqData = { kind, rtpParameters, rtpMapping, keyFrameWaitTime, paused };
+            const reqData = { kind, rtpParameters, rtpMapping, keyFrameRequestDelay, paused };
             const status = yield this._channel.request('transport.produce', internal, reqData);
             const data = {
                 kind,

diff --git a/src/Producer.ts b/src/Producer.ts
@@ -27,10 +27,9 @@ export interface ProducerOptions
 
 	/**
 	 * Just for video. Time (in ms) before asking the sender for a new key frame
-	 * after having asked a previous one. Default 1000. Minimum valid value is
-	 * 500.
+	 * after having asked a previous one. Default 0.
 	 */
-	keyFrameWaitTime?: number;
+	keyFrameRequestDelay?: number;
 
 	/**
 	 * Custom application data.

diff --git a/src/Transport.ts b/src/Transport.ts
@@ -387,7 +387,7 @@ export class Transport extends EnhancedEventEmitter
 			kind,
 			rtpParameters,
 			paused = false,
-			keyFrameWaitTime,
+			keyFrameRequestDelay,
 			appData = {}
 		}: ProducerOptions
 	): Promise<Producer>
@@ -447,7 +447,7 @@ export class Transport extends EnhancedEventEmitter
 			kind, rtpParameters, routerRtpCapabilities, rtpMapping);
 
 		const internal = { ...this._internal, producerId: id || uuidv4() };
-		const reqData = { kind, rtpParameters, rtpMapping, keyFrameWaitTime, paused };
+		const reqData = { kind, rtpParameters, rtpMapping, keyFrameRequestDelay, paused };
 
 		const status =
 			await this._channel.request('transport.produce', internal, reqData);

diff --git a/test/test-Producer.js b/test/test-Producer.js
@@ -199,8 +199,7 @@ test('transport2.produce() succeeds', async () =>
 					cname : 'video-1'
 				}
 			},
-			keyFrameWaitTime : 500,
-			appData          : { foo: 1, bar: '2' }
+			appData : { foo: 1, bar: '2' }
 		});
 
 	expect(onObserverNewProducer).toHaveBeenCalledTimes(1);
@@ -338,33 +337,6 @@ test('transport1.produce() with wrong arguments rejects with TypeError', async (
 		}))
 		.rejects
 		.toThrow(TypeError);
-
-	// Wrong keyFrameWaitTime (less than 500).
-	await expect(transport1.produce(
-		{
-			kind          : 'video',
-			rtpParameters :
-			{
-				codecs :
-				[
-					{
-						mimeType    : 'video/h264',
-						payloadType : 112,
-						clockRate   : 90000,
-						parameters  :
-						{
-							'packetization-mode' : 1,
-							'profile-level-id'   : '4d0032'
-						}
-					}
-				],
-				encodings : [ { ssrc: 6666 } ],
-				rtcp      : { cname: 'qwerty' }
-			},
-			keyFrameWaitTime : 499
-		}))
-		.rejects
-		.toThrow(TypeError);
 }, 2000);
 
 test('transport1.produce() with unsupported codecs rejects with UnsupportedError', async () =>

diff --git a/worker/include/RTC/KeyFrameRequestManager.hpp b/worker/include/RTC/KeyFrameRequestManager.hpp
@@ -16,7 +16,7 @@ namespace RTC
 		};
 
 	public:
-		PendingKeyFrameInfo(Listener* listener, uint32_t ssrc, uint32_t keyFrameWaitTime);
+		PendingKeyFrameInfo(Listener* listener, uint32_t ssrc);
 		~PendingKeyFrameInfo();
 
 		uint32_t GetSsrc() const;
@@ -35,7 +35,36 @@ namespace RTC
 		bool retryOnTimeout{ true };
 	};
 
-	class KeyFrameRequestManager : public PendingKeyFrameInfo::Listener
+	class KeyFrameRequestDelayer : public Timer::Listener
+	{
+	public:
+		class Listener
+		{
+		public:
+			virtual void OnKeyFrameDelayTimeout(KeyFrameRequestDelayer* keyFrameRequestDelayer) = 0;
+		};
+
+	public:
+		KeyFrameRequestDelayer(Listener* listener, uint32_t ssrc, uint32_t delay);
+		~KeyFrameRequestDelayer();
+
+		uint32_t GetSsrc() const;
+		bool GetKeyFrameRequested() const;
+		void SetKeyFrameRequested(bool flag);
+
+		/* Pure virtual methods inherited from Timer::Listener. */
+	public:
+		void OnTimer(Timer* timer) override;
+
+	private:
+		Listener* listener{ nullptr };
+		uint32_t ssrc;
+		Timer* timer{ nullptr };
+		bool keyFrameRequested{ false };
+	};
+
+	class KeyFrameRequestManager : public PendingKeyFrameInfo::Listener,
+	                               public KeyFrameRequestDelayer::Listener
 	{
 	public:
 		class Listener
@@ -45,7 +74,7 @@ namespace RTC
 		};
 
 	public:
-		explicit KeyFrameRequestManager(Listener* listener, uint32_t keyFrameWaitTime);
+		explicit KeyFrameRequestManager(Listener* listener, uint32_t keyFrameRequestDelay);
 		virtual ~KeyFrameRequestManager();
 
 		void KeyFrameNeeded(uint32_t ssrc);
@@ -56,10 +85,15 @@ namespace RTC
 	public:
 		void OnKeyFrameRequestTimeout(PendingKeyFrameInfo* pendingKeyFrameInfo) override;
 
+		/* Pure virtual methods inherited from PendingKeyFrameInfo::Listener. */
+	public:
+		void OnKeyFrameDelayTimeout(KeyFrameRequestDelayer* keyFrameRequestDelayer) override;
+
 	private:
 		Listener* listener{ nullptr };
-		uint32_t keyFrameWaitTime;
+		uint32_t keyFrameRequestDelay{ 0u }; // 0 means disabled.
 		std::map<uint32_t, PendingKeyFrameInfo*> mapSsrcPendingKeyFrameInfo;
+		std::map<uint32_t, KeyFrameRequestDelayer*> mapSsrcKeyFrameRequestDelayer;
 	};
 } // namespace RTC
 
@@ -83,4 +117,19 @@ inline void RTC::PendingKeyFrameInfo::Restart()
 	return this->timer->Restart();
 }
 
+inline uint32_t RTC::KeyFrameRequestDelayer::GetSsrc() const
+{
+	return this->ssrc;
+}
+
+inline bool RTC::KeyFrameRequestDelayer::GetKeyFrameRequested() const
+{
+	return this->keyFrameRequested;
+}
+
+inline void RTC::KeyFrameRequestDelayer::SetKeyFrameRequested(bool flag)
+{
+	this->keyFrameRequested = flag;
+}
+
 #endif