https://bugs.kde.org/show_bug.cgi?id=497435

--- Comment #30 from Ron <kdenlive-b...@contact.dot-oz.net> ---
(In reply to Jean-Baptiste Mardelle from comment #29)
> ... My work is here:
> https://github.com/j-b-m/mlt/tree/work/qtblend-fixes

Looking at the diff between that tree and the one my 'final' patch came from,
I have a couple of initial comments (most easily expressed as a patch ;)

diff --git a/src/modules/qt/filter_qtblend.cpp
b/src/modules/qt/filter_qtblend.cpp
index f77ab94f..0db7b9bb 100644
--- a/src/modules/qt/filter_qtblend.cpp
+++ b/src/modules/qt/filter_qtblend.cpp
@@ -81,7 +81,7 @@ static int filter_get_image(mlt_frame frame,
     double opacity = 1.0;

     // If the _qtblend_scaled property is true, a filter was already applied,
and we cannot get the consumer scaling using *width / *height
-    bool qtblendRescaled = mlt_properties_get_int(frame_properties,
"_qtblend_scaled") == 1;
+   int qtblendRescaled = mlt_properties_get_int(frame_properties,
"_qtblend_scaled");
     if (mlt_properties_get(properties, "rect")) {
         rect = mlt_properties_anim_get_rect(properties, "rect", position,
length);
         if (::strchr(mlt_properties_get(properties, "rect"), '%')) {
@@ -204,7 +204,7 @@ static int filter_get_image(mlt_frame frame,
     if (distort) {
         transform.scale(rect.w / b_width, rect.h / b_height);
     } else {
-        double scale = 1;
+        double scale;
         double resize_dar = rect.w * consumer_ar / rect.h;
         if (b_dar >= resize_dar) {
             scale = rect.w / b_width;

The "int qtblendRescaled" change is a micro optimisation - it will either be 1
if we set it, or default to 0 if we didn't,
so we don't need to test it and massage it into a bool type before using it as
a true/false conditional later.
The compiler would *probably* just do that anyway, but this is a hot path, so
...

And scale doesn't need initialisation there, it's always set one way or the
other just below that.

diff --git a/src/modules/qt/transition_qtblend.cpp
b/src/modules/qt/transition_qtblend.cpp
index 4bc59050..cdebbfec 100644
--- a/src/modules/qt/transition_qtblend.cpp
+++ b/src/modules/qt/transition_qtblend.cpp
@@ -91,15 +91,15 @@ static int get_image(mlt_frame a_frame,
     }

     int request_width = profile->width;
-    int request_height = profile->height;
+//  int request_height = profile->height;
     double scalex = mlt_profile_scale_width(profile, *width);
     double scaley = mlt_profile_scale_height(profile, *height);
     if (scalex != 1.) {
-        request_width *= scalex;
+        request_width = *width;
         b_height *= scalex;
         b_width *= scalex;
     }
-    request_height *= scaley;
+    int request_height = *height;

     // Check transform
     if (mlt_properties_get(transition_properties, "rect")) {
@@ -203,6 +203,7 @@ static int get_image(mlt_frame a_frame,
                                     
"progressive,distort,colorspace,full_range,force_full_luma,"
                                      "top_field_first,color_trc");
             // Prepare output image
+           // XXX Delete this conditional now
             if (false && b_frame->convert_image && (b_width != request_width
|| b_height != request_height)) {
                 mlt_properties_set_int(b_properties, "convert_image_width",
request_width);
                 mlt_properties_set_int(b_properties, "convert_image_height",
request_height);

The change to request_*, unless I'm missing something there, is just what that
really does?
mlt_profile_scale_width sets scalex = width / profile->width
So at best request_width (aka profile->width) * scalex, will always equal width
or at worst it will be slightly different due to fp rounding precision.
The compiler would *probably* optimise out the second multiply - but if I'm
not missing something, we can just beat it to it.

The XXX comment is just about the if (false ...) change there.  If that change
is no longer
WIP, that block can be deleted in the final patch.


I'm also not really clear on what this following code is really doing - I get
what you are aiming
for in principle here, and I think that's the right thing - and this probably
just shows off how
grainy my understanding of mlt's internal machinations still is, but ...

when we get here, b_width is either set to profile->width or meta.media.width,
and I don't see
where either of those might get changed by qtblend previously rescaling, so
it's not clear to
me why it would need to be multiplied by consumerScale, or why that would only
be done
if we are scaling up ...

I have no reason to doubt you had a good reason for doing this - it's just the
one bit I can't
execute in my head and go "yep, that looks like the right thing to me" :)

In filter_qtblend.cpp:
+        if (qtblendRescaled) {
+            // Another qtblend filter was already applied
+            // We requested a image with full media resolution, adjust rect to
profile
+            // Check if we have consumer scaling enabled
+            double consumerScale = mlt_properties_get_double(frame_properties,
"_qtblend_scalex");
+            if (consumerScale > 0.) {
+                b_width *= consumerScale;
+                b_height *= consumerScale;
+            }

I haven't actually built and tested this yet, just looked over what else you
changed, but I'll rebase
this on the master HEAD I have been testing with and do that next ...

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to