From 7d61acb2a8f05356e5adbed62cae7b72867ddd5c Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Wed, 15 Aug 2018 23:57:03 -0700 Subject: [PATCH] Use Pacer to drive all rendering --- .../video/ffmpeg-renderers/dxva2.cpp | 8 ----- app/streaming/video/ffmpeg-renderers/dxva2.h | 4 +-- .../video/ffmpeg-renderers/pacer/pacer.cpp | 26 ++++++++++++---- .../video/ffmpeg-renderers/pacer/pacer.h | 12 ++------ .../video/ffmpeg-renderers/renderer.h | 4 +-- app/streaming/video/ffmpeg-renderers/sdl.cpp | 5 +--- .../video/ffmpeg-renderers/vaapi.cpp | 4 +-- app/streaming/video/ffmpeg-renderers/vaapi.h | 2 +- .../video/ffmpeg-renderers/vdpau.cpp | 8 +---- app/streaming/video/ffmpeg-renderers/vdpau.h | 2 +- app/streaming/video/ffmpeg-renderers/vt.mm | 30 ++++++------------- app/streaming/video/ffmpeg.cpp | 28 ++++++++++++----- app/streaming/video/ffmpeg.h | 7 +++-- 13 files changed, 65 insertions(+), 75 deletions(-) diff --git a/app/streaming/video/ffmpeg-renderers/dxva2.cpp b/app/streaming/video/ffmpeg-renderers/dxva2.cpp index 61effc76..19dfeba6 100644 --- a/app/streaming/video/ffmpeg-renderers/dxva2.cpp +++ b/app/streaming/video/ffmpeg-renderers/dxva2.cpp @@ -11,7 +11,6 @@ DEFINE_GUID(DXVADDI_Intel_ModeH264_E, 0x604F8E68,0x4951,0x4C54,0x88,0xFE,0xAB,0x DXVA2Renderer::DXVA2Renderer() : m_SdlRenderer(nullptr), - m_Pacer(this), m_DecService(nullptr), m_Decoder(nullptr), m_SurfacesUsed(0), @@ -28,8 +27,6 @@ DXVA2Renderer::DXVA2Renderer() : DXVA2Renderer::~DXVA2Renderer() { - m_Pacer.drain(); - SAFE_COM_RELEASE(m_DecService); SAFE_COM_RELEASE(m_Decoder); SAFE_COM_RELEASE(m_Device); @@ -499,11 +496,6 @@ bool DXVA2Renderer::initialize(SDL_Window* window, int videoFormat, int width, i return true; } -void DXVA2Renderer::renderFrame(AVFrame* frame) -{ - m_Pacer.submitFrame(frame); -} - void DXVA2Renderer::renderFrameAtVsync(AVFrame *frame) { IDirect3DSurface9* surface = reinterpret_cast(frame->data[3]); diff --git a/app/streaming/video/ffmpeg-renderers/dxva2.h b/app/streaming/video/ffmpeg-renderers/dxva2.h index a47644be..6dbbbd4e 100644 --- a/app/streaming/video/ffmpeg-renderers/dxva2.h +++ b/app/streaming/video/ffmpeg-renderers/dxva2.h @@ -10,7 +10,7 @@ extern "C" { #include } -class DXVA2Renderer : public IFFmpegRenderer, public IVsyncRenderer +class DXVA2Renderer : public IFFmpegRenderer { public: DXVA2Renderer(); @@ -21,7 +21,6 @@ public: int height, int maxFps); virtual bool prepareDecoderContext(AVCodecContext* context); - virtual void renderFrame(AVFrame* frame); virtual void renderFrameAtVsync(AVFrame* frame); private: @@ -46,7 +45,6 @@ private: int m_DisplayHeight; SDL_Renderer* m_SdlRenderer; - Pacer m_Pacer; struct dxva_context m_DXVAContext; IDirect3DSurface9* m_DecSurfaces[19]; diff --git a/app/streaming/video/ffmpeg-renderers/pacer/pacer.cpp b/app/streaming/video/ffmpeg-renderers/pacer/pacer.cpp index 3b7a867c..9d94adfe 100644 --- a/app/streaming/video/ffmpeg-renderers/pacer/pacer.cpp +++ b/app/streaming/video/ffmpeg-renderers/pacer/pacer.cpp @@ -10,7 +10,7 @@ #define FRAME_HISTORY_ENTRIES 8 -Pacer::Pacer(IVsyncRenderer* renderer) : +Pacer::Pacer(IFFmpegRenderer* renderer) : m_FrameQueueLock(0), m_VsyncSource(nullptr), m_VsyncRenderer(renderer), @@ -111,10 +111,15 @@ bool Pacer::initialize(SDL_Window* window, int maxVideoFps) #elif defined(Q_OS_WIN32) m_VsyncSource = new DxVsyncSource(this); #else - SDL_assert(false); + // Platforms without a VsyncSource will just render frames + // immediately like they used to. #endif - return m_VsyncSource->initialize(window); + if (m_VsyncSource != nullptr && !m_VsyncSource->initialize(window)) { + return false; + } + + return true; } void Pacer::submitFrame(AVFrame* frame) @@ -122,9 +127,18 @@ void Pacer::submitFrame(AVFrame* frame) // Make sure initialize() has been called SDL_assert(m_MaxVideoFps != 0); - SDL_AtomicLock(&m_FrameQueueLock); - m_FrameQueue.enqueue(frame); - SDL_AtomicUnlock(&m_FrameQueueLock); + // Queue the frame until the V-sync callback if + // we have a V-sync source, otherwise deliver it + // immediately and hope for the best. + if (m_VsyncSource != nullptr) { + SDL_AtomicLock(&m_FrameQueueLock); + m_FrameQueue.enqueue(frame); + SDL_AtomicUnlock(&m_FrameQueueLock); + } + else { + m_VsyncRenderer->renderFrameAtVsync(frame); + av_frame_free(&frame); + } } void Pacer::drain() diff --git a/app/streaming/video/ffmpeg-renderers/pacer/pacer.h b/app/streaming/video/ffmpeg-renderers/pacer/pacer.h index 201bef79..2c190cb0 100644 --- a/app/streaming/video/ffmpeg-renderers/pacer/pacer.h +++ b/app/streaming/video/ffmpeg-renderers/pacer/pacer.h @@ -4,14 +4,6 @@ #include -class Pacer; - -class IVsyncRenderer { -public: - virtual ~IVsyncRenderer() {} - virtual void renderFrameAtVsync(AVFrame* frame) = 0; -}; - class IVsyncSource { public: virtual ~IVsyncSource() {} @@ -21,7 +13,7 @@ public: class Pacer { public: - Pacer(IVsyncRenderer* renderer); + Pacer(IFFmpegRenderer* renderer); ~Pacer(); @@ -39,7 +31,7 @@ private: SDL_SpinLock m_FrameQueueLock; IVsyncSource* m_VsyncSource; - IVsyncRenderer* m_VsyncRenderer; + IFFmpegRenderer* m_VsyncRenderer; int m_MaxVideoFps; int m_DisplayFps; }; diff --git a/app/streaming/video/ffmpeg-renderers/renderer.h b/app/streaming/video/ffmpeg-renderers/renderer.h index 066e6df1..c1ad8699 100644 --- a/app/streaming/video/ffmpeg-renderers/renderer.h +++ b/app/streaming/video/ffmpeg-renderers/renderer.h @@ -15,7 +15,7 @@ public: int height, int maxFps) = 0; virtual bool prepareDecoderContext(AVCodecContext* context) = 0; - virtual void renderFrame(AVFrame* frame) = 0; + virtual void renderFrameAtVsync(AVFrame* frame) = 0; }; class SdlRenderer : public IFFmpegRenderer { @@ -28,7 +28,7 @@ public: int height, int maxFps); virtual bool prepareDecoderContext(AVCodecContext* context); - virtual void renderFrame(AVFrame* frame); + virtual void renderFrameAtVsync(AVFrame* frame); private: SDL_Renderer* m_Renderer; diff --git a/app/streaming/video/ffmpeg-renderers/sdl.cpp b/app/streaming/video/ffmpeg-renderers/sdl.cpp index 6c17a6d4..ac1c786a 100644 --- a/app/streaming/video/ffmpeg-renderers/sdl.cpp +++ b/app/streaming/video/ffmpeg-renderers/sdl.cpp @@ -68,7 +68,7 @@ bool SdlRenderer::initialize(SDL_Window* window, return true; } -void SdlRenderer::renderFrame(AVFrame* frame) +void SdlRenderer::renderFrameAtVsync(AVFrame* frame) { SDL_UpdateYUVTexture(m_Texture, nullptr, frame->data[0], @@ -78,9 +78,6 @@ void SdlRenderer::renderFrame(AVFrame* frame) frame->data[2], frame->linesize[2]); - // Done with the frame now - av_frame_free(&frame); - SDL_RenderClear(m_Renderer); SDL_RenderCopy(m_Renderer, m_Texture, nullptr, nullptr); SDL_RenderPresent(m_Renderer); diff --git a/app/streaming/video/ffmpeg-renderers/vaapi.cpp b/app/streaming/video/ffmpeg-renderers/vaapi.cpp index 20b72773..15ee06dd 100644 --- a/app/streaming/video/ffmpeg-renderers/vaapi.cpp +++ b/app/streaming/video/ffmpeg-renderers/vaapi.cpp @@ -142,7 +142,7 @@ VAAPIRenderer::prepareDecoderContext(AVCodecContext* context) } void -VAAPIRenderer::renderFrame(AVFrame* frame) +VAAPIRenderer::renderFrameAtVsync(AVFrame* frame) { VASurfaceID surface = (VASurfaceID)(uintptr_t)frame->data[3]; AVHWDeviceContext* deviceContext = (AVHWDeviceContext*)m_HwContext->data; @@ -197,6 +197,4 @@ VAAPIRenderer::renderFrame(AVFrame* frame) // We don't accept anything else in initialize(). SDL_assert(false); } - - av_frame_free(&frame); } diff --git a/app/streaming/video/ffmpeg-renderers/vaapi.h b/app/streaming/video/ffmpeg-renderers/vaapi.h index f0f63bea..e5543e9e 100644 --- a/app/streaming/video/ffmpeg-renderers/vaapi.h +++ b/app/streaming/video/ffmpeg-renderers/vaapi.h @@ -36,7 +36,7 @@ public: int height, int maxFps); virtual bool prepareDecoderContext(AVCodecContext* context); - virtual void renderFrame(AVFrame* frame); + virtual void renderFrameAtVsync(AVFrame* frame); private: int m_WindowSystem; diff --git a/app/streaming/video/ffmpeg-renderers/vdpau.cpp b/app/streaming/video/ffmpeg-renderers/vdpau.cpp index 649da9a3..7afd4096 100644 --- a/app/streaming/video/ffmpeg-renderers/vdpau.cpp +++ b/app/streaming/video/ffmpeg-renderers/vdpau.cpp @@ -223,7 +223,7 @@ bool VDPAURenderer::prepareDecoderContext(AVCodecContext* context) return true; } -void VDPAURenderer::renderFrame(AVFrame* frame) +void VDPAURenderer::renderFrameAtVsync(AVFrame* frame) { VdpStatus status; VdpVideoSurface videoSurface = (VdpVideoSurface)(uintptr_t)frame->data[3]; @@ -243,7 +243,6 @@ void VDPAURenderer::renderFrame(AVFrame* frame) SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "VdpVideoSurfaceGetParameters() failed: %s", m_VdpGetErrorString(status)); - av_frame_free(&frame); return; } @@ -270,7 +269,6 @@ void VDPAURenderer::renderFrame(AVFrame* frame) SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "VdpVideoMixerCreate() failed: %s", m_VdpGetErrorString(status)); - av_frame_free(&frame); return; } } @@ -313,10 +311,6 @@ void VDPAURenderer::renderFrame(AVFrame* frame) nullptr, 0, nullptr); - - // The decoder can have this surface back now - av_frame_free(&frame); - if (status != VDP_STATUS_OK) { SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "VdpVideoMixerRender() failed: %s", diff --git a/app/streaming/video/ffmpeg-renderers/vdpau.h b/app/streaming/video/ffmpeg-renderers/vdpau.h index 4610406b..0b8b200c 100644 --- a/app/streaming/video/ffmpeg-renderers/vdpau.h +++ b/app/streaming/video/ffmpeg-renderers/vdpau.h @@ -19,7 +19,7 @@ public: int height, int maxFps); virtual bool prepareDecoderContext(AVCodecContext* context); - virtual void renderFrame(AVFrame* frame); + virtual void renderFrameAtVsync(AVFrame* frame); private: uint32_t m_VideoWidth, m_VideoHeight; diff --git a/app/streaming/video/ffmpeg-renderers/vt.mm b/app/streaming/video/ffmpeg-renderers/vt.mm index 92d367e4..59177e50 100644 --- a/app/streaming/video/ffmpeg-renderers/vt.mm +++ b/app/streaming/video/ffmpeg-renderers/vt.mm @@ -13,22 +13,19 @@ #import #import -class VTRenderer : public IFFmpegRenderer, public IVsyncRenderer +class VTRenderer : public IFFmpegRenderer { public: VTRenderer() : m_HwContext(nullptr), m_DisplayLayer(nullptr), m_FormatDesc(nullptr), - m_View(nullptr), - m_Pacer(this) + m_View(nullptr) { } virtual ~VTRenderer() { - m_Pacer.drain(); - if (m_HwContext != nullptr) { av_buffer_unref(&m_HwContext); } @@ -48,6 +45,13 @@ public: OSStatus status; CVPixelBufferRef pixBuf = reinterpret_cast(frame->data[3]); + // FIXME: Only on main thread + if (m_DisplayLayer.status == AVQueuedSampleBufferRenderingStatusFailed) { + SDL_LogWarn(SDL_LOG_CATEGORY_APPLICATION, + "Resetting failed AVSampleBufferDisplay layer"); + setupDisplayLayer(); + } + // If the format has changed or doesn't exist yet, construct it with the // pixel buffer data if (!m_FormatDesc || !CMVideoFormatDescriptionMatchesImageBuffer(m_FormatDesc, pixBuf)) { @@ -97,10 +101,6 @@ public: { int err; - if (!m_Pacer.initialize(window, maxFps)) { - return false; - } - if (videoFormat & VIDEO_FORMAT_MASK_H264) { // Prior to 10.13, we'll just assume everything has // H.264 support and fail open to allow VT decode. @@ -185,17 +185,6 @@ public: return true; } - virtual void renderFrame(AVFrame* frame) override - { - if (m_DisplayLayer.status == AVQueuedSampleBufferRenderingStatusFailed) { - SDL_LogWarn(SDL_LOG_CATEGORY_APPLICATION, - "Resetting failed AVSampleBufferDisplay layer"); - setupDisplayLayer(); - } - - m_Pacer.submitFrame(frame); - } - private: void setupDisplayLayer() { @@ -219,7 +208,6 @@ private: AVSampleBufferDisplayLayer* m_DisplayLayer; CMVideoFormatDescriptionRef m_FormatDesc; NSView* m_View; - Pacer m_Pacer; }; IFFmpegRenderer* VTRendererFactory::createRenderer() { diff --git a/app/streaming/video/ffmpeg.cpp b/app/streaming/video/ffmpeg.cpp index 211a2e75..97cfa3c2 100644 --- a/app/streaming/video/ffmpeg.cpp +++ b/app/streaming/video/ffmpeg.cpp @@ -53,7 +53,8 @@ FFmpegVideoDecoder::FFmpegVideoDecoder() m_DecodeBuffer(1024 * 1024, 0), m_HwDecodeCfg(nullptr), m_Renderer(nullptr), - m_ConsecutiveFailedDecodes(0) + m_ConsecutiveFailedDecodes(0), + m_Pacer(nullptr) { av_init_packet(&m_Pkt); SDL_AtomicSet(&m_QueuedFrames, 0); @@ -91,19 +92,29 @@ void FFmpegVideoDecoder::reset() dropFrame(&event.user); } else { - SDL_Delay(100); + SDL_Delay(10); SDL_PumpEvents(); } } + delete m_Pacer; + m_Pacer = nullptr; + delete m_Renderer; m_Renderer = nullptr; avcodec_free_context(&m_VideoDecoderCtx); } -bool FFmpegVideoDecoder::completeInitialization(AVCodec* decoder, int videoFormat, int width, int height, bool testOnly) +bool FFmpegVideoDecoder::completeInitialization(AVCodec* decoder, SDL_Window* window, + int videoFormat, int width, int height, + int maxFps, bool testOnly) { + m_Pacer = new Pacer(m_Renderer); + if (!m_Pacer->initialize(window, maxFps)) { + return false; + } + m_VideoDecoderCtx = avcodec_alloc_context3(decoder); if (!m_VideoDecoderCtx) { SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, @@ -254,7 +265,7 @@ bool FFmpegVideoDecoder::initialize( m_Renderer = new SdlRenderer(); if (vds != StreamingPreferences::VDS_FORCE_HARDWARE && m_Renderer->initialize(window, videoFormat, width, height, maxFps) && - completeInitialization(decoder, videoFormat, width, height, false)) { + completeInitialization(decoder, window, videoFormat, width, height, maxFps, false)) { return true; } else { @@ -271,12 +282,12 @@ bool FFmpegVideoDecoder::initialize( m_HwDecodeCfg = config; // Submit test frame to ensure this codec really works if (m_Renderer->initialize(window, videoFormat, width, height, maxFps) && - completeInitialization(decoder, videoFormat, width, height, true)) { + completeInitialization(decoder, window, videoFormat, width, height, maxFps, true)) { // OK, it worked, so now let's initialize it for real reset(); if ((m_Renderer = createAcceleratedRenderer(config)) != nullptr && m_Renderer->initialize(window, videoFormat, width, height, maxFps) && - completeInitialization(decoder, videoFormat, width, height, false)) { + completeInitialization(decoder, window, videoFormat, width, height, maxFps, false)) { return true; } else { @@ -380,7 +391,7 @@ int FFmpegVideoDecoder::submitDecodeUnit(PDECODE_UNIT du) void FFmpegVideoDecoder::renderFrame(SDL_UserEvent* event) { AVFrame* frame = reinterpret_cast(event->data1); - m_Renderer->renderFrame(frame); + m_Pacer->submitFrame(frame); SDL_AtomicDecRef(&m_QueuedFrames); } @@ -388,6 +399,9 @@ void FFmpegVideoDecoder::renderFrame(SDL_UserEvent* event) void FFmpegVideoDecoder::dropFrame(SDL_UserEvent* event) { AVFrame* frame = reinterpret_cast(event->data1); + // We should really call Pacer::submitFrame() here and let it + // take care of it, but that will regress frame dropping for + // clients without an IVsyncSource implementation. av_frame_free(&frame); SDL_AtomicDecRef(&m_QueuedFrames); } diff --git a/app/streaming/video/ffmpeg.h b/app/streaming/video/ffmpeg.h index 82d709c8..009f91fb 100644 --- a/app/streaming/video/ffmpeg.h +++ b/app/streaming/video/ffmpeg.h @@ -2,6 +2,7 @@ #include "decoder.h" #include "ffmpeg-renderers/renderer.h" +#include "ffmpeg-renderers/pacer/pacer.h" extern "C" { #include @@ -25,8 +26,9 @@ public: virtual IFFmpegRenderer* getRenderer(); private: - bool completeInitialization(AVCodec* decoder, int videoFormat, - int width, int height, bool testOnly); + bool completeInitialization(AVCodec* decoder, SDL_Window* window, + int videoFormat, int width, int height, + int maxFps, bool testOnly); IFFmpegRenderer* createAcceleratedRenderer(const AVCodecHWConfig* hwDecodeCfg); @@ -43,6 +45,7 @@ private: IFFmpegRenderer* m_Renderer; SDL_atomic_t m_QueuedFrames; int m_ConsecutiveFailedDecodes; + Pacer* m_Pacer; static const uint8_t k_H264TestFrame[]; static const uint8_t k_HEVCTestFrame[];