Maniphest T80717

Crash after "Reassigning Inputs" to newly created Effect Clip
Closed, Resolved
Actions

Assigned To
Richard Antalik (ISS)
Authored By
Thomas Mann (pixtur)
Sep 12 2020, 1:06 AM
Tags
  • BF Blender
  • VFX & Video
Subscribers
Richard Antalik (ISS)
Robert Guetzkow (rjg)
Sergey Sharybin (sergey)
Thomas Mann (pixtur)

Description

System Information
Operating system: Win10
Graphics card: GTX2060

Blender Version
Broken: 2.90, 0330d1af29c0, master, 2020-08-31

Short description of error
Crash after creating color strip in Sequence editor and pressing R

Exact steps for others to reproduce the error

  1. Start Blender
  2. Open Video Sequencer
  3. Add -> Color
  4. Effect Strip -> Reassign Inputs

-> Crash

This is probably a double of https://developer.blender.org/T8263

Related Objects

Mentioned Here
rB2c14a950a718: Fix T81594: Unable to reassign effect inputs
rB0330d1af29c0: Fix T77900: File Browser in macOS fullscreen crashes
T8263: Video Sequence Editor > Sequence > Reassign Inputs > crash

Event Timeline

Thomas Mann (pixtur) created this task.Sep 12 2020, 1:06 AM
Thomas Mann (pixtur) updated the task description.
Robert Guetzkow (rjg) changed the task status from Needs Triage to Confirmed.Sep 12 2020, 1:45 AM
Robert Guetzkow (rjg) added a subscriber: Robert Guetzkow (rjg).

The issues is in sequencer_reassign_inputs_exec, error_msg is NULL when the following if-case is true:

if (!seq_effect_find_selected(
        scene, last_seq, last_seq->type, &seq1, &seq2, &seq3, &error_msg) ||
    BKE_sequence_effect_get_num_inputs(last_seq->type) == 0) {
  BKE_report(op->reports, RPT_ERROR, error_msg);
  return OPERATOR_CANCELLED;
}

Since BKE_report tries to determine the length of the message with strlen() this results in a crash.

BKE_report(ReportList * reports, ReportType type, const unsigned char * _message) Line 123	C
sequencer_reassign_inputs_exec(bContext * C, wmOperator * op) Line 2146	C
wm_operator_invoke(bContext * C, wmOperatorType * ot, wmEvent * event, PointerRNA * properties, ReportList * reports, const bool poll_only, bool use_last_properties) Line 1313	C
wm_handler_operator_call(bContext * C, ListBase * handlers, wmEventHandler * handler_base, wmEvent * event, PointerRNA * properties, const unsigned char * kmi_idname) Line 2132	C
wm_handlers_do_keymap_with_keymap_handler(bContext * C, wmEvent * event, ListBase * handlers, wmEventHandler_Keymap * handler, wmKeyMap * keymap, const bool do_debug_handler) Line 2442	C
wm_handlers_do_intern(bContext * C, wmEvent * event, ListBase * handlers) Line 2739	C
wm_handlers_do(bContext * C, wmEvent * event, ListBase * handlers) Line 2866	C
wm_event_do_handlers(bContext * C) Line 3364	C
WM_main(bContext * C) Line 485	C
main(int argc, const unsigned char * * UNUSED_argv_c) Line 531	C
Robert Guetzkow (rjg) added a project: VFX & Video.Sep 12 2020, 1:46 AM
Robert Guetzkow (rjg) added a comment.Sep 12 2020, 1:21 PM

The solution is to only run BKE_report if the passed pointer to the error message isn't NULL. Either don't call BKE_report if error_msg is NULL or assign a descriptive message to it before calling BKE_report.

Sergey Sharybin (sergey) added subscribers: Richard Antalik (ISS), Sergey Sharybin (sergey).Oct 28 2020, 12:36 PM

@Richard Antalik (ISS), i remember seeing patches from you which seemed to be related to this issue. Are they committed? Is it a separate issue from what you was fixing in patches?

Robert Guetzkow (rjg) added a comment.EditedOct 28 2020, 1:26 PM

The solution depends on what particular error message we want for the neglected case of !seq_effect_find_selected(scene, last_seq, last_seq->type, &seq1, &seq2, &seq3, &error_msg) being true.

Richard Antalik (ISS) added a comment.Oct 28 2020, 1:40 PM
In T80717#1043522, @Sergey Sharybin (sergey) wrote:

@Richard Antalik (ISS), i remember seeing patches from you which seemed to be related to this issue. Are they committed? Is it a separate issue from what you was fixing in patches?

They are comitted, this looks like separate issue. I am checking this

Richard Antalik (ISS) closed this task as Resolved.Oct 28 2020, 1:47 PM
Richard Antalik (ISS) claimed this task.

Looks like I fixed this "accidentally"

Cant reproduce in 2.90 but not in 2.91/2.92, so closing.

Fixed by rB2c14a950a718

Robert Guetzkow (rjg) added a comment.EditedOct 28 2020, 3:02 PM

@Richard Antalik (ISS) My bad, it is sort of fixed. Might still be a good idea to at least assign an empty string or generic error message to the error_msg. If the conditions for the error reporting change, the string is still uninitialized.