[vlmc-devel] [PATCH] Ensure Max Buffer Size after the unlock function in ClipWorkflow

[Hugo Beauzée-Luyssen]

On 04/02/2016 07:04 AM, yikei lu wrote:
> On second thought, that happens when only ( the source renderer of )
> WokflowRenderer dies. If it is alive, setPause( false ) is called in
> getOutput so there is no problem in ClipWorkflow. So that patch will
> be fine ( though I didn't figure out why the workflowRenderer dies
> sometimes... )
>
> 2016-04-02 13:58 GMT+09:00 yikei lu <luyikei.qmltu at gmail.com>:
>> Sorry this patch has a problem: when the renderer stops by
>> limitBufferSize() in unlock, it never gets up again! I'll send another
>> patch...
>>
>> 2016-04-02 13:47 GMT+09:00 Yikai Lu <luyikei.qmltu at gmail.com>:
>>> Currently, we only check the buffer size in "getOutput" function; however, it is called by the source renderer in the WorkflowRenderer. So if the source renderer hangs for some reason, it's highly likely that m_computedBuffers will overflow.
>>> ---
>>>   src/Workflow/AudioClipWorkflow.cpp | 1 +
>>>   src/Workflow/ClipWorkflow.cpp      | 8 +++++++-
>>>   src/Workflow/ClipWorkflow.h        | 1 +
>>>   src/Workflow/VideoClipWorkflow.cpp | 1 +
>>>   4 files changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/Workflow/AudioClipWorkflow.cpp b/src/Workflow/AudioClipWorkflow.cpp
>>> index 1eb2698..3b08877 100644
>>> --- a/src/Workflow/AudioClipWorkflow.cpp
>>> +++ b/src/Workflow/AudioClipWorkflow.cpp
>>> @@ -172,6 +172,7 @@ AudioClipWorkflow::unlock( void* data, uint8_t *pcm_buffer, unsigned int channel
>>>           else
>>>               cw->m_currentPts = pts;
>>>       }
>>> +    cw->limitBufferSize();
>>>       cw->m_renderLock->unlock();
>>>   }
>>>
>>> diff --git a/src/Workflow/ClipWorkflow.cpp b/src/Workflow/ClipWorkflow.cpp
>>> index d5e5432..8da3646 100644
>>> --- a/src/Workflow/ClipWorkflow.cpp
>>> +++ b/src/Workflow/ClipWorkflow.cpp
>>> @@ -155,7 +155,7 @@ ClipWorkflow::waitForCompleteInit()
>>>   }
>>>
>>>   void
>>> -ClipWorkflow::postGetOutput()
>>> +ClipWorkflow::limitBufferSize()
>>>   {
>>>       //If we're running out of computed buffers, refill our stack.
>>>       if ( getNbComputedBuffers() < getMaxComputedBuffers() / 3 )
>>> @@ -171,6 +171,12 @@ ClipWorkflow::postGetOutput()
>>>   }
>>>
>>>   void
>>> +ClipWorkflow::postGetOutput()
>>> +{
>>> +    limitBufferSize();
>>> +}
>>> +
>>> +void
>>>   ClipWorkflow::computePtsDiff( qint64 pts )
>>>   {
>>>       if ( m_pauseDuration != -1 )
>>> diff --git a/src/Workflow/ClipWorkflow.h b/src/Workflow/ClipWorkflow.h
>>> index cdac341..19a6823 100644
>>> --- a/src/Workflow/ClipWorkflow.h
>>> +++ b/src/Workflow/ClipWorkflow.h
>>> @@ -98,6 +98,7 @@ class   ClipWorkflow : public EffectUser
>>>            *  of the rendering process advancement.
>>>            */
>>>           virtual Workflow::OutputBuffer      *getOutput( ClipWorkflow::GetMode mode, qint64 currentFrame ) = 0;
>>> +        void                    limitBufferSize();
>>>           void                    postGetOutput();
>>>           /**
>>>            * @brief Initialize base variables for the SourceRenderer.
>>> diff --git a/src/Workflow/VideoClipWorkflow.cpp b/src/Workflow/VideoClipWorkflow.cpp
>>> index 46b0650..39765f9 100644
>>> --- a/src/Workflow/VideoClipWorkflow.cpp
>>> +++ b/src/Workflow/VideoClipWorkflow.cpp
>>> @@ -163,6 +163,7 @@ VideoClipWorkflow::unlock( void *data, uint8_t *buffer, int width,
>>>       Workflow::Frame     *frame = cw->m_computedBuffers.last();
>>>       frame->ptsDiff = cw->m_currentPts - cw->m_previousPts;
>>>       cw->m_renderWaitCond->wakeAll();
>>> +    cw->limitBufferSize();
>>>       cw->m_renderLock->unlock();
>>>   }
>>>
>>> --
>>> 1.9.1
>>>
> _______________________________________________
> Vlmc-devel mailing list
> Vlmc-devel at videolan.org
> https://mailman.videolan.org/listinfo/vlmc-devel
>

The fact that the rendering (ie. WorkflowRenderer, which in the 
background is a libvlc_media_player_t using the imem access/demux 
module) dies is an issue indeed. However, pausing from the ClipWorkflow 
thread (which in the background is a libvlc_media_player_t with 
transcode & smem modules) is a bad idea, and can lead to deadlocks 
(which was the reasoning behind moving all controlling (stop/pause) to 
the WorkflowRenderer thread)
If we really want a failsafe, I'd be more in favor of emiting a signal 
and letting the main thread handle things as a quick way of doing it.
We could also consider the idea of having our own rendering thread, and 
having the WorkflowRenderer just poping a frame/audio sample from a 
queue that would be filled by the rendering thread.

Regards,