Details

Reviewers

Commits

rB102a7ae2d95f: VSE: Add warnings for failed proxy encoding

Summary

There can be several reasons for proxy rendering fail, but currently, there is no information about what goes wrong, which can make it more difficult for users to figure out how to do proxy rendering successfully.

This patch adds warnings for:

Unselected files(rendering will fail if the active strip is not selected)
If there are no proxy resolutions selected.
If overwrite is not checked.
Check for strip type.
Check for enabled.

Diff Detail

Repository: rB Blender

Event Timeline

Peter Fog (tintwotin) requested review of this revision.May 11 2020, 5:15 PM

Peter Fog (tintwotin) created this revision.

jorge (jorsh) awarded a token.May 12 2020, 3:29 AM

Andrea Monzini (Andrea_Monzini) awarded a token.May 12 2020, 7:33 PM

I have simplified this patch a bit:

file_list = BLI_gset_new(BLI_ghashutil_strhash_p, BLI_ghashutil_strcmp, "file list");
bool selected = false; /* Check for no selected strips */

SEQP_BEGIN (ed, seq) {
  if ((seq->flag & SELECT)) {
    selected = true;
    bool success = BKE_sequencer_proxy_rebuild_context(
        pj->main, pj->depsgraph, pj->scene, seq, file_list, &pj->queue);
    if (!success) {
      if (seq->strip->proxy->build_size_flags == 0) {
        BKE_reportf(reports, RPT_WARNING, "No resolution is selected for %s", seq->name);
      }
      else if ((seq->strip->proxy->build_flags & SEQ_PROXY_SKIP_EXISTING) == 0) {
        BKE_reportf(reports, RPT_WARNING, "Overwrite is not checked for %s", seq->name);
      }
      else { /* Unknown error. */
        BKE_reportf(reports, RPT_ERROR, "Could not build proxy for %s", seq->name);
      }
    }
  }
}
SEQ_END;

if (!selected) {
  BKE_reportf(reports, RPT_WARNING, "Select one or more strips");
  return;
}

BLI_gset_free(file_list, MEM_freeN);

We could check for selected movie, image or meta strip because only those are supported, but just simple check wouldn't produce really reliable result. for example meta may have no image or movie strip inside.
But I think it would cover 99% cases. Now you can click build proxy for color strip and it won't throw any error and show progress bar which is quite confusing.

With this check we could also prevent progress bar from showing. What do you think about that?

source/blender/editors/space_sequencer/sequencer_edit.c
195–208	There is already a loop that goes through all strips, so don't iterate twice.
237	This check should not be necessary, because it is checked in `BKE_sequencer_proxy_rebuild_context` and if strip hasn't got this member initialized, it will return `success = true` Therefore even `else {...}` branch will never be executed and should be removed.
242	This flag is set in `build_flags` not `build_size_flags`
246–248	This condition is incorrect. it should be `if (!overwrite)`

This revision now requires changes to proceed.May 14 2020, 12:05 AM

Added check for strip types - using the same types which can be enabled as proxies.
Added check for use_proxy/enabled.
Changed the order, so the progress bar will show up in fewer cases.

In D7689#186349, @Peter Fog (tintwotin) wrote:

Changed the order, so the progress bar will show up in fewer cases.

Not sure how, because progressbar will show if you have any strip selected. WM_jobs_start(CTX_wm_manager(C), wm_job); is responsible for showing progress bar.

source/blender/editors/space_sequencer/sequencer_edit.c
180	parameter `op` is unused, forgot to mention last time.
228–230	I am not sure if this is useful to check. This should definitely not throw error. Personally `RPT_INFO` would be appropriate with message `Proxy is not enabled for %s, skipping.`
234–236	You can only do this check if `success` is false. bool success = BKE_sequencer_proxy_rebuild_context( pj->main, pj->depsgraph, pj->scene, seq, file_list, &pj->queue); if (!success) { if ((seq->strip->proxy->build_flags & SEQ_PROXY_SKIP_EXISTING) != 0) { BKE_reportf(reports, RPT_WARNING, "Overwrite is not checked for %s", seq->name); } else { /* Unknown error */ BKE_reportf(reports, RPT_ERROR, "Could not build proxy for %s", seq->name); } }
245–247	This can generate too much noise if you select all strips and rebuild proxies. What I was suggesting was to only run `WM_jobs_start(CTX_wm_manager(C), wm_job);` if there is at least 1 valid strip in selection. You could throw this error in case there isn't.
3705	parameter `op` is unused, forgot to mention last time.

This revision now requires changes to proceed.May 20 2020, 5:16 PM

Remove unsupported types of strips(scene and multi-cam) from being enabled/rendered.
Remove bool success, because it would fire on more strips pointing to the same file.
Removed op.