Fix bug #1228: Use a 0.85 mipMap to avoid jerky pan/zoom, also use MLT instead QImage for downsampling#1229
Conversation
…oid jerky animations Also: - Do not multiply for b_ar (pixel ratio), normalize before scaling instead - Uniform aspect ratio conditional operator (>) was synced to (>=) across both transition_qtblend and filter_qtblend
guiodic
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the Qt blend filter/transition to improve transform (pan/zoom) smoothness on large sources by shifting most downscaling work from QImage::scaled() to MLT’s image request sizing, using a stepped “mipmap” approach.
Changes:
- Add stepped downscale (“0.85 mipmap”) logic to request a pre-downscaled frame from
mlt_frame_get_image, leaving only a small remaining scale forQPainter. - Remove
QImage::scaled()downscaling paths and consistently useQPaintertransforms for scaling. - Adjust anamorphic handling by normalizing source dimensions relative to the consumer SAR, and sync a
>→>=aspect-ratio conditional.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/modules/qt/transition_qtblend.cpp |
Adds SAR normalization + mipmap-based pre-downscale and removes QImage::scaled() usage in the transition compositor. |
src/modules/qt/filter_qtblend.cpp |
Adds SAR normalization + mipmap-based pre-downscale and removes QImage::scaled() usage in the filter compositor. |
| while (mltMipmapScale * 0.85 >= scaleTarget && mltMipmapScale > 0.001) { | ||
| mltMipmapScale *= 0.85; | ||
| } |
There was a problem hiding this comment.
The 0.85 mipmap step is a new magic number used in both transition_qtblend.cpp and filter_qtblend.cpp. To make future tuning safer and keep the two implementations in sync, consider introducing a named constant (or shared helper) and adding a short rationale comment next to it in one place.
There was a problem hiding this comment.
please add this as constant to common.h
There was a problem hiding this comment.
please add this as constant to
common.h
Is it ok something like this?
#define MLT_QT_MIPMAP_STEP 0.85
There was a problem hiding this comment.
This is a C++14 header, so you can use static constexpr const double MLT_QT_MIPMAP_STEP = 0.85;
There was a problem hiding this comment.
This is a C++14 header, so you can use
static constexpr const double MLT_QT_MIPMAP_STEP = 0.85;
Very well, that was my first choice, but then I realised it had never been used.
It’s also true that this module is quite different from the rest.
There was a problem hiding this comment.
@ddennedy Since we're on the subject, could I create a helper for the code shared between transitions and filters?
There was a problem hiding this comment.
@ddennedy done. I also applied in separate commits some Gemini's suggestions in PR that sounded sensible to me.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… function like in filter
j-b-m
left a comment
j-b-m
left a comment
There was a problem hiding this comment.
Looks fine to me, thanks a lot.
A previous commit (
fe9ab9104beeef87c13f9e80ca1914276e0d4d1a) introducedQImage::scaledto resolve 'awful' output whenQPainterdownscales significantly.Whilst this significantly improves the quality of the output image, also cause continuous animations (e.g., pan/zoom) to jerk in some circumstances (in my case, the original image is very large and the animation is very slow). See bug report #1228 for some examples.
This may be caused by an internal behaviour of
QPainterwhich is disabled if it is used with scaling.This patch attempts to solve the issue by using a mipmap: we calculate a
0.85target (this 'magic number' is based on a series of tests using very large images, in order to maintain both quality and smoothness). The remaining scaling is handled byQPainter.Also, instead of using
QImagefor downscaling, we request MLT a pre-downscaled frame directly frommlt_frame_get_image.This is at very least 2x faster and far less memory-consuming than
QImage, whilst maintaining good output quality when properly configured (for example, by choosing Lanczos for interpolation in Kdenlive export window, but in my tests even bilinear interpolation doesn’t cause the problems I noticed with MLT 7.36)The following are minor corrections:
* b_armultiplier.>) was synced to (>=) across both transition_qtblend and filter_qtblend.