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

Queued stabilize and export fails to stabilize clips #756

Closed
Gizemquea opened this issue Jul 9, 2019 · 13 comments
Closed

Queued stabilize and export fails to stabilize clips #756

Gizemquea opened this issue Jul 9, 2019 · 13 comments

Comments

@Gizemquea
Copy link

Shotcut 19.06.15 Windows 10 (broken on older versions too)

Steps to reproduce:

1.) Create new project with one video track
2.) Add two clips to project
3.) Add stabilize filter to both clips, set to max shakiness and accuracy, modify filter options like smoothing as you see fit
4.) Queue analyze for both clips
5.) While the job for the first clip is still running, also queue export of the whole timeline
6.) Wait for export to finish

Exported video is NOT stabilized (both clips affected).

Failing to apply analysis results for stabilized is observed with an arbitrary number of jobs, found that out the hard way in a ~2 hour project with far more than 100 scenes and around 30 hours of processing time for analysis and export - this is why I analyzed this very thoroughly.

Clips that have been previewed in their stabilized state BEFORE an export job is queued seem to work, previewing a clip after the export job is queued but NOT already processing will still render the original clip without stabilization.

As far as I can tell, videos that have not been previewed at all will simply NEVER be stabilized on export. This means you cannot use a queued export job if you need stabilize at all, because this behavior forces you to preview EVERY clip that requires stabilize at least once, even if you know that certain stabilize settings will do the job just fine.

@ddennedy
Copy link
Member

@bmatherly Would you give some thought to not requiring a separate property results that needs to be updated? Can we make it such that if filename loads successfully, then we use that? And if an app wants to re-analyze, it can remove or truncate the file?

Meanwhile, I will look into a way to identify each filter that survives graph manipulations, and to= traverse the MLT graph (using mlt_parse?) to locate a filter by id. This is to update a filter after a job completes outside the UI state.

@bmatherly
Copy link
Member

Hi Dan, so If I understand your concept...

  1. Remove the "results" property.
  2. The application will specify the file name using the "filename" property
  3. When the filter is created, it will attempt to load results from "filename"
  4. If any results are loaded successfully, then the filter is in "apply" mode and will apply the results.
  5. If results are not loaded successfully, then the filter is in "analyze" mode and will analyze frames.
    ** When the last frame is analyzed, the filter will put itself into "apply" mode.
  6. If the user wants to re-analyze a file, the application will delete the file (but leave the filename property unchanged)

Does that sound like it will work?

When Shotcut completes a job to create the analysis results file, how will the filter currently instantiated in Shotcut know to switch to "apply" mode?

@ddennedy
Copy link
Member

Yes, something like that.

how will the filter currently instantiated in Shotcut know to switch to "apply" mode?

How about the filter always operates in apply mode unless told otherwise through a property that is set only in the job xml. Then, while in apply mode but not yet active, for each frame check if the specified file exists and try to load it. If it succeeds, switch into active apply mode. To prevent trying to parse an invalid file over and over, save the file size in an unserialized property and only try to open again if that changes. This file size change can also be used to reopen in the case of a stab file being updated. That will be rare, but a person could stop an analyze job and restart it, for example.

I am starting to think my approach will be easier. We have Mlt::Controller::ensureHasUuid() already and can call that when a filter gets added. Then, AnalyzeDelegate is modified to take and store a QUuid instead of a Mlt::Filter. Also, make it inherit Mlt::Parser and when a matching filter is found, then do the work. There are two problems:

  1. mlt_parser is not used often and might have a bug, which is also an opportunity to exercise that API.
  2. If you queue a bunch of stabilize jobs followed by an export job, the export job is not updated. That is not trivial, as it by design that an export job is a snapshot of the state of the composition at the time you enqueue it.

@bmatherly
Copy link
Member

If you queue a bunch of stabilize jobs followed by an export job, the export job is not updated. That is not trivial, as it by design that an export job is a snapshot of the state of the composition at the time you enqueue it.

We could extend the export job to always re-run the first pass of any 2-pass filters in the project. This may result in extra processing in most export jobs, but the output would always be consistent. I think this would be useful in more situations than one might originally think.

