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

Add normalizers when applied on a chain #1054

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

j-b-m
Copy link
Contributor

@j-b-m j-b-m commented Nov 22, 2024

The freeze filter was working when applied on a producer but not when applied on a chain. When applied on a chain, the image was distorted.

The problem is that we use :
mlt_producer producer = mlt_producer_cut_parent(mlt_frame_get_original_producer(frame));

To retrieve the parent producer and cache a frame from it. However, in the case of a chain, we get a producer that doesn't have any normalizer filters on it (the chain removes normalizers from the producer).

Workaround is to check if the producer has normalizers, and in the case it doesn't, manually add them.

Does this approach seem acceptable to you ?

@j-b-m j-b-m requested a review from bmatherly November 22, 2024 08:13
@ddennedy
Copy link
Member

This does not look good to me because it is mixing concerns and duplicating duties. Maybe you should not apply freeze as a filter to a chain and instead implement a freeze link. Then, in kdenlive, you attach the link if it is a chain or the filter if not. Alternatively, hide the Freeze filter and only show it in older projects that have already included it. One can already do a freeze-frame with the timeremap link; however, you might want to offer a simpler UI or preset for this use case.

@bmatherly
Copy link
Member

I agree with Dan. This is exactly why we added chains and links. I propose that we deprecate filter_freeze and plan to phase it out in favor of a complimentary link (or provide a simplified UI for the timeremap filter as Dan suggested).

Maybe it would be OK to allow this patch as a short-term patch while we phase this filter out.

@j-b-m
Copy link
Contributor Author

j-b-m commented Nov 25, 2024

Thank you both for your comments. One thing that worked with the freeze effect that is not possible with time remap is to leave audio untouched (playing normally) while freezing the video when applied on an AV clip. For sure, the best option would be to add a freeze link. I will think a bit more about how to best handle the current situation and will get back to you soon.

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.

3 participants