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.