Changeset View
Standalone View
source/blender/editors/space_sequencer/sequencer_edit.c
| Context not available. | |||||
| WM_main_add_notifier(NC_SCENE | ND_SEQUENCER, pj->scene); | WM_main_add_notifier(NC_SCENE | ND_SEQUENCER, pj->scene); | ||||
| } | } | ||||
| static void seq_proxy_build_job(const bContext *C, ReportList *reports) | static void seq_proxy_build_job(const bContext *C, ReportList *reports, wmOperator *op) | ||||
ISS: parameter `op` is unused, forgot to mention last time. | |||||
| { | { | ||||
| wmJob *wm_job; | wmJob *wm_job; | ||||
| ProxyJob *pj; | ProxyJob *pj; | ||||
| Context not available. | |||||
| return; | return; | ||||
| } | } | ||||
| /* Check for no selected strips */ | |||||
| bool selected = false; | |||||
| SEQP_BEGIN (ed, seq) { | |||||
| if ((seq->flag & SELECT)) { | |||||
| selected = true; | |||||
| } | |||||
| } | |||||
| SEQ_END; | |||||
| if (!selected) { | |||||
| BKE_reportf(reports, RPT_WARNING, "Select one or more strips"); | |||||
| return; | |||||
| } | |||||
ISSUnsubmitted Not Done Inline ActionsThere is already a loop that goes through all strips, so don't iterate twice. ISS: There is already a loop that goes through all strips, so don't iterate twice. | |||||
| wm_job = WM_jobs_get(CTX_wm_manager(C), | wm_job = WM_jobs_get(CTX_wm_manager(C), | ||||
| CTX_wm_window(C), | CTX_wm_window(C), | ||||
| scene, | scene, | ||||
Not Done Inline ActionsI 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. ISS: I am not sure if this is useful to check. This should definitely not throw error. Personally… | |||||
| Context not available. | |||||
| bool success = BKE_sequencer_proxy_rebuild_context( | bool success = BKE_sequencer_proxy_rebuild_context( | ||||
| pj->main, pj->depsgraph, pj->scene, seq, file_list, &pj->queue); | pj->main, pj->depsgraph, pj->scene, seq, file_list, &pj->queue); | ||||
| if (!success) { | if (!success) { | ||||
Not Done Inline ActionsYou 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);
}
}ISS: You can only do this check if `success` is false.
```
bool success =… | |||||
| BKE_reportf(reports, RPT_ERROR, "Could not build proxy for strip %s", seq->name); | if (seq->strip != NULL) { | ||||
ISSUnsubmitted Not Done Inline ActionsThis 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. ISS: This check should not be necessary, because it is checked in… | |||||
| bool proxy_25 = seq->strip->proxy->build_size_flags & SEQ_PROXY_IMAGE_SIZE_25; | |||||
| bool proxy_50 = seq->strip->proxy->build_size_flags & SEQ_PROXY_IMAGE_SIZE_50; | |||||
| bool proxy_75 = seq->strip->proxy->build_size_flags & SEQ_PROXY_IMAGE_SIZE_75; | |||||
| bool proxy_100 = seq->strip->proxy->build_size_flags & SEQ_PROXY_IMAGE_SIZE_100; | |||||
| bool overwrite = seq->strip->proxy->build_size_flags & SEQ_PROXY_SKIP_EXISTING; | |||||
ISSUnsubmitted Not Done Inline ActionsThis flag is set in build_flags not build_size_flags ISS: This flag is set in `build_flags` not `build_size_flags` | |||||
| if (!(proxy_25 || proxy_50 || proxy_75 || proxy_100)) { | |||||
| BKE_reportf(reports, RPT_WARNING, "No resolution is selected for %s", seq->name); | |||||
| } | |||||
| else if (overwrite) { | |||||
| BKE_reportf(reports, RPT_WARNING, "Overwrite is not checked for %s", seq->name); | |||||
Not Done Inline ActionsThis 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. ISS: This can generate too much noise if you select all strips and rebuild proxies.
What I was… | |||||
| } | |||||
ISSUnsubmitted Not Done Inline ActionsThis condition is incorrect. it should be if (!overwrite) ISS: This condition is incorrect. it should be `if (!overwrite)` | |||||
| else { | |||||
| BKE_reportf(reports, RPT_ERROR, "Could not build proxy for %s", seq->name); | |||||
| } | |||||
| } | |||||
| else { | |||||
| BKE_reportf(reports, RPT_ERROR, "Could not build proxy for %s", seq->name); | |||||
| } | |||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
Not Done Inline Actionsparameter op is unused, forgot to mention last time. ISS: parameter `op` is unused, forgot to mention last time. | |||||
| Context not available. | |||||
| wmOperator *op, | wmOperator *op, | ||||
| const wmEvent *UNUSED(event)) | const wmEvent *UNUSED(event)) | ||||
| { | { | ||||
| seq_proxy_build_job(C, op->reports); | seq_proxy_build_job(C, op->reports, op); | ||||
| return OPERATOR_FINISHED; | return OPERATOR_FINISHED; | ||||
| } | } | ||||
| Context not available. | |||||
parameter op is unused, forgot to mention last time.