Skip to content

Commit

Permalink
Fix hang on EVR when seeking
Browse files Browse the repository at this point in the history 
Fix crash in VapourSynth frame handler
  • Loading branch information
@CrendKing
CrendKing committed Aug 14, 2022
1 parent e62dc83 commit 579c15b
Show file tree
Hide file tree
Showing 11 changed files with 73 additions and 81 deletions.
22 changes: 10 additions & 12 deletions avisynth_filter/src/frame_handler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,15 @@ auto FrameHandler::AddInputSample(IMediaSample *inputSample) -> HRESULT {
inputSampleStartTime = _nextSourceFrameNb * MainFrameServer::GetInstance().GetSourceAvgFrameDuration();
}

// since the key of _sourceFrames is frame number, which only strictly increases, rbegin() returns the last emplaced frame
if (const REFERENCE_TIME lastSampleStartTime = _sourceFrames.empty() ? -1 : _sourceFrames.rbegin()->second.startTime;
inputSampleStartTime <= lastSampleStartTime) {
Environment::GetInstance().Log(L"Reject input sample due to start time going backward: curr %10lld last %10lld", inputSampleStartTime, lastSampleStartTime);
return S_FALSE;
{
const std::shared_lock sharedSourceLock(_sourceMutex);

// since the key of _sourceFrames is frame number, which only strictly increases, rbegin() returns the last emplaced frame
if (const REFERENCE_TIME lastSampleStartTime = _sourceFrames.empty() ? -1 : _sourceFrames.rbegin()->second.startTime;
inputSampleStartTime <= lastSampleStartTime) {
Environment::GetInstance().Log(L"Reject input sample due to start time going backward: curr %10lld last %10lld", inputSampleStartTime, lastSampleStartTime);
return S_FALSE;
}
}

RefreshInputFrameRates(_nextSourceFrameNb);
Expand Down Expand Up @@ -177,18 +181,12 @@ auto FrameHandler::BeginFlush() -> void {
_addInputSampleCv.notify_all();
_newSourceFrameCv.notify_all();

_isWorkerLatched.wait(false);

Environment::GetInstance().Log(L"FrameHandler finish BeginFlush()");
}

auto FrameHandler::EndFlush(const std::function<void ()> &interim) -> void {
auto FrameHandler::EndFlush() -> void {
Environment::GetInstance().Log(L"FrameHandler start EndFlush()");

if (interim) {
interim();
}

ResetInput();

_isFlushing = false;
Expand Down
3 changes: 2 additions & 1 deletion avisynth_filter/src/frame_handler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ class FrameHandler {
auto AddInputSample(IMediaSample *inputSample) -> HRESULT;
auto GetSourceFrame(int frameNb) -> PVideoFrame;
auto BeginFlush() -> void;
auto EndFlush(const std::function<void ()> &interim) -> void;
auto EndFlush() -> void;
auto StartWorker() -> void;
auto WaitForWorkerLatch() -> void;
auto GetInputBufferSize() const -> int;
constexpr auto GetSourceFrameNb() const -> int { return _nextSourceFrameNb; }
constexpr auto GetOutputFrameNb() const -> int { return _nextOutputFrameNb; }
Expand Down
32 changes: 29 additions & 3 deletions filter_common/src/filter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@ auto CSynthFilter::CompleteConnect(PIN_DIRECTION direction, IPin *pReceivePin) -
isMediaTypesCompatible = true;
TraverseFiltersInGraph();
frameHandler->StartWorker();

if (Environment::GetInstance().IsRemoteControlEnabled()) {
_remoteControl->Start();
}

break;
}

Expand Down Expand Up @@ -292,6 +297,17 @@ auto CSynthFilter::CompleteConnect(PIN_DIRECTION direction, IPin *pReceivePin) -
return __super::CompleteConnect(direction, pReceivePin);
}

auto CSynthFilter::StartStreaming() -> HRESULT {
AuxFrameServer::GetInstance().ReloadScript(m_pInput->CurrentMediaType(), true);
_inputVideoFormat = Format::GetVideoFormat(m_pInput->CurrentMediaType(), &AuxFrameServer::GetInstance());
_outputVideoFormat = Format::GetVideoFormat(m_pOutput->CurrentMediaType(), &AuxFrameServer::GetInstance());

// the paired BeginFlush() is in StopStreaming()
frameHandler->EndFlush();

return __super::StartStreaming();
}

auto CSynthFilter::Receive(IMediaSample *pSample) -> HRESULT {
HRESULT hr;

Expand Down Expand Up @@ -360,14 +376,24 @@ auto CSynthFilter::BeginFlush() -> HRESULT {

auto CSynthFilter::EndFlush() -> HRESULT {
if (IsActive()) {
frameHandler->EndFlush([]() -> void {
MainFrameServer::GetInstance().StopScript();
});
frameHandler->WaitForWorkerLatch();
MainFrameServer::GetInstance().StopScript();
frameHandler->EndFlush();
}

return __super::EndFlush();
}

auto CSynthFilter::StopStreaming() -> HRESULT {
frameHandler->BeginFlush();
frameHandler->WaitForWorkerLatch();
MainFrameServer::GetInstance().StopScript();

// keep flushing until start streaming

return __super::StopStreaming();
}

auto STDMETHODCALLTYPE CSynthFilter::GetPages(__RPC__out CAUUID *pPages) -> HRESULT {
CheckPointer(pPages, E_POINTER);

Expand Down
2 changes: 2 additions & 0 deletions filter_common/src/filter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ class
auto CheckTransform(const CMediaType *mtIn, const CMediaType *mtOut) -> HRESULT override;
auto DecideBufferSize(IMemAllocator *pAlloc, ALLOCATOR_PROPERTIES *pProperties) -> HRESULT override;
auto CompleteConnect(PIN_DIRECTION direction, IPin *pReceivePin) -> HRESULT override;
auto StartStreaming() -> HRESULT override;
auto Receive(IMediaSample *pSample) -> HRESULT override;
auto BeginFlush() -> HRESULT override;
auto EndFlush() -> HRESULT override;
auto StopStreaming() -> HRESULT override;

// ISpecifyPropertyPages
auto STDMETHODCALLTYPE GetPages(__RPC__out CAUUID *pPages) -> HRESULT override;
Expand Down
24 changes: 11 additions & 13 deletions filter_common/src/frame_handler_common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ FrameHandler::~FrameHandler() {
if (_workerThread.joinable()) {
_isStopping = true;

// the pairing BeginFlush() is in input pin's Inactive()
EndFlush(nullptr);
// the paired BeginFlush() is in StopStreaming()
WaitForWorkerLatch();
EndFlush();

_workerThread.join();
}
Expand All @@ -29,6 +30,10 @@ auto FrameHandler::StartWorker() -> void {
}
}

auto FrameHandler::WaitForWorkerLatch() -> void {
_isWorkerLatched.wait(false);
}

auto FrameHandler::UpdateExtraSrcBuffer() -> void {
if (const int sourceAvgFps = MainFrameServer::GetInstance().GetSourceAvgFrameRate();
_nextSourceFrameNb % (sourceAvgFps / FRAME_RATE_SCALE_FACTOR) == 0) {
Expand Down Expand Up @@ -90,31 +95,24 @@ auto FrameHandler::ChangeOutputFormat() -> bool {

_filter.StopStreaming();

BeginFlush();
EndFlush([this]() -> void {
AuxFrameServer::GetInstance().ReloadScript(_filter.m_pInput->CurrentMediaType(), true);
});

_filter._isInputMediaTypeChanged = false;
_filter._needReloadScript = false;

AuxFrameServer::GetInstance().ReloadScript(_filter.m_pInput->CurrentMediaType(), true);
auto potentialOutputMediaTypes = _filter.InputToOutputMediaType(&_filter.m_pInput->CurrentMediaType());
if (const auto newOutputMediaTypeIter = std::ranges::find_if(potentialOutputMediaTypes, [this](const CMediaType &outputMediaType) -> bool {

if (const auto newOutputMediaTypeIter = std::ranges::find_if(potentialOutputMediaTypes,
[this](const CMediaType &outputMediaType) -> bool {
/*
* "QueryAccept (Downstream)" forces the downstream to use the new output media type as-is, which may lead to wrong rendering result
* "ReceiveConnection" allows downstream to counter-propose suitable media type for the connection
* after ReceiveConnection(), the next output sample should carry the new output media type, which is handled in PrepareOutputSample()
*/

if (_isFlushing) {
return false;
}

const bool result = SUCCEEDED(_filter.m_pOutput->GetConnected()->ReceiveConnection(_filter.m_pOutput, &outputMediaType));
Environment::GetInstance().Log(L"Attempt to reconnect output pin with media type: output %s result %d",
Format::LookupMediaSubtype(outputMediaType.subtype)->name,
result);

if (result) {
_filter.m_pOutput->SetMediaType(&outputMediaType);
_filter._outputVideoFormat = Format::GetVideoFormat(outputMediaType, &AuxFrameServer::GetInstance());
Expand Down
22 changes: 0 additions & 22 deletions filter_common/src/input_pin.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,26 +72,4 @@ auto STDMETHODCALLTYPE CSynthFilterInputPin::GetAllocator(__deref_out IMemAlloca
return S_OK;
}

auto CSynthFilterInputPin::Active() -> HRESULT {
AuxFrameServer::GetInstance().ReloadScript(CurrentMediaType(), true);
_filter._inputVideoFormat = Format::GetVideoFormat(CurrentMediaType(), &AuxFrameServer::GetInstance());
_filter._outputVideoFormat = Format::GetVideoFormat(_filter.m_pOutput->CurrentMediaType(), &AuxFrameServer::GetInstance());

if (Environment::GetInstance().IsRemoteControlEnabled()) {
_filter._remoteControl->Start();
}

_filter.frameHandler->EndFlush(nullptr);

return S_OK;
}

auto CSynthFilterInputPin::Inactive() -> HRESULT {
_filter.frameHandler->BeginFlush();
MainFrameServer::GetInstance().StopScript();
// keep flushing until Active()

return __super::Inactive();
}

}
5 changes: 0 additions & 5 deletions filter_common/src/input_pin.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ class CSynthFilterInputPin : public CTransformInputPin {

auto STDMETHODCALLTYPE ReceiveConnection(IPin *pConnector, const AM_MEDIA_TYPE *pmt) -> HRESULT override;
auto STDMETHODCALLTYPE GetAllocator(__deref_out IMemAllocator **ppAllocator) -> HRESULT override;
auto Active() -> HRESULT override;
auto Inactive() -> HRESULT override;

private:
CSynthFilter &_filter = reinterpret_cast<CSynthFilter &>(*m_pFilter);
};

}
4 changes: 0 additions & 4 deletions filter_common/src/remote_control.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ auto RemoteControl::Start() -> void {
}
}

auto RemoteControl::IsRunning() const -> bool {
return _msgThread.joinable();
}

auto CALLBACK RemoteControl::WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) -> LRESULT {
switch (uMsg) {
case WM_COPYDATA: {
Expand Down
1 change: 0 additions & 1 deletion filter_common/src/remote_control.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ class RemoteControl {
DISABLE_COPYING(RemoteControl)

auto Start() -> void;
auto IsRunning() const -> bool;

private:
static auto CALLBACK WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) -> LRESULT;
Expand Down
36 changes: 17 additions & 19 deletions vapoursynth_filter/src/frame_handler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,15 @@ auto FrameHandler::AddInputSample(IMediaSample *inputSample) -> HRESULT {
inputSampleStartTime = _nextSourceFrameNb * MainFrameServer::GetInstance().GetSourceAvgFrameDuration();
}

// since the key of _sourceFrames is frame number, which only strictly increases, rbegin() returns the last emplaced frame
if (const REFERENCE_TIME lastSampleStartTime = _sourceFrames.empty() ? -1 : _sourceFrames.rbegin()->second.startTime;
inputSampleStartTime <= lastSampleStartTime) {
Environment::GetInstance().Log(L"Reject input sample due to start time going backward: curr %10lld last %10lld", inputSampleStartTime, lastSampleStartTime);
return S_FALSE;
{
const std::shared_lock sharedSourceLock(_sourceMutex);

// since the key of _sourceFrames is frame number, which only strictly increases, rbegin() returns the last emplaced frame
if (const REFERENCE_TIME lastSampleStartTime = _sourceFrames.empty() ? -1 : _sourceFrames.rbegin()->second.startTime;
inputSampleStartTime <= lastSampleStartTime) {
Environment::GetInstance().Log(L"Reject input sample due to start time going backward: curr %10lld last %10lld", inputSampleStartTime, lastSampleStartTime);
return S_FALSE;
}
}

RefreshInputFrameRates(_nextSourceFrameNb);
Expand Down Expand Up @@ -170,7 +174,7 @@ auto FrameHandler::AddInputSample(IMediaSample *inputSample) -> HRESULT {
// any pending request to finish before destroying the script

{
std::unique_lock uniqueOutputLock(_outputMutex);
const std::unique_lock uniqueOutputLock(_outputMutex);

_outputFrames.emplace(_nextOutputFrameNb, nullptr);
}
Expand Down Expand Up @@ -225,12 +229,10 @@ auto FrameHandler::BeginFlush() -> void {
_newSourceFrameCv.notify_all();
_deliverSampleCv.notify_all();

_isWorkerLatched.wait(false);

Environment::GetInstance().Log(L"FrameHandler finish BeginFlush()");
}

auto FrameHandler::EndFlush(const std::function<void ()> &interim) -> void {
auto FrameHandler::EndFlush() -> void {
Environment::GetInstance().Log(L"FrameHandler start EndFlush()");

{
Expand All @@ -242,13 +244,9 @@ auto FrameHandler::EndFlush(const std::function<void ()> &interim) -> void {
}, &AutoReleaseVSFrame::frame);
});
}

if (interim) {
interim();
}
_outputFrames.clear();

ResetInput();
_outputFrames.clear();

_isFlushing = false;
_isFlushing.notify_all();
Expand All @@ -267,21 +265,21 @@ auto VS_CC FrameHandler::VpsGetFrameCallback(void *userData, const VSFrame *f, i

if (frameHandler->_isFlushing) {
{
std::unique_lock uniqueOutputLock(frameHandler->_outputMutex);
const std::unique_lock uniqueOutputLock(frameHandler->_outputMutex);

frameHandler->_outputFrames.erase(n);
}
frameHandler->_flushOutputSampleCv.notify_all();

AVSF_VPS_API->freeFrame(f);
} else {
{
std::shared_lock sharedOutputLock(frameHandler->_outputMutex);
const std::unique_lock uniqueOutputLock(frameHandler->_outputMutex);

frameHandler->_outputFrames[n] = const_cast<VSFrame *>(f);
}
frameHandler->_deliverSampleCv.notify_all();
}

frameHandler->_flushOutputSampleCv.notify_all();
}

auto FrameHandler::ResetInput() -> void {
Expand Down Expand Up @@ -470,7 +468,7 @@ auto FrameHandler::WorkerProc() -> void {
}

{
std::unique_lock uniqueOutputLock(_outputMutex);
const std::unique_lock uniqueOutputLock(_outputMutex);

_outputFrames.erase(iter);
}
Expand Down
3 changes: 2 additions & 1 deletion vapoursynth_filter/src/frame_handler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ class FrameHandler {
auto AddInputSample(IMediaSample *inputSample) -> HRESULT;
auto GetSourceFrame(int frameNb) -> const VSFrame *;
auto BeginFlush() -> void;
auto EndFlush(const std::function<void ()> &interim) -> void;
auto EndFlush() -> void;
auto StartWorker() -> void;
auto WaitForWorkerLatch() -> void;
auto GetInputBufferSize() const -> int;
constexpr auto GetSourceFrameNb() const -> int { return _nextSourceFrameNb; }
constexpr auto GetOutputFrameNb() const -> int { return _nextOutputFrameNb; }
Expand Down

2 comments on commit 579c15b

@chainikdn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a reason why _remoteControl->Start() called after _inputVideoFormat initialization. Please move _remoteControl->Start() into CSynthFilter::StartStreaming(), since you moved _inputVideoFormat initialization there.

@CrendKing
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I added a comment so this should not happen again. d6c205f

Please sign in to comment.