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

[Hugo Beauzée-Luyssen]

On 04/02/2016 12:07 PM, Yikai Lu wrote:
> 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 | 2 ++
>   src/Workflow/ClipWorkflow.cpp      | 8 ++++++++
>   src/Workflow/ClipWorkflow.h        | 7 +++++++
>   src/Workflow/VideoClipWorkflow.cpp | 2 ++
>   4 files changed, 19 insertions(+)
>
> diff --git a/src/Workflow/AudioClipWorkflow.cpp b/src/Workflow/AudioClipWorkflow.cpp
> index 1eb2698..e5e41b8 100644
> --- a/src/Workflow/AudioClipWorkflow.cpp
> +++ b/src/Workflow/AudioClipWorkflow.cpp
> @@ -172,6 +172,8 @@ AudioClipWorkflow::unlock( void* data, uint8_t *pcm_buffer, unsigned int channel
>           else
>               cw->m_currentPts = pts;
>       }
> +    if ( cw->getMaxComputedBuffers() <= cw->getNbComputedBuffers() )
> +        emit cw->bufferReachedMax();
>       cw->m_renderLock->unlock();
>   }
>
> diff --git a/src/Workflow/ClipWorkflow.cpp b/src/Workflow/ClipWorkflow.cpp
> index d5e5432..0c51aaa 100644
> --- a/src/Workflow/ClipWorkflow.cpp
> +++ b/src/Workflow/ClipWorkflow.cpp
> @@ -84,6 +84,7 @@ ClipWorkflow::initialize()
>       connect( m_eventWatcher, SIGNAL( endReached() ), this, SLOT( clipEndReached() ), Qt::DirectConnection );
>       connect( m_eventWatcher, SIGNAL( errorEncountered() ), this, SLOT( errorEncountered() ) );
>       connect( m_eventWatcher, &RendererEventWatcher::stopped, this, &ClipWorkflow::mediaPlayerStopped );
> +    connect( this, &ClipWorkflow::bufferReachedMax, this, &ClipWorkflow::pause );

I'd feel safer if this was connected using a queued connection

>       m_renderer->start();
>   }
>
> @@ -131,6 +132,13 @@ ClipWorkflow::stop()
>   }
>
>   void
> +ClipWorkflow::pause()
> +{
> +    if ( m_renderer != nullptr )
> +        m_renderer->setPause( true );

We probably should print a warning here, since this it not an expected 
behavior.

> +}
> +
> +void
>   ClipWorkflow::setTime( qint64 time )
>   {
>       vlmcDebug() << "Setting ClipWorkflow" << m_clipHelper->uuid() << "time:" << time;
> diff --git a/src/Workflow/ClipWorkflow.h b/src/Workflow/ClipWorkflow.h
> index cdac341..98658be 100644
> --- a/src/Workflow/ClipWorkflow.h
> +++ b/src/Workflow/ClipWorkflow.h
> @@ -140,6 +140,12 @@ class   ClipWorkflow : public EffectUser
>            *  \brief  Stop this workflow.
>            */
>           void                    stop();
> +
> +        /**
> +         *  \brief  Pause this workflow.
> +         */
> +        void                    pause();
> +
>           /**
>            *  \brief  Set the rendering position
>            *  \param  time    The position in millisecond
> @@ -243,6 +249,7 @@ class   ClipWorkflow : public EffectUser
>
>       signals:
>           void                    error( ClipWorkflow* );
> +        void                    bufferReachedMax();
>   };
>
>   #endif // CLIPWORKFLOW_H
> diff --git a/src/Workflow/VideoClipWorkflow.cpp b/src/Workflow/VideoClipWorkflow.cpp
> index 46b0650..482ee2e 100644
> --- a/src/Workflow/VideoClipWorkflow.cpp
> +++ b/src/Workflow/VideoClipWorkflow.cpp
> @@ -163,6 +163,8 @@ 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();
> +    if ( cw->getMaxComputedBuffers() <= cw->getNbComputedBuffers() )
> +        emit cw->bufferReachedMax();
>       cw->m_renderLock->unlock();
>   }
>
>

LGTM otherwise!