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

Fix mask start/apply corruption by intermediate filters #940

Closed
wants to merge 1 commit into from

Conversation

bmatherly
Copy link
Member

When using mask_start and mask_apply, filters between them can corrupt the mask. This patch stores the mask on the clone frame so that intermediate filters can not corrupt it. The cloned frame mask is inverted and the composite is reversed to achieve the same effect.

Originally reported here:
https://forum.shotcut.org/t/beta-version-23-04-now-available-to-test/38269/125

Blur: Gaussian and Blur: Box do not work with Mask: Start and Mask: Apply because they apply the blur operation to the alpha mask that was generated by Mask: Start. Blur: Low Pass and Blur Exponential work just fine because they do not operate on the alpha mask. I did not take an inventory of what other filters are able to corrupt the alpha mask.

This patch is one idea I had to solve this problem for all filters. Maybe there are potential side effects I am not thinking of. Another idea I had is to add an option to Blur: Gaussian and Blur: Box to let the user tell the filter to not operate on the alpha channel. This option would have to be added to any filter that operates on alpha.

When using mask_start and mask_apply, filters between them can
corrupt the mask. This patch stores the mask on the clone frame
so that intermediate filters can not corrupt it. The cloned frame
mask is inverted and the coposite is reversed to achieve the same
effect.

Originally reported here:
https://forum.shotcut.org/t/beta-version-23-04-now-available-to-test/38269/125
@ddennedy
Copy link
Member

But it is not mask corruption. It is intentional that you can use other filters after mask_start to alter the alpha channel. As far as blur filter behaviors on alpha channel, it is what it is, and it is good to have options.

@ddennedy ddennedy closed this Aug 14, 2023
@bmatherly
Copy link
Member Author

But it is not mask corruption. It is intentional that you can use other filters after mask_start to alter the alpha channel. As far as blur filter behaviors on alpha channel, it is what it is, and it is good to have options.

Thanks for the explanation. I see that the documentation explains how the mask can be manipulated. However, I still think that the blur filters specifically are creating unexpected results from a user perspective. There is no indication to a user that these filters will modify the alpha channel while the other two will not.

Maybe we should add some indication to video filters that modify the alpha channel. Something to think about for the future.

In the meantime, I might add an option to the blur filters to ignore alpha channel. I would do it in a backwards compatible way so that existing projects to not suddenly experience unexpected results.

@ddennedy
Copy link
Member

Box and gaussian blur filters blur the alpha channel while exponential and low pass do not..

Maybe we should add some indication to video filters that modify the alpha channel

I already added "alpha" keyword to a number of filters where it was obvious, but some filters it depends - for example, Crop: Rectangle and Size, Position, & Rotate. We can add "alpha" to the box and gaussian filter keywords. Gaussian blur comes from libavfilter so will be difficult to add an option.

@bmatherly
Copy link
Member Author

I was able to add an option to "Blur alpha" for box and Gaussian blurs. This satisfies me for these filters. Let me know if you notice any other filters that people might want to disable alpha.
mltframework/shotcut@be4d41a

@bmatherly bmatherly deleted the mask_corruption branch November 6, 2023 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants