From 7bcd1368f6951b471e88736b0d1d76069630c77a Mon Sep 17 00:00:00 2001 From: Bernard Laberge <117092886+bernie-laberge@users.noreply.github.com> Date: Tue, 3 Dec 2024 06:01:47 -0800 Subject: [PATCH] 616: Fix MediaLibrary crash when prog source loading enabled (#643) ### 616: Fix MediaLibrary crash when prog source loading enabled ### Linked issues Fixes #616 ### Summarize your change. #### Note about progressive source loading in Open RV Note: Progressive source loading is NOT enabled by default in Open RV as it significantly complexifies custom scripting since many previously synchronous operations become asynchronous. For reference: https://aswf-openrv.readthedocs.io/en/latest/rv-manuals/rv-reference-manual/rv-reference-manual-chapter-two.html#progressive-source-loading Progressive source loading in RV can be enabled by setting the following environment variable: `export RV_PROGRESSIVE_SOURCE_LOADING=1` #### Problem Open RV sometimes crashes when executing the following script repeatedly: ```python from rv import commands as rvc rvc.setProgressiveSourceLoading(True) srcA = { "Streaming": "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerBlazes.mp4", "Frames": "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerBlazes.mp4" } srcAp = { "Streaming": "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerEscapes.mp4", "Frames": "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerEscapes.mp4" } rvc.addSourcesVerbose([[srcA["Streaming"], "+mediaRepName", "Small"]]) rvc.addSourceMediaRep("last", "Large", [srcA["Frames"]]) rvc.addSourcesVerbose([[srcAp["Streaming"], "+mediaRepName", "Small"]]) rvc.addSourceMediaRep("last", "Large", [srcAp["Frames"]]) ``` #### Cause When progressive source loading is enabled (which is not the default in RV), the actual loading of the media is done in worker threads in order to parallelize and thus optimize multiple source load as much as possible. The actual code executed is here: `FileSourceIPNode::openMovieTask()` This code contains MediaLibrary accesses. When RV was open sourced, the MediaLibrary code was refactored to be Python plugin based to make the MediaLibrary functionality more generic. The original implementation was c++ based and thread safe. The new implementation should not be called outside of the main thread. This is what was causing the random crash when progressive source loading was enabled. #### Solution I have extracted the MediaLibrary code and moved it into its own dedicated method named `FileSourceIPNode::lookupFilenameInMediaLibrary()`, which is now invoked from the main thread prior to launch the openMovieTasks . Note that the code is exactly like before. It is just invoked differently, that's all. ### Describe the reason for the change. Open RV sometimes crashes when adding media with progressive source loading enabled ### Describe what you have tested and on which operating system. Successfully tested on macOS ### Add a list of changes, and note any that might need special attention during the review. ### If possible, provide screenshots. Signed-off-by: Bernard Laberge --- src/lib/ip/IPBaseNodes/FileSourceIPNode.cpp | 189 ++++++++++-------- .../IPBaseNodes/FileSourceIPNode.h | 4 + 2 files changed, 108 insertions(+), 85 deletions(-) diff --git a/src/lib/ip/IPBaseNodes/FileSourceIPNode.cpp b/src/lib/ip/IPBaseNodes/FileSourceIPNode.cpp index a61a3f58e..cd3099cab 100644 --- a/src/lib/ip/IPBaseNodes/FileSourceIPNode.cpp +++ b/src/lib/ip/IPBaseNodes/FileSourceIPNode.cpp @@ -2600,6 +2600,8 @@ namespace IPCore } else { + filename = lookupFilenameInMediaLibrary( filename ); + openMovieTask( filename, SharedMediaPointer() ); } } @@ -2608,6 +2610,17 @@ namespace IPCore { if( !media.empty() ) { + // Lookup the filenames in the media library, save associated HTTP header and cookies + // if any, and return the actual filename to be used for opening the movie in case of + // redirection. + // Note: This needs to be done in the main thread, NOT in the worker thread + // because the media library is python based and thus should only be executed + // in the main thread. + for( size_t i = 0; i < media.size(); i++ ) + { + media[i] = lookupFilenameInMediaLibrary( media[i] ); + } + // Then add a work item to load the actual media movies. m_workItemID = graph()->addWorkItem( [this, sharedMedias, media]() @@ -2639,87 +2652,6 @@ namespace IPCore // graph using this thread (which is usually some worker thread // in IPGraph). // - Bundle* bundle = Bundle::mainBundle(); - - string file = filename; - if( TwkMediaLibrary::isLibraryURL( file ) ) - { - // - // If this file looks like a library URL try to convert it - // into a local file URL and then a path - // - - string local = TwkMediaLibrary::libraryURLtoMediaURL( file ); - local.erase( 0, 7 ); // assuming "file://" not good - file = local; - } - else if( TwkMediaLibrary::isLibraryMediaURL( file ) ) - { - // - // URL is a media URL tracked by one of the libraries. Find its - // media node and find out if its streaming. - // - - TwkMediaLibrary::NodeVector nodes = - TwkMediaLibrary::libraryNodesAssociatedWithURL( file ); - for( int n = 0; n < nodes.size(); n++ ) - { - const TwkMediaLibrary::Node* node = nodes[n]; - std::string nodeName = node->name(); - - if( const TwkMediaLibrary::MediaAPI* api = - TwkMediaLibrary::api_cast( node ) ) - { - if( api->isStreaming() ) - { - TwkMediaLibrary::HTTPCookieVector cookies = api->httpCookies(); - if( !cookies.empty() ) - { - ostringstream cookieStm; - - for( int c = 0; c < cookies.size(); c++ ) - { - if( c > 0 ) cookieStm << "\n"; - cookieStm << cookies[c].name << "=" << cookies[c].value; - - if( !cookies[c].path.empty() ) - cookieStm << "; path=" << cookies[c].path; - if( !cookies[c].domain.empty() ) - cookieStm << "; domain=" << cookies[c].domain; - } - - if (evDebugCookies.getValue()) std::cout << "Cookies:\n" << cookieStm.str() << std::endl; - - m_inparams.push_back( StringPair( "cookies", cookieStm.str() ) ); - } - - TwkMediaLibrary::HTTPHeaderVector headers = api->httpHeaders(); - if( !headers.empty() ) - { - ostringstream headersStm; - - for( size_t h = 0, size = headers.size(); h < size; ++h ) - { - if( h > 0 ) headersStm << "\r\n"; - headersStm << headers[h].name << ": " << headers[h].value; - } - - if (evDebugCookies.getValue()) std::cout << "Headers:\n" << headersStm.str() << std::endl; - - m_inparams.push_back( StringPair( "headers", headersStm.str() ) ); - } - } - - if( api->isRedirecting() ) - { - std::string redirection = api->httpRedirection(); - std::cout << "INFO: " << nodeName << " is redirecting " << file - << " to " << redirection << std::endl; - file = redirection; - } - } - } - } Movie* mov = 0; bool failed = false; @@ -2727,12 +2659,12 @@ namespace IPCore try { - mov = openMovie( file ); + mov = openMovie( filename ); } catch( std::exception& exc ) { failed = true; - errMsg << "Open of '" << file << "' failed: " << exc.what(); + errMsg << "Open of '" << filename << "' failed: " << exc.what(); cerr << "ERROR: " << errMsg.str() << endl; } @@ -2745,14 +2677,14 @@ namespace IPCore { if( errMsg.tellp() == std::streampos( 0 ) ) { - errMsg << "Could not locate \"" << file + errMsg << "Could not locate \"" << filename << "\". Relocate source to fix."; } mov = openProxyMovie( errMsg.str(), 0.0, filename, defaultFPS ); ostringstream str; - str << name() << ";;" << file << ";;" << mediaRepName(); + str << name() << ";;" << filename << ";;" << mediaRepName(); TwkApp::GenericStringEvent event( "source-media-unavailable", graph(), str.str() ); // The following instructions can only be executed on the main thread. @@ -2842,6 +2774,93 @@ namespace IPCore return hashString; } + string FileSourceIPNode::lookupFilenameInMediaLibrary( const string& filename ) + { + string file(filename); + + if( TwkMediaLibrary::isLibraryURL( file ) ) + { + // + // If this file looks like a library URL try to convert it + // into a local file URL and then a path + // + + file = TwkMediaLibrary::libraryURLtoMediaURL( file ); + file.erase( 0, 7 ); // assuming "file://" not good + return file; + } + + if( TwkMediaLibrary::isLibraryMediaURL( file ) ) + { + // + // URL is a media URL tracked by one of the libraries. Find its + // media node and find out if its streaming. + // + + TwkMediaLibrary::NodeVector nodes = + TwkMediaLibrary::libraryNodesAssociatedWithURL( file ); + for( int n = 0; n < nodes.size(); n++ ) + { + const TwkMediaLibrary::Node* node = nodes[n]; + std::string nodeName = node->name(); + + if( const TwkMediaLibrary::MediaAPI* api = + TwkMediaLibrary::api_cast( node ) ) + { + if( api->isStreaming() ) + { + TwkMediaLibrary::HTTPCookieVector cookies = api->httpCookies(); + if( !cookies.empty() ) + { + ostringstream cookieStm; + + for( int c = 0; c < cookies.size(); c++ ) + { + if( c > 0 ) cookieStm << "\n"; + cookieStm << cookies[c].name << "=" << cookies[c].value; + + if( !cookies[c].path.empty() ) + cookieStm << "; path=" << cookies[c].path; + if( !cookies[c].domain.empty() ) + cookieStm << "; domain=" << cookies[c].domain; + } + + if (evDebugCookies.getValue()) std::cout << "Cookies:\n" << cookieStm.str() << std::endl; + + m_inparams.push_back( StringPair( "cookies", cookieStm.str() ) ); + } + + TwkMediaLibrary::HTTPHeaderVector headers = api->httpHeaders(); + if( !headers.empty() ) + { + ostringstream headersStm; + + for( size_t h = 0, size = headers.size(); h < size; ++h ) + { + if( h > 0 ) headersStm << "\r\n"; + headersStm << headers[h].name << ": " << headers[h].value; + } + + if (evDebugCookies.getValue()) std::cout << "Headers:\n" << headersStm.str() << std::endl; + + m_inparams.push_back( StringPair( "headers", headersStm.str() ) ); + } + } + + if( api->isRedirecting() ) + { + std::string redirection = api->httpRedirection(); + std::cout << "INFO: " << nodeName << " is redirecting " << file + << " to " << redirection << std::endl; + file = redirection; + } + } + } + } + + return file; + } + bool FileSourceIPNode::findCachedMediaMetaData( const string& filename, PropertyContainer* pc ) { diff --git a/src/lib/ip/IPBaseNodes/IPBaseNodes/FileSourceIPNode.h b/src/lib/ip/IPBaseNodes/IPBaseNodes/FileSourceIPNode.h index 5bc500b5a..110912d9e 100644 --- a/src/lib/ip/IPBaseNodes/IPBaseNodes/FileSourceIPNode.h +++ b/src/lib/ip/IPBaseNodes/IPBaseNodes/FileSourceIPNode.h @@ -170,6 +170,10 @@ class FileSourceIPNode : public SourceIPNode static std::string cacheHash(const std::string& filename, const std::string& prefix); + // Lookup filename in the media library, save associated HTTP header and cookies if any, + // and return the actual filename to be used for opening the movie in case of redirection. + std::string lookupFilenameInMediaLibrary(const std::string& filename); + static void setSourceNameInID(bool b) { m_sourceNameInID = b; } static bool sourceNameInID() { return m_sourceNameInID; };