Move the Vsync logic from VTRenderer into a VsyncSource

app/app.pro

@@ -114,12 +114,12 @@ ffmpeg {
     SOURCES += \
         streaming/video/ffmpeg.cpp \
         streaming/video/ffmpeg-renderers/sdl.cpp \
-        streaming/video/ffmpeg-renderers/pacer.cpp
+        streaming/video/ffmpeg-renderers/pacer/pacer.cpp
 
     HEADERS += \
         streaming/video/ffmpeg.h \
         streaming/video/ffmpeg-renderers/renderer.h \
-        streaming/video/ffmpeg-renderers/pacer.h
+        streaming/video/ffmpeg-renderers/pacer/pacer.h
 }
 libva {
     message(VAAPI renderer selected)
@@ -165,8 +165,13 @@ win32 {
 macx {
     message(VideoToolbox renderer selected)
 
-    SOURCES += streaming/video/ffmpeg-renderers/vt.mm
+    SOURCES += \
-    HEADERS += streaming/video/ffmpeg-renderers/vt.h
+        streaming/video/ffmpeg-renderers/vt.mm \
+        streaming/video/ffmpeg-renderers/pacer/displaylinkvsyncsource.cpp
+
+    HEADERS += \
+        streaming/video/ffmpeg-renderers/vt.h \
+        streaming/video/ffmpeg-renderers/pacer/displaylinkvsyncsource.h
 }
 
 RESOURCES += \

app/streaming/video/ffmpeg-renderers/pacer.h

@@ -1,29 +0,0 @@
-#pragma once
-
-#include "renderer.h"
-
-#include <QQueue>
-
-class Pacer
-{
-public:
-    Pacer();
-
-    ~Pacer();
-
-    AVFrame* getFrameAtVsync();
-
-    void submitFrame(AVFrame* frame);
-
-    void initialize(SDL_Window* window, int maxVideoFps);
-
-    void drain();
-
-private:
-    QQueue<AVFrame*> m_FrameQueue;
-    QQueue<int> m_FrameQueueHistory;
-    SDL_SpinLock m_FrameQueueLock;
-
-    int m_MaxVideoFps;
-    int m_DisplayFps;
-};

app/streaming/video/ffmpeg-renderers/pacer/displaylinkvsyncsource.cpp

@@ -0,0 +1,48 @@
+#include "displaylinkvsyncsource.h"
+
+DisplayLinkVsyncSource::DisplayLinkVsyncSource(Pacer* pacer)
+    : m_Pacer(pacer),
+      m_DisplayLink(nullptr)
+{
+
+}
+
+DisplayLinkVsyncSource::~DisplayLinkVsyncSource()
+{
+    if (m_DisplayLink != nullptr) {
+        CVDisplayLinkStop(m_DisplayLink);
+        CVDisplayLinkRelease(m_DisplayLink);
+    }
+}
+
+CVReturn
+DisplayLinkVsyncSource::displayLinkOutputCallback(
+    CVDisplayLinkRef,
+    const CVTimeStamp* /* now */,
+    const CVTimeStamp* /* vsyncTime */,
+    CVOptionFlags,
+    CVOptionFlags*,
+    void *displayLinkContext)
+{
+    auto me = reinterpret_cast<DisplayLinkVsyncSource*>(displayLinkContext);
+
+    // In my testing on macOS 10.13, this callback is invoked about 24 ms
+    // prior to the specified v-sync time (now - vsyncTime). Since this is
+    // greater than the standard v-sync interval (16 ms = 60 FPS), we will
+    // draw using the current host time, rather than the actual v-sync target
+    // time. Because the CVDisplayLink is in sync with the actual v-sync
+    // interval, even if many ms prior, we can safely use the current host time
+    // and get a consistent callback for each v-sync. This reduces video latency
+    // by at least 1 frame vs. rendering with the actual vsyncTime.
+    me->m_Pacer->vsyncCallback();
+
+    return kCVReturnSuccess;
+}
+
+bool DisplayLinkVsyncSource::initialize(SDL_Window*)
+{
+    CVDisplayLinkCreateWithActiveCGDisplays(&m_DisplayLink);
+    CVDisplayLinkSetOutputCallback(m_DisplayLink, displayLinkOutputCallback, this);
+    CVDisplayLinkStart(m_DisplayLink);
+    return true;
+}

app/streaming/video/ffmpeg-renderers/pacer/displaylinkvsyncsource.h

@@ -0,0 +1,30 @@
+#pragma once
+
+#include <CoreVideo/CoreVideo.h>
+
+#include "pacer.h"
+
+class DisplayLinkVsyncSource : public IVsyncSource
+{
+public:
+    DisplayLinkVsyncSource(Pacer* pacer);
+
+    virtual ~DisplayLinkVsyncSource();
+
+    virtual bool initialize(SDL_Window* window);
+
+private:
+    static
+    CVReturn
+    displayLinkOutputCallback(
+        CVDisplayLinkRef,
+        const CVTimeStamp* now,
+        const CVTimeStamp* /* vsyncTime */,
+        CVOptionFlags,
+        CVOptionFlags*,
+        void *displayLinkContext);
+
+    Pacer* m_Pacer;
+    CVDisplayLinkRef m_DisplayLink;
+};
+

app/streaming/video/ffmpeg-renderers/pacer/pacer.cpp

@@ -1,9 +1,15 @@
 #include "pacer.h"
 
+#ifdef Q_OS_DARWIN
+#include "displaylinkvsyncsource.h"
+#endif
+
 #define FRAME_HISTORY_ENTRIES 8
 
-Pacer::Pacer() :
+Pacer::Pacer(IVsyncRenderer* renderer) :
     m_FrameQueueLock(0),
+    m_VsyncSource(nullptr),
+    m_VsyncRenderer(renderer),
     m_MaxVideoFps(0),
     m_DisplayFps(0)
 {
@@ -12,10 +18,70 @@ Pacer::Pacer() :
 
 Pacer::~Pacer()
 {
+    // Stop V-sync callbacks
+    delete m_VsyncSource;
+
     drain();
 }
 
-void Pacer::initialize(SDL_Window* window, int maxVideoFps)
+// Called in an arbitrary thread by the IVsyncSource on V-sync
+// or an event synchronized with V-sync
+void Pacer::vsyncCallback()
+{
+    // Make sure initialize() has been called
+    SDL_assert(m_MaxVideoFps != 0);
+
+    SDL_AtomicLock(&m_FrameQueueLock);
+
+    // If the queue length history entries are large, be strict
+    // about dropping excess frames.
+    int frameDropTarget = 1;
+
+    // If we may get more frames per second than we can display, use
+    // frame history to drop frames only if consistently above the
+    // one queued frame mark.
+    if (m_MaxVideoFps >= m_DisplayFps) {
+        for (int i = 0; i < m_FrameQueueHistory.count(); i++) {
+            if (m_FrameQueueHistory[i] <= 1) {
+                // Be lenient as long as the queue length
+                // resolves before the end of frame history
+                frameDropTarget = 3;
+                break;
+            }
+        }
+
+        if (m_FrameQueueHistory.count() == FRAME_HISTORY_ENTRIES) {
+            m_FrameQueueHistory.dequeue();
+        }
+
+        m_FrameQueueHistory.enqueue(m_FrameQueue.count());
+    }
+
+    // Catch up if we're several frames ahead
+    while (m_FrameQueue.count() > frameDropTarget) {
+        AVFrame* frame = m_FrameQueue.dequeue();
+        av_frame_free(&frame);
+    }
+
+    if (m_FrameQueue.isEmpty()) {
+        SDL_AtomicUnlock(&m_FrameQueueLock);
+
+        // Nothing to render at this time
+        return;
+    }
+
+    // Grab the first frame
+    AVFrame* frame = m_FrameQueue.dequeue();
+    SDL_AtomicUnlock(&m_FrameQueueLock);
+
+    // Render it
+    m_VsyncRenderer->renderFrameAtVsync(frame);
+
+    // Free the frame
+    av_frame_free(&frame);
+}
+
+bool Pacer::initialize(SDL_Window* window, int maxVideoFps)
 {
     m_MaxVideoFps = maxVideoFps;
 
@@ -38,54 +104,14 @@ void Pacer::initialize(SDL_Window* window, int maxVideoFps)
     SDL_LogInfo(SDL_LOG_CATEGORY_APPLICATION,
                 "Frame pacing: target %d Hz with %d FPS stream",
                 m_DisplayFps, m_MaxVideoFps);
-}
 
-AVFrame* Pacer::getFrameAtVsync()
+#ifdef Q_OS_DARWIN
-{
+    m_VsyncSource = new DisplayLinkVsyncSource(this);
-    // Make sure initialize() has been called
+#else
-    SDL_assert(m_MaxVideoFps != 0);
+    SDL_assert(false);
+#endif
 
-    SDL_AtomicLock(&m_FrameQueueLock);
+    return m_VsyncSource->initialize(window);
-
-    // If the queue length history entries are large, be strict
-    // about dropping excess frames.
-    int frameDropTarget = 1;
-
-    // If we may get more frames per second than we can display, use
-    // frame history to drop frames only if consistently above the
-    // one queued frame mark.
-    if (m_MaxVideoFps >= m_DisplayFps) {
-        for (int i = 0; i < m_FrameQueueHistory.count(); i++) {
-            if (m_FrameQueueHistory[i] <= 1) {
-                // Be lenient as long as the queue length
-                // resolves before the end of frame history
-                frameDropTarget = 3;
-            }
-        }
-
-        if (m_FrameQueueHistory.count() == FRAME_HISTORY_ENTRIES) {
-            m_FrameQueueHistory.dequeue();
-        }
-
-        m_FrameQueueHistory.enqueue(m_FrameQueue.count());
-    }
-
-    // Catch up if we're several frames ahead
-    while (m_FrameQueue.count() > frameDropTarget) {
-        AVFrame* frame = m_FrameQueue.dequeue();
-        av_frame_free(&frame);
-    }
-
-    if (m_FrameQueue.isEmpty()) {
-        SDL_AtomicUnlock(&m_FrameQueueLock);
-        return nullptr;
-    }
-
-    // Grab the first frame
-    AVFrame* frame = m_FrameQueue.dequeue();
-    SDL_AtomicUnlock(&m_FrameQueueLock);
-
-    return frame;
 }
 
 void Pacer::submitFrame(AVFrame* frame)

app/streaming/video/ffmpeg-renderers/pacer/pacer.h

@@ -0,0 +1,45 @@
+#pragma once
+
+#include "../renderer.h"
+
+#include <QQueue>
+
+class Pacer;
+
+class IVsyncRenderer {
+public:
+    virtual ~IVsyncRenderer() {}
+    virtual void renderFrameAtVsync(AVFrame* frame) = 0;
+};
+
+class IVsyncSource {
+public:
+    virtual ~IVsyncSource() {}
+    virtual bool initialize(SDL_Window* window) = 0;
+};
+
+class Pacer
+{
+public:
+    Pacer(IVsyncRenderer* renderer);
+
+    ~Pacer();
+
+    void submitFrame(AVFrame* frame);
+
+    bool initialize(SDL_Window* window, int maxVideoFps);
+
+    void vsyncCallback();
+
+    void drain();
+
+private:
+    QQueue<AVFrame*> m_FrameQueue;
+    QQueue<int> m_FrameQueueHistory;
+    SDL_SpinLock m_FrameQueueLock;
+
+    IVsyncSource* m_VsyncSource;
+    IVsyncRenderer* m_VsyncRenderer;
+    int m_MaxVideoFps;
+    int m_DisplayFps;
+};

app/streaming/video/ffmpeg-renderers/vt.mm

@@ -2,18 +2,18 @@
 // libavutil both defining AVMediaType
 #define AVMediaType AVMediaType_FFmpeg
 #include "vt.h"
-#include "pacer.h"
+#include "pacer/pacer.h"
 #undef AVMediaType
 
 #include <SDL_syswm.h>
 #include <Limelight.h>
 
+#include <mach/mach_time.h>
 #import <Cocoa/Cocoa.h>
 #import <VideoToolbox/VideoToolbox.h>
 #import <AVFoundation/AVFoundation.h>
-#import <CoreVideo/CoreVideo.h>
 
-class VTRenderer : public IFFmpegRenderer
+class VTRenderer : public IFFmpegRenderer, public IVsyncRenderer
 {
 public:
     VTRenderer()
@@ -21,7 +21,7 @@ public:
           m_DisplayLayer(nullptr),
           m_FormatDesc(nullptr),
           m_View(nullptr),
-          m_DisplayLink(nullptr)
+          m_Pacer(this)
     {
     }
 
@@ -37,25 +37,15 @@ public:
             CFRelease(m_FormatDesc);
         }
 
-        if (m_DisplayLink != nullptr) {
-            CVDisplayLinkStop(m_DisplayLink);
-            CVDisplayLinkRelease(m_DisplayLink);
-        }
-
         if (m_View != nullptr) {
             [m_View removeFromSuperview];
         }
     }
 
-    void drawFrame(uint64_t vsyncTime)
+    // Caller frees frame after we return
+    virtual void renderFrameAtVsync(AVFrame* frame) override
     {
         OSStatus status;
-
-        AVFrame* frame = m_Pacer.getFrameAtVsync();
-        if (frame == nullptr) {
-            return;
-        }
-
         CVPixelBufferRef pixBuf = reinterpret_cast<CVPixelBufferRef>(frame->data[3]);
 
         // If the format has changed or doesn't exist yet, construct it with the
@@ -70,7 +60,6 @@ public:
                 SDL_LogError(SDL_LOG_CATEGORY_APPLICATION,
                              "CMVideoFormatDescriptionCreateForImageBuffer() failed: %d",
                              status);
-                av_frame_free(&frame);
                 return;
             }
         }
@@ -79,7 +68,7 @@ public:
         CMSampleTimingInfo timingInfo = {
             .duration = kCMTimeInvalid,
             .decodeTimeStamp = kCMTimeInvalid,
-            .presentationTimeStamp = CMTimeMake(vsyncTime, 1000 * 1000 * 1000)
+            .presentationTimeStamp = CMTimeMake(mach_absolute_time(), 1000 * 1000 * 1000)
         };
 
         CMSampleBufferRef sampleBuffer;
@@ -92,39 +81,12 @@ public:
             SDL_LogError(SDL_LOG_CATEGORY_APPLICATION,
                          "CMSampleBufferCreateReadyWithImageBuffer() failed: %d",
                          status);
-            av_frame_free(&frame);
             return;
         }
 
         [m_DisplayLayer enqueueSampleBuffer:sampleBuffer];
 
         CFRelease(sampleBuffer);
