From e68a15c825b72dcf9d7b77a9782c21308ee3825d Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Wed, 15 Aug 2018 22:02:15 -0700 Subject: [PATCH] Move the Vsync logic from VTRenderer into a VsyncSource --- app/app.pro | 13 +- app/streaming/video/ffmpeg-renderers/pacer.h | 29 ----- .../pacer/displaylinkvsyncsource.cpp | 48 +++++++ .../pacer/displaylinkvsyncsource.h | 30 +++++ .../ffmpeg-renderers/{ => pacer}/pacer.cpp | 122 +++++++++++------- .../video/ffmpeg-renderers/pacer/pacer.h | 45 +++++++ app/streaming/video/ffmpeg-renderers/vt.mm | 61 ++------- 7 files changed, 216 insertions(+), 132 deletions(-) delete mode 100644 app/streaming/video/ffmpeg-renderers/pacer.h create mode 100644 app/streaming/video/ffmpeg-renderers/pacer/displaylinkvsyncsource.cpp create mode 100644 app/streaming/video/ffmpeg-renderers/pacer/displaylinkvsyncsource.h rename app/streaming/video/ffmpeg-renderers/{ => pacer}/pacer.cpp (77%) create mode 100644 app/streaming/video/ffmpeg-renderers/pacer/pacer.h diff --git a/app/app.pro b/app/app.pro index afde7514..7ff21a31 100644 --- a/app/app.pro +++ b/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 - HEADERS += streaming/video/ffmpeg-renderers/vt.h + SOURCES += \ + 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 += \ diff --git a/app/streaming/video/ffmpeg-renderers/pacer.h b/app/streaming/video/ffmpeg-renderers/pacer.h deleted file mode 100644 index 1578198f..00000000 --- a/app/streaming/video/ffmpeg-renderers/pacer.h +++ /dev/null @@ -1,29 +0,0 @@ -#pragma once - -#include "renderer.h" - -#include - -class Pacer -{ -public: - Pacer(); - - ~Pacer(); - - AVFrame* getFrameAtVsync(); - - void submitFrame(AVFrame* frame); - - void initialize(SDL_Window* window, int maxVideoFps); - - void drain(); - -private: - QQueue m_FrameQueue; - QQueue m_FrameQueueHistory; - SDL_SpinLock m_FrameQueueLock; - - int m_MaxVideoFps; - int m_DisplayFps; -}; diff --git a/app/streaming/video/ffmpeg-renderers/pacer/displaylinkvsyncsource.cpp b/app/streaming/video/ffmpeg-renderers/pacer/displaylinkvsyncsource.cpp new file mode 100644 index 00000000..1272618d --- /dev/null +++ b/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(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; +} diff --git a/app/streaming/video/ffmpeg-renderers/pacer/displaylinkvsyncsource.h b/app/streaming/video/ffmpeg-renderers/pacer/displaylinkvsyncsource.h new file mode 100644 index 00000000..18fa0508 --- /dev/null +++ b/app/streaming/video/ffmpeg-renderers/pacer/displaylinkvsyncsource.h @@ -0,0 +1,30 @@ +#pragma once + +#include + +#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; +}; + diff --git a/app/streaming/video/ffmpeg-renderers/pacer.cpp b/app/streaming/video/ffmpeg-renderers/pacer/pacer.cpp similarity index 77% rename from app/streaming/video/ffmpeg-renderers/pacer.cpp rename to app/streaming/video/ffmpeg-renderers/pacer/pacer.cpp index 607bf41c..c3bc1794 100644 --- a/app/streaming/video/ffmpeg-renderers/pacer.cpp +++ b/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() -{ - // Make sure initialize() has been called - SDL_assert(m_MaxVideoFps != 0); +#ifdef Q_OS_DARWIN + m_VsyncSource = new DisplayLinkVsyncSource(this); +#else + SDL_assert(false); +#endif - 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; - } - } - - 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; + return m_VsyncSource->initialize(window); } void Pacer::submitFrame(AVFrame* frame) diff --git a/app/streaming/video/ffmpeg-renderers/pacer/pacer.h b/app/streaming/video/ffmpeg-renderers/pacer/pacer.h new file mode 100644 index 00000000..201bef79 --- /dev/null +++ b/app/streaming/video/ffmpeg-renderers/pacer/pacer.h @@ -0,0 +1,45 @@ +#pragma once + +#include "../renderer.h" + +#include + +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 m_FrameQueue; + QQueue m_FrameQueueHistory; + SDL_SpinLock m_FrameQueueLock; + + IVsyncSource* m_VsyncSource; + IVsyncRenderer* m_VsyncRenderer; + int m_MaxVideoFps; + int m_DisplayFps; +}; diff --git a/app/streaming/video/ffmpeg-renderers/vt.mm b/app/streaming/video/ffmpeg-renderers/vt.mm index 68483a3c..92d367e4 100644 --- a/app/streaming/video/ffmpeg-renderers/vt.mm +++ b/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 #include +#include #import #import #import -#import -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(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(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; };