Improvement suggested by @Peter Fog (tintwotin):

• Using the length of the sequences as a maximum length for the fade, so that the operator can still insert a valid fade even if the sequence is too short. This would be consistent with how fade handles work in most video editing programs and daws I've worked with.

It gave me errors, but worked when I changed the names of the registered classes to the 2.80 class naming convention.
I uploaded a diff here: D5167 to fix that.
I also added the operator to the to "add" menu and "context" menu.
I changed the descriptions visible in the menu to capitalized, so they would be in consistency with all other menus.

Suggestions:

• Fade In/Out to/from the current level(at the next/previous frame of graph) and not the max level of the strip.
• Avoid operations outside the length of the strip and shorten duration if it is outside of the strip.
• @William Reynish (billreynish) Enable using 'layout.prop(view, "show_region_hud")' to the Sequencer so it is visible to the users how values like duration and fade type can be changed via the 'Adjust previous operation' panel.
• Let context.space_data.show_seconds determine if the duration should be shown in frames or sec.

• Fade In/Out to/from the current level(at the next/previous frame of graph) and not the max level of the strip.: should the fade use the first keyframe on either side of the curve, or use the value right after the fade?

• Avoid operations outside the length of the strip and shorten duration if it is outside of the strip. -> Makes sense. Should I use the length of each selected strip as individual maximums, or use the shortest sequence's duration? If I use an interactive tool in another video editing program or a daw, it will typically use the shortest strip's duration.

• Let context.space_data.show_seconds determine if the duration should be shown in frames or sec.: I don't think it's possible. When defining properties, you don't have access to the context or that value. I don't know if it's even okay to have conditional statements when defining properties.

I added the improvements I could based on @Peter Fog (tintwotin)'s feedback. It's ready for review now. I'd just need to know if the operator should work in frames instead of seconds?

There's code from @Peter Fog (tintwotin) to add the op to the menus. Is there a way to add him as a co-author of the diff or commits?

You simply mention other contributors in the commit itself when it comes to that.

As for the patch, it should use the Adjust Last Operation system, which currently isn't supported in the Sequencer. This should be added first.

Thanks William, I'll update the diff when adjust last op's been added 🙂

Possible improvements:

1. @Peter Fog (tintwotin) brought up an issue with how the fade works currently. If the user has an existing animation curve with few keyframes, the fades will go directly to the value of the next keyframe on the curve, in the fade direction. As changing the behavior of the fades will produce new edge cases, I would like people to test the tool to see if this is an issue when you're working on real video projects.
2. @snu told me that he tries to detect existing fades if there is already an animation curve on the strips in his add-on, and if so he only moves the keyframes instead of adding new ones to avoid edge cases linked to adding and removing keyframes from the Fcurve. E.g. having residual keyframes if you create a fade, and later you create a shorter to fade on the same strip. This would also allow the clear fades operator to only remove detected fades.

We'd need some way to draw the fades reliably so users know which strips have fades in, out, or in and out. That'd be really useful. If someone can point me in the right direction, I could give a hand there. Although this feature and the implementation need review first.

I think there will always be *some edge cases* with this approach. As unlike in other audio and video editing programs, there is currently no separation between the concept of fades and the opacity or volume animation. Although fades are conceptually different: they're usually an automated curve that multiplies the user's automation/animation. With visual strips the lack of separation isn't a big issue in my experience, but for audio it can be inconvenient even with simple projects.

The biggest issue right now are, to me:

• If you extend a strip, the fade animation won't update.
• There is no drawing to represent fades in the VSE, making it hard to keep track of strips with fade animation.

Don't really like the complexity of this OP, but that isn't any dealbreaker, as this is covering functionality, that should be native, thus temporary.

Also I am not used to work with animation API/OPs so I assume this is correct and efficient.
If I will have spare time I could analyze this in detail, but now I will mainly test functionality.

@Richard Antalik (ISS) Do you have an opinion about preserving the curve between the inserted fades?

And using s.ms instead of frames as value style?