-        av_frame_free(&frame);
-    }
-
-    static
-    CVReturn
-    displayLinkOutputCallback(
-        CVDisplayLinkRef,
-        const CVTimeStamp* now,
-        const CVTimeStamp* /* vsyncTime */,
-        CVOptionFlags,
-        CVOptionFlags*,
-        void *displayLinkContext)
-    {
-        VTRenderer* me = reinterpret_cast<VTRenderer*>(displayLinkContext);
-
-        // In my testing on macOS 10.13, this callback is invoked about 24 ms
-        // prior to the specified v-sync time (now - vsyncTime). Since this is
-        // greater than the standard v-sync interval (16 ms = 60 FPS), we will
-        // draw using the current host time, rather than the actual v-sync target
-        // time. Because the CVDisplayLink is in sync with the actual v-sync
-        // interval, even if many ms prior, we can safely use the current host time
-        // and get a consistent callback for each v-sync. This reduces video latency
-        // by at least 1 frame vs. rendering with the actual vsyncTime.
-        me->drawFrame(now->hostTime);
-
-        return kCVReturnSuccess;
     }
 
     virtual bool initialize(SDL_Window* window,
@@ -135,7 +97,9 @@ public:
     {
         int err;
 
-        m_Pacer.initialize(window, maxFps);
+        if (!m_Pacer.initialize(window, maxFps)) {
+            return false;
+        }
 
         if (videoFormat & VIDEO_FORMAT_MASK_H264) {
             // Prior to 10.13, we'll just assume everything has
@@ -208,10 +172,6 @@ public:
             return false;
         }
 
-        CVDisplayLinkCreateWithActiveCGDisplays(&m_DisplayLink);
-        CVDisplayLinkSetOutputCallback(m_DisplayLink, displayLinkOutputCallback, this);
-        CVDisplayLinkStart(m_DisplayLink);
-
         return true;
     }
 
@@ -259,7 +219,6 @@ private:
     AVSampleBufferDisplayLayer* m_DisplayLayer;
     CMVideoFormatDescriptionRef m_FormatDesc;
     NSView* m_View;
-    CVDisplayLinkRef m_DisplayLink;
     Pacer m_Pacer;
 };