Add otiotool support #137
Add otiotool support #137
Conversation
Initial implementation of a tools menu and an example tool, redact. Redact is based of the otiotool command of the same name that replaces all names and references in and OTIO file as well as deleting all metadata. Added new source files tools.cpp and tools.h Signed-off-by: Thomas Wilshaw <thomaswilshaw@gmail.com>
Signed-off-by: Thomas Wilshaw <thomaswilshaw@gmail.com>
Signed-off-by: Thomas Wilshaw <thomaswilshaw@gmail.com>
Signed-off-by: Thomas Wilshaw <thomaswilshaw@gmail.com>
Signed-off-by: Thomas Wilshaw <thomaswilshaw@gmail.com>
Signed-off-by: Thomas Wilshaw <thomaswilshaw@gmail.com>
|
|
||
| if (pipe == nullptr) { | ||
| std::cout << "Failed to open pipe" << std::endl; | ||
| return std::string(); |
There was a problem hiding this comment.
Do you need to set return_val in this early exit?
There was a problem hiding this comment.
Thanks that is a good point
There was a problem hiding this comment.
Is 1 a suitable return value here or would -1 be better to indicate it isn't returning the value from the subprocess but a prior error?
tools.cpp
Outdated
| GetActiveRoot()->to_json_file(file.generic_string()); | ||
|
|
||
| // Build command | ||
| std::string command = "otiotool --input " + file.generic_string() + " " + options + " --output -"; |
There was a problem hiding this comment.
I'm not sure if temp_directory_path() or tmpnam() would ever have spaces or punctuation in them, but if they did this command would get tripped up. In Python there's a variant of popen that takes an array of strings instead of relying on shell parsing to avoid this. Is there an equivalent in C++?
Alternately, if you can pass the input to the subprocess's stdin, then you could use "otiotool --input - " + options + " --output -"
There was a problem hiding this comment.
It looks like on both Linux and Windows the temp folder can be overridden using various enviroment variables, so yes in theory a path could have spaces in it. Presumably wrapping the path in quote marks would solve this?
I did look into passing to stdin but pipes in c++ can only go one way, either write to stdin or read from std out (which we are doing here). You can get around it by using two pipes, one for read and another for write but it gets a lot more complicated and less cross platform friendly. I also tried messing around with passing the input otio as a string but that failed straight away as there is a character limit to terminal commands.
tools.cpp
Outdated
| } | ||
|
|
||
| bool FlattenVideoTracks() { | ||
| return run_otiotool_command("--flatten video"); |
There was a problem hiding this comment.
This is a really elegant and simple way to capture otiotool's features, and I could see a future way to gather some more complex recipes into simple menu items like this too.
There was a problem hiding this comment.
Thank you. I was thinking this would also be the place to put any UI code that might need to pop up to configure a command or display a result.
|
With #133 merged, we could make the standard OpenTimelineIO package a Python dependency and then UV would make sure |
|
@timlehr I guess we would still need to manually check for otiotool though as users might compile from source or run Raven outwith UV? |
@ThomasWilshaw yeah, I would say so, but at least in the case of the uv style environment, it should be guaranteed that otiotool is there (once we add OpenTimelineIO as a dependency). |
Signed-off-by: Thomas Wilshaw <thomaswilshaw@gmail.com>
… called Signed-off-by: Thomas Wilshaw <thomaswilshaw@gmail.com>
|
I've added support for more complex otiotool interactions now, let me know what you think. The first example, 
|
3 participants
Following some discussion in #63 I've had a go at implementing otiotool support via subprocesses. This is definitely a draft as it may not be the best approach.
otiotool -hand checking the return value.run_otio_commandtakes an argument string an runs the new command. You can set a debug flag to make sure you're creating the correct command. It creates a temporary file of the current root object and then callsotiotoolon that file. The result is sent back to the parent program and read as a json_string.It seems fairly clean and easy to extend but I've not been able to test on anything other than Windows, both
popenand the whole temp file method could potentially fail here. The functions called by the menu items could be more complex with maybe pop-up windows for options (e.g. select files to concat) or results (e.g.otiotool --stat).