Skip to content

Commit

Permalink
616: Fix MediaLibrary crash when prog source loading enabled (#643)
Browse files Browse the repository at this point in the history 
### 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 <[email protected]>
  • Loading branch information
@bernie-laberge
bernie-laberge authored Dec 3, 2024
1 parent ee1ecd5 commit 7bcd136
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 85 deletions.
189 changes: 104 additions & 85 deletions src/lib/ip/IPBaseNodes/FileSourceIPNode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2600,6 +2600,8 @@ namespace IPCore
}
else
{
filename = lookupFilenameInMediaLibrary( filename );

openMovieTask( filename, SharedMediaPointer() );
}
}
Expand All @@ -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]()
Expand Down Expand Up @@ -2639,100 +2652,19 @@ 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<TwkMediaLibrary::MediaAPI>( 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;
ostringstream errMsg;

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;
}
Expand All @@ -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.
Expand Down Expand Up @@ -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<TwkMediaLibrary::MediaAPI>( 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 )
{
Expand Down
4 changes: 4 additions & 0 deletions src/lib/ip/IPBaseNodes/IPBaseNodes/FileSourceIPNode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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; };

Expand Down

0 comments on commit 7bcd136

Please sign in to comment.