From 112e18a5d1f9b08188793c1062973763aa83d9f2 Mon Sep 17 00:00:00 2001 From: degasus Date: Sat, 15 Oct 2016 19:00:34 +0200 Subject: [PATCH 1/5] AVIDump: Drop stored frame. This used an invalid pointer, which was only valid within AddFrame. This drops a feature which shall dump the last frame as it might was dropped before. A good implementation however should "overwrite" the last frame if the time matches. But this needs to delay every frame a bit. --- Source/Core/VideoCommon/AVIDump.cpp | 30 +---------------------------- Source/Core/VideoCommon/AVIDump.h | 1 - 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/Source/Core/VideoCommon/AVIDump.cpp b/Source/Core/VideoCommon/AVIDump.cpp index 58782f9b0f..adfbe022ad 100644 --- a/Source/Core/VideoCommon/AVIDump.cpp +++ b/Source/Core/VideoCommon/AVIDump.cpp @@ -42,16 +42,10 @@ static int s_height; static u64 s_last_frame; static bool s_last_frame_is_valid = false; static bool s_start_dumping = false; -static bool s_stop_dumping = false; static u64 s_last_pts; static int s_current_width; static int s_current_height; static int s_file_index = 0; -static const u8* s_stored_frame_data; -static int s_stored_frame_width; -static int s_stored_frame_height; -static int s_stored_frame_stride; -static u64 s_stored_frame_ticks; static void InitAVCodec() { @@ -75,8 +69,6 @@ bool AVIDump::Start(int w, int h) s_last_frame_is_valid = false; s_last_pts = 0; - s_stop_dumping = false; - InitAVCodec(); bool success = CreateFile(); if (!success) @@ -172,14 +164,7 @@ static void PreparePacket(AVPacket* pkt) void AVIDump::AddFrame(const u8* data, int width, int height, int stride, u64 ticks) { - // Store current frame data in case frame dumping stops before next frame update, - // but make sure that you don't store the last stored frame and check the resolution upon - // closing the file or else you store recursion, and dolphins don't like recursion. - if (!s_stop_dumping) - { - StoreFrameData(data, width, height, stride, ticks); - CheckResolution(width, height); - } + CheckResolution(width, height); s_src_frame->data[0] = const_cast(data); s_src_frame->linesize[0] = stride; s_src_frame->format = s_pix_fmt; @@ -259,10 +244,6 @@ void AVIDump::AddFrame(const u8* data, int width, int height, int stride, u64 ti void AVIDump::Stop() { - s_stop_dumping = true; - // Write the last stored frame just in case frame dumping stops before the next frame update - AddFrame(s_stored_frame_data, s_stored_frame_width, s_stored_frame_height, s_stored_frame_stride, - s_stored_frame_ticks); av_write_trailer(s_format_context); CloseFile(); s_file_index = 0; @@ -322,12 +303,3 @@ void AVIDump::CheckResolution(int width, int height) s_current_height = height; } } - -void AVIDump::StoreFrameData(const u8* data, int width, int height, int stride, u64 ticks) -{ - s_stored_frame_data = data; - s_stored_frame_width = width; - s_stored_frame_height = height; - s_stored_frame_stride = stride; - s_stored_frame_ticks = ticks; -} diff --git a/Source/Core/VideoCommon/AVIDump.h b/Source/Core/VideoCommon/AVIDump.h index c0c205210f..d4a43a4522 100644 --- a/Source/Core/VideoCommon/AVIDump.h +++ b/Source/Core/VideoCommon/AVIDump.h @@ -12,7 +12,6 @@ private: static bool CreateFile(); static void CloseFile(); static void CheckResolution(int width, int height); - static void StoreFrameData(const u8* data, int width, int height, int stride, u64 ticks); public: static bool Start(int w, int h); From 03d8efc270c5a522c2991d37e43f753b2ca906c3 Mon Sep 17 00:00:00 2001 From: degasus Date: Sat, 15 Oct 2016 19:06:01 +0200 Subject: [PATCH 2/5] AVIDump: Merge redundant variables. They were always the same. We also don't scale at all. --- Source/Core/VideoCommon/AVIDump.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/Source/Core/VideoCommon/AVIDump.cpp b/Source/Core/VideoCommon/AVIDump.cpp index adfbe022ad..e4fb4d7302 100644 --- a/Source/Core/VideoCommon/AVIDump.cpp +++ b/Source/Core/VideoCommon/AVIDump.cpp @@ -43,8 +43,6 @@ static u64 s_last_frame; static bool s_last_frame_is_valid = false; static bool s_start_dumping = false; static u64 s_last_pts; -static int s_current_width; -static int s_current_height; static int s_file_index = 0; static void InitAVCodec() @@ -63,8 +61,6 @@ bool AVIDump::Start(int w, int h) s_width = w; s_height = h; - s_current_width = w; - s_current_height = h; s_last_frame_is_valid = false; s_last_pts = 0; @@ -171,8 +167,7 @@ void AVIDump::AddFrame(const u8* data, int width, int height, int stride, u64 ti s_src_frame->width = s_width; s_src_frame->height = s_height; - // Convert image from {BGR24, RGBA} to desired pixel format, and scale to initial - // width and height + // Convert image from {BGR24, RGBA} to desired pixel format if ((s_sws_context = sws_getCachedContext(s_sws_context, width, height, s_pix_fmt, s_width, s_height, s_stream->codec->pix_fmt, SWS_BICUBIC, nullptr, nullptr, nullptr))) @@ -293,13 +288,11 @@ void AVIDump::CheckResolution(int width, int height) // (possibly width as well, but no examples known) to have a value of zero. This can occur as the // VI is able to be set to a zero value for height/width to disable output. If this is the case, // simply keep the last known resolution of the video for the added frame. - if ((width != s_current_width || height != s_current_height) && (width > 0 && height > 0)) + if ((width != s_width || height != s_height) && (width > 0 && height > 0)) { int temp_file_index = s_file_index; Stop(); s_file_index = temp_file_index + 1; Start(width, height); - s_current_width = width; - s_current_height = height; } } From dad5041737053a75c58123c637c28c350d99dd08 Mon Sep 17 00:00:00 2001 From: degasus Date: Sun, 16 Oct 2016 13:26:37 +0200 Subject: [PATCH 3/5] AVIDump: Inline OSD error handling. This fixes a review feedback in PR #4345. --- Source/Core/VideoCommon/AVIDump.cpp | 8 ++++++++ Source/Core/VideoCommon/RenderBase.cpp | 12 +----------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/Source/Core/VideoCommon/AVIDump.cpp b/Source/Core/VideoCommon/AVIDump.cpp index e4fb4d7302..7b09d10c1e 100644 --- a/Source/Core/VideoCommon/AVIDump.cpp +++ b/Source/Core/VideoCommon/AVIDump.cpp @@ -23,7 +23,9 @@ extern "C" { #include "Core/HW/SystemTimers.h" #include "Core/HW/VideoInterface.h" //for TargetRefreshRate #include "Core/Movie.h" + #include "VideoCommon/AVIDump.h" +#include "VideoCommon/OnScreenDisplay.h" #include "VideoCommon/VideoConfig.h" #if LIBAVCODEC_VERSION_INT < AV_VERSION_INT(55, 28, 1) @@ -68,7 +70,10 @@ bool AVIDump::Start(int w, int h) InitAVCodec(); bool success = CreateFile(); if (!success) + { CloseFile(); + OSD::AddMessage("AVIDump Start failed"); + } return success; } @@ -148,6 +153,9 @@ bool AVIDump::CreateFile() return false; } + OSD::AddMessage(StringFromFormat("Dumping Frames to \"%s\" (%dx%d)", s_format_context->filename, + s_width, s_height)); + return true; } diff --git a/Source/Core/VideoCommon/RenderBase.cpp b/Source/Core/VideoCommon/RenderBase.cpp index 92817805e3..013ee7f07d 100644 --- a/Source/Core/VideoCommon/RenderBase.cpp +++ b/Source/Core/VideoCommon/RenderBase.cpp @@ -660,7 +660,7 @@ bool Renderer::IsFrameDumping() AVIDump::Stop(); std::vector().swap(m_frame_data); m_AVI_dumping = false; - OSD::AddMessage("Stop dumping frames", 2000); + OSD::AddMessage("Stop dumping frames"); } m_last_frame_dumped = false; #endif @@ -698,16 +698,6 @@ void Renderer::DumpFrameData(const u8* data, int w, int h, int stride, u64 ticks if (!m_last_frame_dumped) { m_AVI_dumping = AVIDump::Start(w, h); - if (!m_AVI_dumping) - { - OSD::AddMessage("AVIDump Start failed", 2000); - } - else - { - OSD::AddMessage(StringFromFormat("Dumping Frames to \"%sframedump0.avi\" (%dx%d RGB24)", - File::GetUserPath(D_DUMPFRAMES_IDX).c_str(), w, h), - 2000); - } } if (m_AVI_dumping) { From be29090aaea5482234c06e1220a22797aad0b114 Mon Sep 17 00:00:00 2001 From: degasus Date: Fri, 4 Nov 2016 18:19:35 +0100 Subject: [PATCH 4/5] AVIDump: Add a struct for the state. So AddFrame use no global state and can be threaded well. --- Source/Core/VideoBackends/D3D/Render.cpp | 3 ++- Source/Core/VideoBackends/D3D12/Render.cpp | 3 ++- Source/Core/VideoBackends/OGL/Render.cpp | 3 ++- .../VideoBackends/Software/SWRenderer.cpp | 3 ++- Source/Core/VideoBackends/Vulkan/Renderer.cpp | 3 ++- Source/Core/VideoCommon/AVIDump.cpp | 26 ++++++++++++------- Source/Core/VideoCommon/AVIDump.h | 15 ++++++++++- Source/Core/VideoCommon/RenderBase.cpp | 4 +-- Source/Core/VideoCommon/RenderBase.h | 2 +- 9 files changed, 44 insertions(+), 18 deletions(-) diff --git a/Source/Core/VideoBackends/D3D/Render.cpp b/Source/Core/VideoBackends/D3D/Render.cpp index aa52ffc4ed..bf0b487276 100644 --- a/Source/Core/VideoBackends/D3D/Render.cpp +++ b/Source/Core/VideoBackends/D3D/Render.cpp @@ -824,8 +824,9 @@ void Renderer::SwapImpl(u32 xfbAddr, u32 fbWidth, u32 fbStride, u32 fbHeight, D3D11_MAPPED_SUBRESOURCE map; D3D::context->Map(s_screenshot_texture, 0, D3D11_MAP_READ, 0, &map); + AVIDump::Frame state = AVIDump::FetchState(ticks); DumpFrameData(reinterpret_cast(map.pData), source_width, source_height, map.RowPitch, - ticks); + state); FinishFrameData(); D3D::context->Unmap(s_screenshot_texture, 0); diff --git a/Source/Core/VideoBackends/D3D12/Render.cpp b/Source/Core/VideoBackends/D3D12/Render.cpp index 9c1cbdad76..8b3c36651b 100644 --- a/Source/Core/VideoBackends/D3D12/Render.cpp +++ b/Source/Core/VideoBackends/D3D12/Render.cpp @@ -777,8 +777,9 @@ void Renderer::SwapImpl(u32 xfb_addr, u32 fb_width, u32 fb_stride, u32 fb_height D3D12_RANGE read_range = {0, dst_location.PlacedFootprint.Footprint.RowPitch * source_height}; CheckHR(s_screenshot_texture->Map(0, &read_range, &screenshot_texture_map)); + AVIDump::Frame state = AVIDump::FetchState(ticks); DumpFrameData(reinterpret_cast(screenshot_texture_map), source_width, source_height, - dst_location.PlacedFootprint.Footprint.RowPitch, ticks); + dst_location.PlacedFootprint.Footprint.RowPitch, state); FinishFrameData(); D3D12_RANGE write_range = {}; diff --git a/Source/Core/VideoBackends/OGL/Render.cpp b/Source/Core/VideoBackends/OGL/Render.cpp index de7dc8fff6..e3d3bea2d5 100644 --- a/Source/Core/VideoBackends/OGL/Render.cpp +++ b/Source/Core/VideoBackends/OGL/Render.cpp @@ -1455,8 +1455,9 @@ void Renderer::SwapImpl(u32 xfbAddr, u32 fbWidth, u32 fbStride, u32 fbHeight, glReadPixels(flipped_trc.left, flipped_trc.bottom, flipped_trc.GetWidth(), flipped_trc.GetHeight(), GL_RGBA, GL_UNSIGNED_BYTE, image.data()); + AVIDump::Frame state = AVIDump::FetchState(ticks); DumpFrameData(image.data(), flipped_trc.GetWidth(), flipped_trc.GetHeight(), - flipped_trc.GetWidth() * 4, ticks, true); + flipped_trc.GetWidth() * 4, state, true); FinishFrameData(); } // Finish up the current frame, print some stats diff --git a/Source/Core/VideoBackends/Software/SWRenderer.cpp b/Source/Core/VideoBackends/Software/SWRenderer.cpp index 2e7992c705..df77a45466 100644 --- a/Source/Core/VideoBackends/Software/SWRenderer.cpp +++ b/Source/Core/VideoBackends/Software/SWRenderer.cpp @@ -125,7 +125,8 @@ void SWRenderer::SwapImpl(u32 xfbAddr, u32 fbWidth, u32 fbStride, u32 fbHeight, // Save screenshot if (IsFrameDumping()) { - DumpFrameData(GetCurrentColorTexture(), fbWidth, fbHeight, fbWidth * 4, ticks); + AVIDump::Frame state = AVIDump::FetchState(ticks); + DumpFrameData(GetCurrentColorTexture(), fbWidth, fbHeight, fbWidth * 4, state); FinishFrameData(); } diff --git a/Source/Core/VideoBackends/Vulkan/Renderer.cpp b/Source/Core/VideoBackends/Vulkan/Renderer.cpp index 066f3f512a..0449d49f14 100644 --- a/Source/Core/VideoBackends/Vulkan/Renderer.cpp +++ b/Source/Core/VideoBackends/Vulkan/Renderer.cpp @@ -747,10 +747,11 @@ bool Renderer::DrawFrameDump(const EFBRectangle& rc, u32 xfb_addr, void Renderer::DumpFrame(u64 ticks) { + AVIDump::Frame state = AVIDump::FetchState(ticks); DumpFrameData(reinterpret_cast(m_frame_dump_readback_texture->GetMapPointer()), static_cast(m_frame_dump_render_texture->GetWidth()), static_cast(m_frame_dump_render_texture->GetHeight()), - static_cast(m_frame_dump_readback_texture->GetRowStride()), ticks); + static_cast(m_frame_dump_readback_texture->GetRowStride()), state); FinishFrameData(); } diff --git a/Source/Core/VideoCommon/AVIDump.cpp b/Source/Core/VideoCommon/AVIDump.cpp index 7b09d10c1e..1756be97a8 100644 --- a/Source/Core/VideoCommon/AVIDump.cpp +++ b/Source/Core/VideoCommon/AVIDump.cpp @@ -166,7 +166,7 @@ static void PreparePacket(AVPacket* pkt) pkt->size = 0; } -void AVIDump::AddFrame(const u8* data, int width, int height, int stride, u64 ticks) +void AVIDump::AddFrame(const u8* data, int width, int height, int stride, const Frame& state) { CheckResolution(width, height); s_src_frame->data[0] = const_cast(data); @@ -196,26 +196,25 @@ void AVIDump::AddFrame(const u8* data, int width, int height, int stride, u64 ti // incorrectly. if (!s_last_frame_is_valid) { - s_last_frame = ticks; + s_last_frame = state.ticks; s_last_frame_is_valid = true; } - if (!s_start_dumping && Movie::GetCurrentFrame() < 1) + if (!s_start_dumping && state.first_frame) { - delta = ticks; + delta = state.ticks; last_pts = AV_NOPTS_VALUE; s_start_dumping = true; } else { - delta = ticks - s_last_frame; - last_pts = (s_last_pts * s_stream->codec->time_base.den) / SystemTimers::GetTicksPerSecond(); + delta = state.ticks - s_last_frame; + last_pts = (s_last_pts * s_stream->codec->time_base.den) / state.ticks_per_second; } u64 pts_in_ticks = s_last_pts + delta; - s_scaled_frame->pts = - (pts_in_ticks * s_stream->codec->time_base.den) / SystemTimers::GetTicksPerSecond(); + s_scaled_frame->pts = (pts_in_ticks * s_stream->codec->time_base.den) / state.ticks_per_second; if (s_scaled_frame->pts != last_pts) { - s_last_frame = ticks; + s_last_frame = state.ticks; s_last_pts = pts_in_ticks; error = avcodec_encode_video2(s_stream->codec, &pkt, s_scaled_frame, &got_packet); } @@ -304,3 +303,12 @@ void AVIDump::CheckResolution(int width, int height) Start(width, height); } } + +AVIDump::Frame AVIDump::FetchState(u64 ticks) +{ + Frame state; + state.ticks = ticks; + state.first_frame = Movie::GetCurrentFrame() < 1; + state.ticks_per_second = SystemTimers::GetTicksPerSecond(); + return state; +} diff --git a/Source/Core/VideoCommon/AVIDump.h b/Source/Core/VideoCommon/AVIDump.h index d4a43a4522..bbea5e2bbf 100644 --- a/Source/Core/VideoCommon/AVIDump.h +++ b/Source/Core/VideoCommon/AVIDump.h @@ -14,8 +14,21 @@ private: static void CheckResolution(int width, int height); public: + struct Frame + { + u64 ticks = 0; + u32 ticks_per_second = 0; + bool first_frame = false; + }; + static bool Start(int w, int h); - static void AddFrame(const u8* data, int width, int height, int stride, u64 ticks); + static void AddFrame(const u8* data, int width, int height, int stride, const Frame& state); static void Stop(); static void DoState(); + +#if defined(HAVE_LIBAV) || defined(_WIN32) + static Frame FetchState(u64 ticks); +#else + static Frame FetchState(u64 ticks) { return {}; } +#endif }; diff --git a/Source/Core/VideoCommon/RenderBase.cpp b/Source/Core/VideoCommon/RenderBase.cpp index 013ee7f07d..65f716ba9e 100644 --- a/Source/Core/VideoCommon/RenderBase.cpp +++ b/Source/Core/VideoCommon/RenderBase.cpp @@ -667,7 +667,7 @@ bool Renderer::IsFrameDumping() return false; } -void Renderer::DumpFrameData(const u8* data, int w, int h, int stride, u64 ticks, +void Renderer::DumpFrameData(const u8* data, int w, int h, int stride, const AVIDump::Frame& state, bool swap_upside_down) { if (w == 0 || h == 0) @@ -701,7 +701,7 @@ void Renderer::DumpFrameData(const u8* data, int w, int h, int stride, u64 ticks } if (m_AVI_dumping) { - AVIDump::AddFrame(m_frame_data.data(), w, h, stride, ticks); + AVIDump::AddFrame(m_frame_data.data(), w, h, stride, state); } m_last_frame_dumped = true; diff --git a/Source/Core/VideoCommon/RenderBase.h b/Source/Core/VideoCommon/RenderBase.h index 8b3495b82e..f3526e8516 100644 --- a/Source/Core/VideoCommon/RenderBase.h +++ b/Source/Core/VideoCommon/RenderBase.h @@ -147,7 +147,7 @@ protected: static void RecordVideoMemory(); bool IsFrameDumping(); - void DumpFrameData(const u8* data, int w, int h, int stride, u64 ticks, + void DumpFrameData(const u8* data, int w, int h, int stride, const AVIDump::Frame& state, bool swap_upside_down = false); void FinishFrameData(); From 3c65c5f2c53f3efedb3bd94cf1f55ff4f90309c9 Mon Sep 17 00:00:00 2001 From: degasus Date: Fri, 4 Nov 2016 18:24:03 +0100 Subject: [PATCH 5/5] AVIDump: Drop frames which are delayed over a savestate. --- Source/Core/VideoCommon/AVIDump.cpp | 13 ++++++++++++- Source/Core/VideoCommon/AVIDump.h | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Source/Core/VideoCommon/AVIDump.cpp b/Source/Core/VideoCommon/AVIDump.cpp index 1756be97a8..b442afd9fe 100644 --- a/Source/Core/VideoCommon/AVIDump.cpp +++ b/Source/Core/VideoCommon/AVIDump.cpp @@ -46,6 +46,8 @@ static bool s_last_frame_is_valid = false; static bool s_start_dumping = false; static u64 s_last_pts; static int s_file_index = 0; +static int s_savestate_index = 0; +static int s_last_savestate_index = 0; static void InitAVCodec() { @@ -168,6 +170,14 @@ static void PreparePacket(AVPacket* pkt) void AVIDump::AddFrame(const u8* data, int width, int height, int stride, const Frame& state) { + // Assume that the timing is valid, if the savestate id of the new frame + // doesn't match the last one. + if (state.savestate_index != s_last_savestate_index) + { + s_last_savestate_index = state.savestate_index; + s_last_frame_is_valid = false; + } + CheckResolution(width, height); s_src_frame->data[0] = const_cast(data); s_src_frame->linesize[0] = stride; @@ -285,7 +295,7 @@ void AVIDump::CloseFile() void AVIDump::DoState() { - s_last_frame_is_valid = false; + s_savestate_index++; } void AVIDump::CheckResolution(int width, int height) @@ -310,5 +320,6 @@ AVIDump::Frame AVIDump::FetchState(u64 ticks) state.ticks = ticks; state.first_frame = Movie::GetCurrentFrame() < 1; state.ticks_per_second = SystemTimers::GetTicksPerSecond(); + state.savestate_index = s_savestate_index; return state; } diff --git a/Source/Core/VideoCommon/AVIDump.h b/Source/Core/VideoCommon/AVIDump.h index bbea5e2bbf..b9158774a3 100644 --- a/Source/Core/VideoCommon/AVIDump.h +++ b/Source/Core/VideoCommon/AVIDump.h @@ -19,6 +19,7 @@ public: u64 ticks = 0; u32 ticks_per_second = 0; bool first_frame = false; + int savestate_index = 0; }; static bool Start(int w, int h);