Skip to content

Conversation

@j-b-m
Copy link
Contributor

@j-b-m j-b-m commented Feb 3, 2025

This fixes multiple issues when applying several qtblend filters on the same frame.
One issue is that the qtblend filter, like the affine filter/transition, sometimes query an image using the producer's original resolution, not in profile resolution in order to be able to display for example to zoom in a 4k clip with an HD profile, while keeping the 4K resolution.

However this caused issues because in several parts of the code, we calculate the preview scaling by comparing the size of the requested image with the size of the project profile.

Among the several fixes in this patch, we now check if another qtblend filter was applies and use a cached value for consumer scaling.

Copy link
Member

@bmatherly bmatherly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have much opinion on the substance of the change. It seems a little strange that a filter would keep track of whether it has been applied already. But I don't have a better idea for how to handle that situation. Maybe there should be a generic way to indicate that the original image has already been scaled (like affine).

/*
* filter_lightshow.cpp -- animate color to the audio
* filter_qtblend.cpp -- Qt composite filter
* Copyright (C) 2015 Meltytech, LLC
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to update the copyright date in all of these files that are changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks for looking at it. Since these changes only affect qtblend that is mostly used in Kdenlive, I will merge the changes in a few days if there is no opposition.

@j-b-m j-b-m merged commit de332f8 into mltframework:master Feb 14, 2025
5 checks passed
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.

4 participants