gnash-commit
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Gnash-commit] gnash ChangeLog server/asobj/NetStream.cpp serv... [gnash


From: Bastiaan Jacques
Subject: [Gnash-commit] gnash ChangeLog server/asobj/NetStream.cpp serv... [gnash_0_8_3_branch]
Date: Tue, 20 May 2008 08:02:55 +0000

CVSROOT:        /sources/gnash
Module name:    gnash
Branch:         gnash_0_8_3_branch
Changes by:     Bastiaan Jacques <bjacques>     08/05/20 08:02:54

Modified files:
        .              : ChangeLog 
        server/asobj   : NetStream.cpp NetStream.h NetStreamFfmpeg.cpp 
                         NetStreamFfmpeg.h 

Log message:
                * server/asobj/NetStream.{h,cpp}: Remove the m_go and m_pause 
status
                variables, because their semantics are ill-defined. Extend 
imagemutex
                to the m_newFrameReady flagging boolean so that it doesn't need 
to be
                volatile.
                * server/asobj/NetStreamFfmpeg.{h,cpp}: No longer depend on
                NetConnection to download URLs. This allows two NetStreams to 
share
                one Netconnection object. It also fixes a segmentation fault. 
m_go and
                m_pause are replaced by playbackStatus() and decodingStatus
                mutex-protected methods, resolving ambiguity. updateVideoFrame 
no
                longer relies on the presence of audio in order to show video 
frames 
                (fixing bug #22400 and #22439). These changes reduce the 
potential
                race conditions that Helgrind detects from +/- 90 to three on a 
short
                movie.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&only_with_tag=gnash_0_8_3_branch&r1=1.6573.2.25&r2=1.6573.2.26
http://cvs.savannah.gnu.org/viewcvs/gnash/server/asobj/NetStream.cpp?cvsroot=gnash&only_with_tag=gnash_0_8_3_branch&r1=1.89.2.2&r2=1.89.2.3
http://cvs.savannah.gnu.org/viewcvs/gnash/server/asobj/NetStream.h?cvsroot=gnash&only_with_tag=gnash_0_8_3_branch&r1=1.61.2.2&r2=1.61.2.3
http://cvs.savannah.gnu.org/viewcvs/gnash/server/asobj/NetStreamFfmpeg.cpp?cvsroot=gnash&only_with_tag=gnash_0_8_3_branch&r1=1.116.2.2&r2=1.116.2.3
http://cvs.savannah.gnu.org/viewcvs/gnash/server/asobj/NetStreamFfmpeg.h?cvsroot=gnash&only_with_tag=gnash_0_8_3_branch&r1=1.60&r2=1.60.2.1

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.6573.2.25
retrieving revision 1.6573.2.26
diff -u -b -r1.6573.2.25 -r1.6573.2.26
--- ChangeLog   19 May 2008 18:26:54 -0000      1.6573.2.25
+++ ChangeLog   20 May 2008 08:02:53 -0000      1.6573.2.26
@@ -1,3 +1,19 @@
+2008-05-19 Bastiaan Jacques <address@hidden>   
+
+       * server/asobj/NetStream.{h,cpp}: Remove the m_go and m_pause status
+       variables, because their semantics are ill-defined. Extend imagemutex
+       to the m_newFrameReady flagging boolean so that it doesn't need to be
+       volatile.
+       * server/asobj/NetStreamFfmpeg.{h,cpp}: No longer depend on
+       NetConnection to download URLs. This allows two NetStreams to share
+       one Netconnection object. It also fixes a segmentation fault. m_go and
+       m_pause are replaced by playbackStatus() and decodingStatus
+       mutex-protected methods, resolving ambiguity. updateVideoFrame no
+       longer relies on the presence of audio in order to show video frames
+       (fixing bug #22400 and #22439). These changes reduce the potential
+       race conditions that Helgrind detects from +/- 90 to three on a short
+       movie.
+
 2008-05-19 Benjamin Wolsey <address@hidden>
 
         * po/Makefile.am, po/ja.po: add Japanese translation by

Index: server/asobj/NetStream.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/asobj/NetStream.cpp,v
retrieving revision 1.89.2.2
retrieving revision 1.89.2.3
diff -u -b -r1.89.2.2 -r1.89.2.3
--- server/asobj/NetStream.cpp  15 May 2008 08:25:55 -0000      1.89.2.2
+++ server/asobj/NetStream.cpp  20 May 2008 08:02:54 -0000      1.89.2.3
@@ -71,9 +71,7 @@
        m_bufferTime(100), // The default size needed to begin playback of 
media is 100 miliseconds
        m_videoFrameFormat(gnash::render::videoFrameFormat()),
        m_newFrameReady(false),
-       m_go(false),
        m_imageframe(NULL),
-       m_pause(false),
        m_parser(NULL),
        m_isFLV(false),
        m_start_onbuffer(false),
@@ -541,6 +539,8 @@
 bool
 NetStream::newFrameReady()
 {
+       boost::mutex::scoped_lock lock(image_mutex);
+
        if (m_newFrameReady) {
                m_newFrameReady = false;
                return true;

Index: server/asobj/NetStream.h
===================================================================
RCS file: /sources/gnash/gnash/server/asobj/NetStream.h,v
retrieving revision 1.61.2.2
retrieving revision 1.61.2.3
diff -u -b -r1.61.2.2 -r1.61.2.3
--- server/asobj/NetStream.h    15 May 2008 08:25:56 -0000      1.61.2.2
+++ server/asobj/NetStream.h    20 May 2008 08:02:54 -0000      1.61.2.3
@@ -131,20 +131,14 @@
        int m_videoFrameFormat;
 
        // Are a new frame ready to be returned?
-       volatile bool m_newFrameReady;
+       bool m_newFrameReady;
 
        // Mutex to insure we don't corrupt the image
        boost::mutex image_mutex;
 
-       // Are the playing loop running or not
-       volatile bool m_go;
-
        // The image/videoframe which is given to the renderer
        image::image_base* m_imageframe;
 
-       // paused or not
-       volatile bool m_pause;
-
        // The video URL
        std::string url;
 

Index: server/asobj/NetStreamFfmpeg.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/asobj/NetStreamFfmpeg.cpp,v
retrieving revision 1.116.2.2
retrieving revision 1.116.2.3
diff -u -b -r1.116.2.2 -r1.116.2.3
--- server/asobj/NetStreamFfmpeg.cpp    13 May 2008 23:57:41 -0000      
1.116.2.2
+++ server/asobj/NetStreamFfmpeg.cpp    20 May 2008 08:02:54 -0000      
1.116.2.3
@@ -33,6 +33,8 @@
 #include "sound_handler.h"
 #include "VideoDecoderFfmpeg.h"
 #include "ClockTime.h" // TODO: use the VirtualClock instead ?
+#include "URLAccessManager.h"
+#include "URL.h"
 
 #include "FLVParser.h" 
 
@@ -63,6 +65,8 @@
 
 
 NetStreamFfmpeg::NetStreamFfmpeg():
+       _playback_state(PLAY_NONE),
+       _decoding_state(DEC_NONE),
        m_video_index(-1),
        m_audio_index(-1),
 
@@ -96,7 +100,7 @@
 {
   switch ( mode ) {
     case pauseModeToggle:
-                       if ( m_pause ) {
+                       if ( playbackStatus() == PLAY_PAUSED ) {
                          unpausePlayback();
                        } else {
                          pausePlayback();
@@ -112,9 +116,9 @@
                        break;
   }
 
-  if ( !m_pause && !m_go ) {
+  if ( playbackStatus() == PLAY_NONE && decodingStatus() == DEC_NONE) {
     setStatus( playStart );
-    m_go = true;
+    decodingStatus(DEC_DECODING);
 
     _decodeThread = new boost::thread( 
boost::bind(NetStreamFfmpeg::av_streamer, this) );
   }
@@ -123,17 +127,18 @@
 void NetStreamFfmpeg::close()
 {
 
-       if (m_go)
+       if (decodingStatus() != DEC_STOPPED && decodingStatus() != DEC_NONE)
        {
                // request decoder thread termination
-               m_go = false;
+               decodingStatus(DEC_STOPPED);
+               assert(_decodeThread);
 
-               // wait till thread is complete before main continues
                _decodeThread->join();
+       }
 
                delete _decodeThread;
+       _decodeThread = NULL;
 
-       }
 
        // When closing gnash before playback is finished, the soundhandler 
        // seems to be removed before netstream is destroyed.
@@ -186,9 +191,9 @@
 
        NetStreamFfmpeg* ns = static_cast<NetStreamFfmpeg*>(opaque);
        boost::mutex::scoped_lock lock(ns->_netcon_mutex);
-       boost::intrusive_ptr<NetConnection> nc = ns->_netCon;
+       std::auto_ptr<tu_file>& nc = ns->_downloader;
 
-       size_t ret = nc->read(static_cast<void*>(buf), buf_size);
+       size_t ret = nc->read_bytes(static_cast<void*>(buf), buf_size);
        ns->inputPos += ret;
        return ret;
 
@@ -200,20 +205,20 @@
 {
        NetStreamFfmpeg* ns = static_cast<NetStreamFfmpeg*>(opaque);
        boost::mutex::scoped_lock lock(ns->_netcon_mutex);
-       boost::intrusive_ptr<NetConnection> nc = ns->_netCon;
+       std::auto_ptr<tu_file>& nc = ns->_downloader;
 
 
        // Offset is absolute new position in the file
        if (whence == SEEK_SET)
        {       
-               nc->seek(offset);
+               nc->set_position(offset);
                ns->inputPos = offset;
 
        // New position is offset + old position
        }
        else if (whence == SEEK_CUR)
        {
-               nc->seek(ns->inputPos + offset);
+               nc->set_position(ns->inputPos + offset);
                ns->inputPos = ns->inputPos + offset;
 
        // New position is offset + end of file
@@ -222,7 +227,7 @@
        {
                // This is (most likely) a streamed file, so we can't seek to 
the end!
                // Instead we seek to 50.000 bytes... seems to work fine...
-               nc->seek(50000);
+               nc->set_position(50000);
                ns->inputPos = 50000;
 
        }
@@ -235,12 +240,19 @@
 {
 
        // Is it already playing ?
-       if (m_go)
+       if (playbackStatus() != PLAY_NONE)
        {
                unpausePlayback(); // will check for m_pause itself..
                return;
        }
 
+       if (url.size() == 0) url += c_url;
+       // Remove any "mp3:" prefix. Maybe should use this to mark as audio-only
+       if (url.compare(0, 4, std::string("mp3:")) == 0)
+       {
+               url = url.substr(4);
+       }
+
        {       
                boost::mutex::scoped_lock lock(_netcon_mutex);
 
@@ -252,16 +264,18 @@
                        );
                        return;
                }
-       }
 
-       if (url.size() == 0) url += c_url;
-       // Remove any "mp3:" prefix. Maybe should use this to mark as audio-only
-       if (url.compare(0, 4, std::string("mp3:")) == 0)
+               url = _netCon->validateURL(url);
+               if (url.empty())
        {
-               url = url.substr(4);
+                       log_error("Couldn't load URL %s", c_url);
+                       return;
+               }
        }
 
-       m_go = true;
+
+       decodingStatus(DEC_DECODING);
+       
        pausePlayback();
 
        // This starts the decoding thread
@@ -407,35 +421,49 @@
 {
        boost::mutex::scoped_lock lock(_netcon_mutex);
 
-       boost::intrusive_ptr<NetConnection> nc = _netCon;
-       assert(nc);
+       log_security( _("Connecting to movie: %s"), url );
 
-       // Pass stuff from/to the NetConnection object.
-       if ( !nc->openConnection(url) )
-       {
-               log_error(_("Gnash could not open movie: %s"), url.c_str());
+       URL uri(url, get_base_url());
+       std::string uriStr = uri.str();
+
+       if (!URLAccessManager::allow(uri)) {
+               log_security(_("Gnash is not allowed to open this url: %s"), 
uriStr.c_str());
+               setStatus(streamNotFound);
+               return false;
+       }
+
+       StreamProvider& streamProvider = StreamProvider::getDefaultInstance();
+       _downloader.reset( streamProvider.getStream( uri ) );
+
+       if ( ! _downloader.get() ) {
+               log_error( _("Gnash could not open this url: %s"), uriStr );
+               _downloader.reset();
                setStatus(streamNotFound);
+
                return false;
        }
 
-       nc->seek(0);
+       log_debug(_("Connection established to movie: %s"), uriStr );
+
+
        inputPos = 0;
+       _downloader->set_position(0);
 
        // Check if the file is a FLV, in which case we use our own parser
        char head[4] = {0, 0, 0, 0};
-       if (nc->read(head, 3) < 3)
+       if (_downloader->read_bytes(head, 3) < 3)
        {
                setStatus(streamNotFound);
                return false;
        }
 
-       nc->seek(0);
+       _downloader->set_position(0);
        if (std::string(head) == "FLV")
        {
                m_isFLV = true;
                if (!m_parser.get())
                {
-                       m_parser = nc->getConnectedParser();
+                       m_parser.reset( new FLVParser(*_downloader) );
                        if (! m_parser.get() )
                        {
                                setStatus(streamNotFound);
@@ -467,10 +495,11 @@
                m_video_index = 0;
                m_audio_index = 1;
 
-               m_start_onbuffer = true;
+               decodingStatus(DEC_BUFFERING);
 
                // Allocate a frame to store the decoded frame in
                m_Frame = avcodec_alloc_frame();
+               unpausePlayback();
                return true;
        }
 
@@ -488,7 +517,7 @@
        }
 
        // After the format probe, reset to the beginning of the file.
-       nc->seek(0);
+       _downloader->set_position(0);
 
        // Setup the filereader/seeker mechanism. 7th argument (NULL) is the 
writer function,
        // which isn't needed.
@@ -651,7 +680,7 @@
        //GNASH_REPORT_FUNCTION;
 
        // This should only happen if close() is called before this thread is 
ready
-       if (!ns->m_go)
+       if (ns->decodingStatus() == DEC_NONE)
        {
                log_debug("av_streamer: !ns->m_go, returning");
                return;
@@ -686,7 +715,7 @@
        ns->m_unqueued_data = NULL;
 
        // Loop while we're playing
-       while (ns->m_go)
+       while (ns->decodingStatus() != DEC_STOPPED)
        {
 #ifdef GNASH_DEBUG_THREADS
                log_debug("Decoding iteration. bufferTime=%lu, bufferLen=%lu", 
ns->bufferTime(), ns->bufferLength());
@@ -699,7 +728,7 @@
                        {
 
                                // If we have problems with decoding - break
-                               if (!ns->decodeFLVFrame() && 
ns->m_start_onbuffer == false && ns->m_qvideo.size() == 0 && 
ns->m_qaudio.size() == 0)
+                               if (!ns->decodeFLVFrame() && 
ns->decodingStatus() != DEC_BUFFERING && ns->m_qvideo.size() == 0 && 
ns->m_qaudio.size() == 0)
                                {
                                        // TODO: do we really want to break 
here !?
                                        break;
@@ -721,7 +750,7 @@
                {
 
                        // If we have problems with decoding - break
-                       if (ns->decodeMediaFrame() == false && 
ns->m_start_onbuffer == false && ns->m_qvideo.size() == 0 && 
ns->m_qaudio.size() == 0)
+                       if (ns->decodeMediaFrame() == false && 
ns->decodingStatus() != DEC_BUFFERING && ns->m_qvideo.size() == 0 && 
ns->m_qaudio.size() == 0)
                        {
                                break;
                        }
@@ -729,17 +758,20 @@
                }
 
                usleep(1000); // Sleep 1ms to avoid busying the processor.
-
        }
 
 #ifdef GNASH_DEBUG_THREADS
        log_debug("Out of decoding loop");
 #endif
-       ns->m_go = false;
 
 #ifdef GNASH_DEBUG_STATUS
        log_debug("Setting playStop status");
 #endif
+       // FIXME; this is most likely incorrect. This signals to AS that
+       // playback has stopped, which is incorrect. In reality, decoding
+       // has stopped, which should have no bearing on anything. So it would
+       // be better to test for EOF when no frames are available in advance().
+       // That said,this doesn't seem to break anything, so I'll leave it...
        ns->setStatus(playStop);
 }
 
@@ -750,7 +782,7 @@
 
        NetStreamFfmpeg* ns = static_cast<NetStreamFfmpeg*>(owner);
 
-       if (!ns->m_go || ns->m_pause)
+       if (ns->playbackStatus() == PLAY_PAUSED)
        {
                return false;
        }
@@ -794,19 +826,19 @@
        if (frame == NULL)
        {
                boost::mutex::scoped_lock lock(_netcon_mutex);
-               if (_netCon->loadCompleted())
+               if (_downloader->get_eof())
                {
 #ifdef GNASH_DEBUG_THREADS
                        log_debug("decodeFLVFrame: load completed, stopping");
 #endif
                        // Stop!
-                       this->m_go = false;
+                       decodingStatus(DEC_STOPPED);
                }
                else
                {
                        pausePlayback();
                        setStatus(bufferEmpty);
-                       m_start_onbuffer = true;
+                       decodingStatus(DEC_BUFFERING);
                }
                return false;
        }
@@ -1255,7 +1287,9 @@
 NetStreamFfmpeg::refreshVideoFrame()
 {
        // If we're paused or not running, there is no need to do this
-       if (!m_go || m_pause) return;
+       if (playbackStatus() == PLAY_PAUSED) {
+          return;
+        }
 
        // Loop until a good frame is found
        while(1)
@@ -1271,16 +1305,8 @@
                }
 
                // Caclulate the current time
-               boost::uint32_t current_clock;
-               if (m_ACodecCtx && get_sound_handler())
-               {
-                       current_clock = m_current_timestamp;
-               }
-               else
-               {
-                       current_clock = clocktime::getTicks() - m_start_clock;
+               boost::uint32_t current_clock = clocktime::getTicks() - 
m_start_clock;
                        m_current_timestamp = current_clock;
-               }
 
                boost::uint32_t video_clock = video->m_pts;
 
@@ -1307,7 +1333,6 @@
 
                        // A frame is ready for pickup
                        m_newFrameReady = true;
-
                }
                else
                {
@@ -1331,7 +1356,7 @@
        //    miliseconds).
        // 2) The buffer has be "starved" (not being filled as quickly as 
needed),
        //    and we then wait until the buffer contains some data (1 sec) 
again.
-       if (m_go && m_pause && m_start_onbuffer) {
+       if (playbackStatus() == PLAY_PAUSED && decodingStatus() == 
DEC_BUFFERING) {
 
                boost::mutex::scoped_lock lock(_netcon_mutex);
     
@@ -1341,7 +1366,7 @@
 #endif
                        setStatus(bufferFull);
                        unpausePlayback();
-                       m_start_onbuffer = false;
+                       decodingStatus(DEC_DECODING);
                }
        }
 
@@ -1380,9 +1405,9 @@
 {
        //GNASH_REPORT_FUNCTION
 
-       if (m_pause) return;
+       if (playbackStatus() == PLAY_PAUSED) return;
 
-       m_pause = true;
+       playbackStatus(PLAY_PAUSED);
 
        // Save the current time so we later can tell how long the pause lasted
        m_time_of_pause = clocktime::getTicks();
@@ -1390,12 +1415,12 @@
 
 void NetStreamFfmpeg::unpausePlayback()
 {
-       if (!m_pause) // already not paused
+       if (playbackStatus() != PLAY_PAUSED) // already not paused
        {
                return;
        }
 
-       m_pause = false;        
+       playbackStatus(PLAY_PLAYING);
 
        if (m_current_timestamp == 0)
        {
@@ -1422,9 +1447,9 @@
        
        boost::mutex::scoped_lock lock(_netcon_mutex);
 
-       if ( _netCon ) 
+       if ( _downloader.get() ) 
        {
-               ret_val = _netCon->getBytesLoaded();
+               ret_val = _downloader->get_position();
        }
 
        return ret_val;
@@ -1438,15 +1463,39 @@
 
        boost::mutex::scoped_lock lock(_netcon_mutex);
     
-       if ( _netCon ) 
+       if ( _downloader.get() ) 
        {
-               ret_val = _netCon->getBytesTotal();
+               ret_val = _downloader->get_size();
        }
 
        return ret_val;
 }
 
 
+NetStreamFfmpeg::PlaybackState
+NetStreamFfmpeg::playbackStatus(PlaybackState newstate)
+{
+       boost::mutex::scoped_lock lock(_state_mutex);
+
+       if (newstate != PLAY_NONE) {
+               _playback_state = newstate;
+       }
+
+       return _playback_state;
+}
+
+NetStreamFfmpeg::DecodingState
+NetStreamFfmpeg::decodingStatus(DecodingState newstate)
+{
+       boost::mutex::scoped_lock lock(_state_mutex);
+
+       if (newstate != DEC_NONE) {
+               _decoding_state = newstate;
+       }
+
+       return _decoding_state;
+}
+
 } // gnash namespcae
 
 #endif // USE_FFMPEG

Index: server/asobj/NetStreamFfmpeg.h
===================================================================
RCS file: /sources/gnash/gnash/server/asobj/NetStreamFfmpeg.h,v
retrieving revision 1.60
retrieving revision 1.60.2.1
diff -u -b -r1.60 -r1.60.2.1
--- server/asobj/NetStreamFfmpeg.h      9 May 2008 10:03:25 -0000       1.60
+++ server/asobj/NetStreamFfmpeg.h      20 May 2008 08:02:54 -0000      1.60.2.1
@@ -52,6 +52,7 @@
 
 class NetStreamFfmpeg: public NetStream {
 public:
+
        NetStreamFfmpeg();
        ~NetStreamFfmpeg();
 
@@ -102,6 +103,25 @@
 
 private:
 
+       enum PlaybackState {
+               PLAY_NONE,
+               PLAY_STOPPED,
+               PLAY_PLAYING,
+               PLAY_PAUSED
+       };
+
+       enum DecodingState {
+               DEC_NONE,
+               DEC_STOPPED,
+               DEC_DECODING,
+               DEC_BUFFERING,
+       };
+
+       PlaybackState _playback_state;
+       DecodingState _decoding_state;
+
+       boost::mutex _state_mutex;
+
        // Setups the playback
        bool startPlayback();
 
@@ -164,6 +184,12 @@
                return time.num / (double) time.den;
        }
 
+       PlaybackState playbackStatus(PlaybackState newstate = PLAY_NONE);
+       DecodingState decodingStatus(DecodingState newstate = DEC_NONE);
+
+       std::auto_ptr<tu_file> _downloader;
+
+
        int m_video_index;
        int m_audio_index;
        




reply via email to

[Prev in Thread] Current Thread [Next in Thread]