Hi David, change now merged.  Cheers, Robert.

On Mon, Jul 26, 2010 at 5:21 PM, David Fries <[email protected]> wrote:
> ffmpeg, improve wait for close logic
>
> Both audio and video destructors have been succesfully using the logic,
> if(isRunning())
> {
>   m_exit = true;
>   join();
> }
> since it was introduced,
>
> but the close routines are using,
> m_exit = true;
> if(isRunning() && waitForThreadToExit)
> {
>   while(isRunning()) { OpenThreads::Thread::YieldCurrentThread(); }
> }
> which not only is it doing an unnecessary busy wait, but it doesn't
> guaranteed that the other thread has terminated, just that it has
> progressed far enough that OpenThreads has set the thread status as
> not running.  Like the destructor set the m_exit after checking
> isRunning() to avoid the race condition of not getting to join()
> because the thread was running, but isRunning() returns false.
>
> Now that FFmpeg*close is fixed, call it from the destructor as well
> to have that code in only one location.
> ---
>  src/osgPlugins/ffmpeg/FFmpegDecoderAudio.cpp |   18 +++++-------------
>  src/osgPlugins/ffmpeg/FFmpegDecoderVideo.cpp |   19 +++++--------------
>  2 files changed, 10 insertions(+), 27 deletions(-)
>
> diff --git a/src/osgPlugins/ffmpeg/FFmpegDecoderAudio.cpp 
> b/src/osgPlugins/ffmpeg/FFmpegDecoderAudio.cpp
> index 769bda1..b738a3b 100644
> --- a/src/osgPlugins/ffmpeg/FFmpegDecoderAudio.cpp
> +++ b/src/osgPlugins/ffmpeg/FFmpegDecoderAudio.cpp
> @@ -54,15 +54,7 @@ FFmpegDecoderAudio::FFmpegDecoderAudio(PacketQueue & 
> packets, FFmpegClocks & clo
>
>  FFmpegDecoderAudio::~FFmpegDecoderAudio()
>  {
> -    if (isRunning())
> -    {
> -        m_exit = true;
> -#if 0
> -        while(isRunning()) { OpenThreads::YieldCurrentThread(); }
> -#else
> -        join();
> -#endif
> -    }
> +    this->close(true);
>  }
>
>
> @@ -123,11 +115,11 @@ void FFmpegDecoderAudio::pause(bool pause)
>
>  void FFmpegDecoderAudio::close(bool waitForThreadToExit)
>  {
> -    m_exit = true;
> -
> -    if (isRunning() && waitForThreadToExit)
> +    if (isRunning())
>     {
> -        while(isRunning()) { OpenThreads::Thread::YieldCurrentThread(); }
> +        m_exit = true;
> +        if (waitForThreadToExit)
> +            join();
>     }
>  }
>
> diff --git a/src/osgPlugins/ffmpeg/FFmpegDecoderVideo.cpp 
> b/src/osgPlugins/ffmpeg/FFmpegDecoderVideo.cpp
> index eed5825..e2bc11e 100644
> --- a/src/osgPlugins/ffmpeg/FFmpegDecoderVideo.cpp
> +++ b/src/osgPlugins/ffmpeg/FFmpegDecoderVideo.cpp
> @@ -57,17 +57,8 @@ FFmpegDecoderVideo::~FFmpegDecoderVideo()
>  {
>     OSG_INFO<<"Destructing FFmpegDecoderVideo..."<<std::endl;
>
> +    this->close(true);
>
> -    if (isRunning())
> -    {
> -        m_exit = true;
> -#if 0
> -        while(isRunning()) { OpenThreads::YieldCurrentThread(); }
> -#else
> -        join();
> -#endif
> -    }
> -
>  #ifdef USE_SWSCALE
>     if (m_swscale_ctx)
>     {
> @@ -132,11 +123,11 @@ void FFmpegDecoderVideo::open(AVStream * const stream)
>
>  void FFmpegDecoderVideo::close(bool waitForThreadToExit)
>  {
> -    m_exit = true;
> -
> -    if (isRunning() && waitForThreadToExit)
> +    if (isRunning())
>     {
> -        while(isRunning()) { OpenThreads::Thread::YieldCurrentThread(); }
> +        m_exit = true;
> +        if (waitForThreadToExit)
> +            join();
>     }
>  }
>
> --
> 1.7.0
>
>
> _______________________________________________
> osg-submissions mailing list
> [email protected]
> http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
>
>
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org

Reply via email to