Regarding complexity: that's what it takes to provide the functionality. There are parts of the API that are a bit inconvenient, e.g. sequences don't have animation data attached to them, so you need code to find and create anim data on the scene's anim action, etc. Same, there's no notion of fades in the VSE's back-end, so you have to code everything. 200 isn't a lot if you keep this in mind + the op's features + the fact that it already accounts for some corner cases.

The main loop is hopefully pretty readable as well, it should be easy to maintain:

for sequence in sequences:
    duration = self.calculate_fade_duration(context, sequence)
    duration = min(duration, max_duration)
    if not self.is_long_enough(sequence, duration):
        continue

    animated_property = 'volume' if hasattr(sequence, 'volume') else 'blend_alpha'
    fade_fcurve = self.fade_find_or_create_fcurve(context, sequence, animated_property)
    fades = self.calculate_fades(sequence, fade_fcurve, animated_property, duration)
    self.fade_animation_clear(context, fade_fcurve, fades)
    self.fade_animation_create(fade_fcurve, fades)

The Fade class adds some locs but it also helps to decouple the op. Before I added it, the changes requested in the vse chat were a pain to code.

Done with the changes. Only one left: the duration_seconds thing. Should I remove it and have the operator work in frames instead, to be consistent with the rest of Blender?

Ok so I tested this, seems good, but:

• Clear fades operator isn't available from UI, unless it is searched. It really should be.
• How can I choose duration as shown in demo? I mean functionality is provided by from/to playhead, but just asking if this was dropped or something.

In D5166#120126, @Richard Antalik (ISS) wrote:

Ok so I tested this, seems good, but:

• Clear fades operator isn't available from UI, unless it is searched. It really should be.

@Peter Fog (tintwotin) where would you put the Fade Clear operator? How about the menu Strip -> Fade Clear under Hold Cut?

• How can I choose duration as shown in demo? I mean functionality is provided by from/to playhead, but just asking if this was dropped or something.

After calling the operator, F9, or Edit -> Adjust Last Operation. That's how operators work in blender. It'll be more accessible once a floating window like the one in the 3d view gets added. Also, I'd like to turn this into a tool once we have a VSE toolbar.

Just added the last bit: now Fade and Clear Fade show in the VSE context menu, but only if you have at least 1 sequence selected. Before, Fade would show even without a selection, but would be greyed out.

Thanks for the review. I'll be around to maintain my code, ping me anytime if you need me. 🙂

@Campbell Barton (campbellbarton) Haven't noticed this during review.

Not sure what reopening this diff means - can I or author just make fix for this issue and commit separately?

This is just

In D5166#133441, @Richard Antalik (ISS) wrote:

@Campbell Barton (campbellbarton) Haven't noticed this during review.

Not sure what reopening this diff means - can I or author just make fix for this issue and commit separately?

Re-opened just to mark it as needing changes. Talked to @Nathan Lovato (gdquest) in chat, he's looking into an alternative to eval(..)

Another thing I haven't noticed is that there are some warnings caused by this patch:

WARN (rna.define): c:\blender-git\2\source\blender\makesrna\intern\rna_define.c:1239 RNA_def_struct_ui_text: 'Removes fade animation from selected sequences.' description from 'SEQUENCER_OT_fades_clear' '' ends with a '.' !
WARN (rna.define): c:\blender-git\2\source\blender\makesrna\intern\rna_define.c:1647 RNA_def_property_ui_text: 'Fade in, out, both in and out, to, or from the playhead. Default is both in and out.' description from 'type' '' ends with a '.' !
WARN (rna.define): c:\blender-git\2\source\blender\makesrna\intern\rna_define.c:1239 RNA_def_struct_ui_text: 'Adds or updates a fade animation for either visual or audio strips.' description from 'SEQUENCER_OT_fades_add' '' ends with a '.' !

Here is the patch to fix the issue: https://developer.blender.org/D5807

Another thing I haven't noticed is that there are some warnings caused by this patch:

I think Campbell already fixed them. I double-checked and descriptions don't end with '.' in the latest master.

Just a heads-up that this was already solved, the feature is in Blender 2.81, so you can close the diff. I can't see an option to mark it as done/closed myself.