Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding ability for a Clip to auto-detect and instantiate a Timeline Reader #459

Merged
merged 2 commits into from Mar 11, 2020

Conversation

jonoomph
Copy link
Member

@jonoomph jonoomph commented Mar 9, 2020

Adding ability for a Clip to auto-detect and instantiate a Timeline Reader from the *.osp file type. Added new Timeline constructor, to auto load UTF-8 JSON file, and regex convert all paths to absolute. Fixed a dead lock issue when a Timeline loads another Timeline.

This is the first big step towards allowing multiple *.osp files to exist inside an OpenShot Project. This opens up an infinite number of cool possibilities, such as splitting projects into scenes, and then combining scenes at the end. Or importing templates (i.e. *.osp projects) with the complexity hidden from the user (since it looks like a normal clip). I have big plans for this soon!!!

However, performance seems suspect currently, and we'll need to dig deeper, and determine how to most efficiently set thread limits and cache limits for nested Timeline readers.

…eader from the *.osp file type. Added new Timeline constructor, to auto load UTF-8 JSON file, and regex convert all paths to absolute. Fixed a dead lock issue when a Timeline loads another Timeline.
src/Timeline.cpp Outdated Show resolved Hide resolved
src/Timeline.cpp Outdated Show resolved Hide resolved
src/Timeline.cpp Outdated Show resolved Hide resolved
src/Timeline.cpp Outdated Show resolved Hide resolved
src/Timeline.cpp Outdated Show resolved Hide resolved
Copy link
Member Author

@jonoomph jonoomph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate all the feedback! @ferdnyc, the \2 makes a ton of sense now! Unfortunately, it's not as pretty to loop through the matches, but I think it's needed at this point. Take a look at the latest code I just pushed here.

  1. It gets a list of matches of all "image" or "path" paths... much simpler one than before
  2. Adds the matches to a vector, ughh, so I can get a reverse iterator (Qt regex matches have no way to be reversed I guess... not that I could find)
  3. Loop backwards through the match vector, converting paths and replacing strings as I go

I still believe this is quite good on performance, at least with my large project files I've been testing, it's basically instantaneous. And now I'm using the canonicalFilePath() method correctly, with fully exploded paths, and everything works much better.

It supports @assets, @transitions, ../, ../../, ../../../, ./FileName, etc... I can now import any *.osp project on my computer into OpenShot, and drop it on the timeline like any other file. Pretty nuts!

src/Timeline.cpp Outdated Show resolved Hide resolved
@jonoomph
Copy link
Member Author

Oh, and I accidentally started a "review" on this PR, and it's threatening me that if I cancel the review, it will delete all our comments. So... hmmm, I'm gonna just ignore it. 😉

@ferdnyc
Copy link
Contributor

ferdnyc commented Mar 10, 2020

Oh, and I accidentally started a "review" on this PR, and it's threatening me that if I cancel the review, it will delete all our comments. So... hmmm, I'm gonna just ignore it.

The review functionality gets really weird once reviews have been started on a PR, I've noticed — even normal conversational replies like this one can end up being submitted as reviews, and that's apparently just the way it's meant to work. It's thrown me a few times too, though. Whenever it starts going on about reviews, I just submit everything that way, and it seems to work out.

¯\_(ツ)_/¯ #GithubIsConfusing

@jonoomph jonoomph merged commit 629517f into develop Mar 11, 2020
@jonoomph
Copy link
Member Author

Merging, excited about this one!

@ferdnyc ferdnyc deleted the timeline-reader-improvements branch August 7, 2020 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants