Skip to content

Commit

Permalink
Reduce locking scope
Browse files Browse the repository at this point in the history
Speeds up execution by only locking the block that's critical.
Changed the lock from window critical section to std++ mutex and
lockGuard, to obtain cheap RIAA locking. This also addressed a potential
race condition where de(init) of the CS could be triggered in the wrong
order by avisynth.
  • Loading branch information
jojje committed Feb 3, 2019
1 parent 5b2eca3 commit 21f7987
Showing 1 changed file with 18 additions and 18 deletions.
36 changes: 18 additions & 18 deletions SmoothSkip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
// USA.

#include <Windows.h>
#include <thread>
#include <mutex>
#include <stdio.h>
#include "SmoothSkip.h"
#include "CycleCache.h"
Expand All @@ -30,10 +30,9 @@
#define DEBUG 0
#endif

CRITICAL_SECTION lock;

void raiseError(IScriptEnvironment* env, const char* msg);
double GetFps(PClip clip);
std::mutex frameMappingMutex;

// ==========================================================================
// PUBLIC methods
Expand All @@ -42,21 +41,21 @@ double GetFps(PClip clip);
PVideoFrame __stdcall SmoothSkip::GetFrame(int n, IScriptEnvironment* env) {
PVideoFrame frame;

EnterCriticalSection(&lock); // Comparison of the source clip's previous frame is currently done single-threaded.

Cycle &cycle = *cycles->GetCycleForFrame(n);

if (DEBUG) {
auto tid = GetCurrentThreadId();
printf("frame %d, cycle-address %X, thread-id: %X\n", n, (unsigned int)&cycle, GetCurrentThreadId());
}

// Comparison of the source clip's previous frame is currently done single-threaded.
FrameMap map = getFrameMapping(env, n);

// Fetching the alternative clip, or the source clip a second time
// (from avisynth-cache) is done multi-threaded.
int cn = map.srcframe, acn = cn;
bool alt = map.altclip;

LeaveCriticalSection(&lock); // Fetching the alternative clip, or the source clip a second time
// (from avisynth-cache) is done multi-threaded.

if (alt) {
acn = max(cn + offset, 0); // original frame number for alternate clip, offset as specified by user.
acn = min(acn, altclip->GetVideoInfo().num_frames - 1); // ensure the altclip frame to get is 0 <= x <= [last frame number] in alt clip
Expand Down Expand Up @@ -135,12 +134,9 @@ GenericVideoFilter(_child), altclip(_altclip), offset(_offset), debug(_debug) {
vi.num_frames += newFrames;

child->SetCacheHints(CACHE_RANGE, cycleLen);

InitializeCriticalSection(&lock);
}

SmoothSkip::~SmoothSkip() {
DeleteCriticalSection(&lock);
delete cycles;
}

Expand Down Expand Up @@ -178,17 +174,21 @@ FrameMap SmoothSkip::getFrameMapping(IScriptEnvironment* env, int n) {
Cycle& cycle = *cycles->GetCycleForFrame(n);
int cycleCount = n / (cycle.length + cycle.creates);
int cycleOffset = n % (cycle.length + cycle.creates);
int ccsf = cycleCount * cycle.length; // child cycle start frame

if (!cycle.includes(ccsf)) { // cycle boundary crossed, so update the cycle info.
if (DEBUG) {
printf("Frame %d not in cycle, updating!\n", n);
int ccsf = cycleCount * cycle.length; // Child cycle start frame

{
std::lock_guard<std::mutex> lockGuard(frameMappingMutex); // Ensure only one thread updates the frame map at a time to optimize disk I/O and Avisynth cache use.
if (!cycle.includes(ccsf)) { // Cycle stats have not been computed, so try to update the cycle.
if (DEBUG) {
printf("Frame %d not in cycle, updating!\n", n);
}
updateCycle(env, ccsf, child->GetVideoInfo(), cycle);
}
updateCycle(env, ccsf, child->GetVideoInfo(), cycle);
}

FrameMap map = cycle.frameMap[cycleOffset];
if (map.dstframe != n) raiseError(env, "Frame counting is out of whack");
if (map.dstframe != n)
raiseError(env, "Frame counting is out of whack");

return map;
}
Expand Down

0 comments on commit 21f7987

Please sign in to comment.