For example: If I add a clip to the timeline and then apply the audio normalization filter (analyze and apply). Now I might think I'm done with that while I edit. But later, I add equalizer filter and move it before the normalize filter. Now the normalize results are invalid, but the normalize filter does not know that.

So there are many ways that a 2-pass filter could be invalidated. Re-running it on export would always take care of that.

@ddennedy
Copy link
Member

ddennedy commented Jul 10, 2019 via email

@bmatherly
Copy link
Member

OK. I could probably draft a proposal in a code review by the end of the week. How much do you care about maintaining backwards compatibility with the current filename and results properties?

Or maybe you have an idea in mind that you would like to draft sooner?

@ddennedy
Copy link
Member

Backwards compatibility with projects and apps still using "results" is important. We can require a new property to flag new behavior such as no_results=1. It will not be in the v19.07 release as the beta already started, and this is a pretty big change.

@Gizemquea
Copy link
Author

Is there any chance the v19.07 might at least get a partial solution/workaround so that queued analysis results will be used upon export? From the user's perspective this would still be a valuable improvement, even if any UI actions like changing the order in which filters are applied cannot yet be handled consistently.

And since you are talking about backwards compatibility in the sense that you want to ensure that older projects will render correctly (even if that triggers a re-analyze): would you also consider auto-detection of implausible result file names (related to #753)?

If there is a project that uses identical result files for different clips, this is most probably wrong (the only counter-example I can think of are different clips with exactly the same length and identical audio).

For stabilize this is in fact ALWAYS wrong, because vidstab as used by shotcut will NEVER produce identical result files (even when re-calculating for the same clip with identical settings). In an ideal world, shotcut would trigger re-analyze automatically once it detects inconsistent result files in an older project, just as it might have to do when the ordering of filters has been changed.

@ddennedy
Copy link
Member

No, we do monthly releases, and we will try to get this into August. The other issues such as filter order and automatical running outstanding analysis jobs are separate from this.

@bmatherly
Copy link
Member

@bmatherly Would you give some thought to not requiring a separate property results that needs to be updated? Can we make it such that if filename loads successfully, then we use that? And if an app wants to re-analyze, it can remove or truncate the file?

I have prototyped this and I found a fatal flaw: This can work for the stabilize filter because the results are stored in a file. But it will not work for the Normalize (2 Pass) filter because the results are a property that must be applied to the filter.

I have a couple of thoughts/opinions based on this:

  1. I think it is necessary to implement a post-analyze-job lookup in the application to force the application of the results property when the UI is not active for that filter.

  2. Alternately, I think we could consider making the analysis step of all 2-pass filters modal. That is, we would block the application while the analysis takes place. This would force the user to observe the results and make sure they get the expected outcome.

  3. I understand my suggestion in Logo #2 would not support this use case where the user queues up a bunch of analysis jobs (and a following export job). But I think that might be OK because we could find a better way to handle that situation. I have been thinking for a long time about adding a "normalize" feature to the "Convert to edit friendly" feature. The idea is that we could perform scaling, deinterlacing, audio normalization (and now I think stabilization) in a batch process upon import. That would reduce processing overhead when actually performing edits and previewing. This is a feature that I would be willing to help work on.

@ddennedy
Copy link
Member

Just a note to say I have a plan for how to address this in Shotcut that is not modal.

@ddennedy ddennedy added this to the v19.08 milestone Jul 29, 2019
@ddennedy
Copy link
Member

Not done under this ticket because it is out-of-scope is to find filters where analyze had not yet been run and enqueue them before export. But I plan to do that. At this time, I am not volunteering to try to look for filters with invalid results.

@ddennedy
Copy link
Member

ddennedy commented Aug 4, 2019

Today I added the option to run pending analysis jobs upon export:
8a03985

It is behind a confirmation dialog. It also applied to Normalize: Two Pass. This only works for Stabilize if you are using the project folder feature (see New Project view on startup), or if you have stopped a Stabilize analysis job such that it has the name assigned. Otherwise, Shotcut does not know or assume the .stab file name and location.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants