fix: stamp custom media pushed without a timestamp on WebRTC's clock

docs/guide/audio/custom-audio-source.md

@@ -104,7 +104,7 @@ audioSource.pushAudio(pcm, 16, sampleRate, channels, frameCount,
         baseUs + presentationUs);
 ```
 
-Use the same base for the audio and the video of one source, and the two stay in sync on the receiving side. See [Custom Video Source](/guide/video/custom-video-source) for the video half. Chunks must still be pushed in real time; the timestamp describes the audio, it does not schedule it.
+Use the same base for the audio and the video of one source, and the two stay in sync on the receiving side. See [Custom Video Source](/guide/video/custom-video-source) for the video half. Chunks must still be pushed in real time; the timestamp describes the audio, it does not schedule it. A chunk pushed without a timestamp is stamped with the same clock, so a source may mix the two calls.
 
 ## Audio Format Considerations
 

docs/guide/video/custom-video-source.md

@@ -169,6 +169,8 @@ Frames must still be pushed in real time. A frame is encoded and sent when it ar
 Timestamps must also advance by at least one millisecond from frame to frame. WebRTC drops a frame whose capture time does not move forward.
 :::
 
+`pushFrame(VideoFrame)` stamps its frames with the same clock, so a source may mix the two calls.
+
 Push audio with timestamps from the same clock, as described in [Custom Audio Source](/guide/audio/custom-audio-source), and the two stay in sync on the receiving side.
 
 ## Integration with Video Tracks

webrtc-jni/src/main/cpp/include/media/audio/CustomAudioSource.h

@@ -43,8 +43,9 @@ namespace jni
             bool remote() const override;
 
             // Push audio data with synchronization. The chunk is captured now,
-            // so it is stamped with this source's clock less the configured
-            // capture delay. This is what the Java pushAudio() calls.
+            // so it is stamped with webrtc::TimeMicros(), the clock of the
+            // overload below, less the configured capture delay. This is what
+            // the Java pushAudio() calls.
             void PushAudioData(const void * audio_data, int bits_per_sample,
                               int sample_rate, size_t number_of_channels,
                               size_t number_of_frames);

webrtc-jni/src/main/cpp/include/media/video/CustomVideoSource.h

@@ -40,10 +40,10 @@ namespace jni
             SourceState state() const override;
             bool remote() const override;
 
-            // Delivers a frame captured now, stamping it with this source's
-            // clock. This is what the Java pushFrame() calls, so a caller that
-            // has no capture time of its own gets the timing of the moment it
-            // pushes.
+            // Delivers a frame captured now, stamping it with
+            // webrtc::TimeMicros(), the clock of the overload below. This is
+            // what the Java pushFrame() calls, so a caller that has no capture
+            // time of its own gets the timing of the moment it pushes.
             void PushFrame(const webrtc::VideoFrame & frame);
 
             // Delivers a frame captured at the given time, in microseconds on

webrtc-jni/src/main/cpp/include/webrtc_java_api.h

@@ -117,8 +117,9 @@ struct wj_i420_frame {
 
 	/**
 	 * Capture time in the clock of now_us(). Frames must be pushed in
-	 * increasing timestamp order. A value of zero means "stamp it with the
-	 * source's own clock", which is what the Java pushFrame() does.
+	 * increasing timestamp order. A value of zero means "stamp it with
+	 * now_us() at the moment of the push", which is what the Java
+	 * pushFrame() does.
 	 */
 	int64_t timestamp_us;
 
@@ -147,7 +148,8 @@ struct wj_audio_chunk {
 
 	/**
 	 * Capture time in the clock of now_us(), or zero to stamp the chunk with
-	 * the source's own clock, which is what the Java pushAudio() does.
+	 * now_us() at the moment of the push, which is what the Java pushAudio()
+	 * does.
 	 */
 	int64_t timestamp_us;
 };

webrtc-jni/src/main/cpp/src/media/audio/CustomAudioSource.cpp

@@ -54,8 +54,12 @@ namespace jni
     {
         webrtc::MutexLock lock(&mutex_);
 
-        // Calculate absolute capture time
-        int64_t timestamp_us = clock_->GetTimestampUs();
+        // Captured now, on the clock AudioTrackSinkInterface requires of an
+        // absolute capture timestamp and the one a caller's capture time is
+        // on. WebRTC ignores a capture time that is not positive, and turns
+        // the gap between two of them into an RTP timestamp step when
+        // sending resumes, so both calls must stay on this clock.
+        int64_t timestamp_us = webrtc::TimeMicros();
 
         // Apply delay if audio capture has inherent latency
         timestamp_us -= audio_capture_delay_us_;

webrtc-jni/src/main/cpp/src/media/video/CustomVideoSource.cpp

@@ -48,8 +48,12 @@ namespace jni
 
     void CustomVideoSource::PushFrame(const webrtc::VideoFrame& frame)
     {
-        // No capture time from the caller, so the frame is captured now.
-        DeliverFrame(frame, clock_->GetTimestampUs(), clock_->GetNtpTime().ToMs());
+        // No capture time from the caller, so the frame is captured now, on
+        // the same clock as a caller's capture time. A source that mixes both
+        // calls would otherwise hand out frame times that jump between two
+        // clocks, to the frame rate adapter, the encoder's cadence adapter and
+        // every sink of the track, which all expect them to increase.
+        PushFrame(frame, webrtc::TimeMicros());
     }
 
     void CustomVideoSource::PushFrame(const webrtc::VideoFrame& frame, int64_t timestamp_us)

webrtc/src/main/java/dev/onvoid/webrtc/media/SyncClock.java

@@ -53,7 +53,8 @@ public SyncClock() {
      * example by {@link dev.onvoid.webrtc.media.video.CustomVideoSource#pushFrame(
      * dev.onvoid.webrtc.media.video.VideoFrame, long) pushFrame} and
      * {@link dev.onvoid.webrtc.media.audio.CustomAudioSource#pushAudio(byte[],
-     * int, int, int, int, long) pushAudio}. It is the monotonic clock WebRTC
+     * int, int, int, int, long) pushAudio}, and the one those sources stamp
+     * media pushed without a timestamp with. It is the monotonic clock WebRTC
      * itself runs on, which is what makes those timestamps comparable with
      * WebRTC's own notion of now.
      * <p>

webrtc/src/main/java/dev/onvoid/webrtc/media/audio/CustomAudioSource.java

@@ -75,6 +75,11 @@ public CustomAudioSource(SyncClock clock) {
 	 * 10 ms is the size WebRTC works with; at most
 	 * {@value #MAX_SAMPLES_PER_PUSH} samples, counting every channel, fit in
 	 * one chunk.
+	 * <p>
+	 * The chunk is treated as captured at the moment of the call, and stamped
+	 * with {@link dev.onvoid.webrtc.media.SyncClock#currentTimeUs()}, the clock
+	 * of {@link #pushAudio(byte[], int, int, int, int, long)}, so the two may
+	 * be mixed on one source.
 	 *
 	 * @param audioData       The raw audio data bytes to process. Must hold at
 	 *                        least {@code frameCount * channels * 2} bytes.

webrtc/src/main/java/dev/onvoid/webrtc/media/video/CustomVideoSource.java

@@ -48,7 +48,10 @@ public CustomVideoSource(SyncClock clock) {
 
 	/**
 	 * Pushes a video frame to be processed by this video source. The frame is
-	 * treated as captured at the moment of the call.
+	 * treated as captured at the moment of the call, and stamped with
+	 * {@link dev.onvoid.webrtc.media.SyncClock#currentTimeUs()}, the clock of
+	 * {@link #pushFrame(VideoFrame, long)}, so the two may be mixed on one
+	 * source.
 	 *
 	 * @param frame The video frame to be pushed to the source.
 	 */

webrtc/src/test/java/dev/onvoid/webrtc/media/video/CustomVideoSourceTest.java

@@ -138,9 +138,9 @@ void pushFrameWithTimestamp() {
     }
 
     @Test
-    void pushFrameWithoutTimestampUsesSourceClock() {
-        // Without a capture time the source stamps the frame itself, which is
-        // the behaviour of pushFrame(VideoFrame) and must stay that way.
+    void pushFrameWithoutTimestampUsesMediaClock() {
+        // Without a capture time the source stamps the frame itself, with the
+        // moment of the push on the clock a caller's capture time is on.
         VideoTrack videoTrack = factory.createVideoTrack("videoTrack", customVideoSource);
 
         final List<Long> timestamps = new ArrayList<>();
@@ -152,17 +152,74 @@ void pushFrameWithoutTimestampUsesSourceClock() {
         NativeI420Buffer buffer = NativeI420Buffer.allocate(320, 240);
         VideoFrame frame = new VideoFrame(buffer, 424242L);
 
+        long beforeUs = SyncClock.currentTimeUs();
         customVideoSource.pushFrame(frame);
+        long afterUs = SyncClock.currentTimeUs();
         frame.release();
 
         assertEquals(1, timestamps.size());
         assertNotEquals(424242L, timestamps.get(0),
                 "Frame kept the timestamp it was constructed with");
+        assertPushedBetween(beforeUs, afterUs, timestamps.get(0));
 
         videoTrack.removeSink(sink);
         videoTrack.dispose();
     }
 
+    @Test
+    void pushFrameWithAndWithoutTimestampShareOneClock() {
+        // A source may push some frames with a capture time and others
+        // without. Both must be on one clock, or the frame time jumps back and
+        // forth between them, which the adapter and the encoder take for time
+        // running backwards.
+        SyncClock clock = new SyncClock();
+        CustomVideoSource source = new CustomVideoSource(clock);
+        VideoTrack videoTrack = factory.createVideoTrack("videoTrack", source);
+
+        final List<Long> timestamps = new ArrayList<>();
+
+        VideoTrackSink sink = frame -> timestamps.add(frame.timestampNs);
+
+        videoTrack.addSink(sink);
+
+        long[] beforeUs = new long[3];
+        long[] afterUs = new long[3];
+
+        for (int i = 0; i < 3; i++) {
+            NativeI420Buffer buffer = NativeI420Buffer.allocate(320, 240);
+            VideoFrame frame = new VideoFrame(buffer, 0);
+
+            beforeUs[i] = SyncClock.currentTimeUs();
+
+            if (i == 1) {
+                source.pushFrame(frame);
+            }
+            else {
+                source.pushFrame(frame, beforeUs[i]);
+            }
+
+            afterUs[i] = SyncClock.currentTimeUs();
+
+            frame.release();
+        }
+
+        assertEquals(3, timestamps.size(), "Not every frame reached the sink");
+
+        for (int i = 0; i < 3; i++) {
+            assertPushedBetween(beforeUs[i], afterUs[i], timestamps.get(i));
+        }
+        for (int i = 1; i < 3; i++) {
+            assertTrue(timestamps.get(i) >= timestamps.get(i - 1),
+                    "Frame time went back from " + timestamps.get(i - 1)
+                            + " ns to " + timestamps.get(i) + " ns");
+        }
+
+        videoTrack.removeSink(sink);
+        videoTrack.dispose();
+        source.dispose();
+        clock.dispose();
+    }
+
     @Test
     void constructWithSyncClock() {
         // Create a SyncClock.
@@ -214,6 +271,14 @@ public void onVideoFrame(VideoFrame frame) {
         clock.dispose();
     }
 
+    private static void assertPushedBetween(long beforeUs, long afterUs, long timestampNs) {
+        long timestampUs = timestampNs / 1000;
+
+        assertTrue(timestampUs >= beforeUs && timestampUs <= afterUs,
+                "Frame time " + timestampUs + " µs is not between " + beforeUs
+                        + " µs and " + afterUs + " µs on SyncClock.currentTimeUs()");
+    }
+
     private void testVideoFrame(int width, int height) {
         VideoTrack videoTrack = factory.createVideoTrack("videoTrack", customVideoSource);