Skip to content

Add "Remove Spot" filter.#643

Closed
bmatherly wants to merge 1 commit intomltframework:masterfrom
bmatherly:spot_remover
Closed

Add "Remove Spot" filter.#643
bmatherly wants to merge 1 commit intomltframework:masterfrom
bmatherly:spot_remover

Conversation

@bmatherly
Copy link
Copy Markdown
Member

I tried this filter in various scenarios and I think it will be pretty useful. However, there are some issues that I don't like:

  1. The avfilter filters do not support keyframes. I expect people would like to be able to keyframe this so that they could remove a spot that is moving.

  2. avfilter.delogo uses absolute pixel positions. This will cause trouble when the project resolution is changed. I know we have that problem now with some other filters. But those will all be resolved when we implement a general solution. This filter could not be fixed with a general solution and would always have this problem unless we implement some kind of filter-specific solution.

  3. avfilter.delogo is very sensitive about the x/y/w/h parameters. If they go outside of the 1 pixel boundary, the filter breaks. This limitation forced me to add more complicated bounds checking to the code. Not terrible, but would rather not have to worry about it.

Feel free to review this pull request. But don't merge it yet. I want to take some time to assess the effort to port the avfilter.delogo code to MLT so that it can use rect properties and keyframing to fix the issues I mentioned above.

@ddennedy
Copy link
Copy Markdown
Member

ddennedy commented Oct 2, 2018

I found it will crash (using Windows) if the rectangle is nearly the size of image. I see messages like the following:

[filt @ 000000005c6c70c0] auto-inserting filter 'auto_scaler_1' between the filter 'in' and the filter 'filt'
[auto_scaler_1 @ 000000005c6c80c0] w:1280 h:720 fmt:yuyv422 sar:1/1001 -> w:1280 h:720 fmt:yuv422p sar:1/1001 flags:0x2
[auto_scaler_0 @ 000000005c6c6bc0] w:1280 h:720 fmt:yuv422p sar:1/1001 -> w:1280 h:720 fmt:yuyv422 sar:1/1001 flags:0x2

That suggests like the filter is rather complex and might be difficult to port. If we move forward with this, we should constrain the rectangle to something like 20% of profile resolution.

@bmatherly
Copy link
Copy Markdown
Member Author

That suggests like the filter is rather complex and might be difficult to port. If we move forward with this, we should constrain the rectangle to something like 20% of profile resolution.

I agree. I looked at the code and I don't think it will be a clean port. I think I would rather start from scratch. It might take me a week or two to come up with something. But we will probably end up abandoning this review.

@bmatherly
Copy link
Copy Markdown
Member Author

Closing in favor of this implementation #650

@bmatherly bmatherly closed this Oct 14, 2018
@bmatherly bmatherly deleted the spot_remover branch November 17, 2018 23:42
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