Bug #1: Put two tracks on top of each other. Have the top one at say,
50% fade or so. (any fade that isn't 100% has the bug). Have the
bottom at 0% fade. The bottom track is visible, although it shouldn't
be. The lower the top track's fade, the more noticable the bug.
Bug #2: Put two tracks on top of each other. Have the clip on the
bottom track start a few seconds after the first. Have the top one at
50% fade. Mute the bottom track. There is a noticable change in
brightness when the clip on the bottom track starts.
Bug #3: Put two tracks on top of each other. Have the top one at 50%
fade. Change the fade of the bottom track between 0% and 50%. There is
no difference in output despite the changes on the bottom track's
fade.
Three bugs that I found... but they all have to do with the same few
lines of code in the render engine. Unfortunately, there is a ton of
copy/paste code in this source file... so most likely this bug exists
in other forms or on other render settings. However, this patch fixes
Cinelerra as far as my current project is concerned. If this patch
doesn't work for you... I guess you can apply a similar technique to
one of the other copy/paste regions of that file and see if it works.
Questions / Comments are welcome, but its a pretty self-explanatory
patch.
Index: overlayframe.C
===================================================================
--- overlayframe.C (revision 1060)
+++ overlayframe.C (working copy)
@@ -204,13 +204,17 @@
{ \
temp_type r, g, b, a; \
temp_type pixel_opacity, pixel_transparency; \
+ temp_type max_squared; \
+ temp_type output_weight; \
temp_type output1 = output[0]; \
temp_type output2 = output[1]; \
temp_type output3 = output[2]; \
temp_type output4 = output[3]; \
\
pixel_opacity = opacity * input4; \
- pixel_transparency = (temp_type)max * max - pixel_opacity; \
+ max_squared = (temp_type) max*max; \
+ pixel_transparency = (temp_type)max_squared - pixel_opacity; \
+ output_weight = (pixel_transparency * output4) / max ; \
\
switch(mode) \
{ \
@@ -310,16 +314,17 @@
break; \
case TRANSFER_NORMAL: \
r = (input1 * pixel_opacity + \
- output1 * pixel_transparency) / max / max; \
+ output1 * output_weight) / max / max; \
g = ((input2 - chroma_offset) * pixel_opacity + \
- (output2 - chroma_offset) * pixel_transparency)
\
+ (output2 - chroma_offset) * output_weight) \
/ max / max + \
chroma_offset; \
b = ((input3 - chroma_offset) * pixel_opacity + \
- (output3 - chroma_offset) * pixel_transparency)
\
+ (output3 - chroma_offset) * output_weight) \
/ max / max + \
chroma_offset; \
- a = input4 > output4 ? input4 : output4; \
+/* a = (max_squared - (((max_squared-pixel_opacity)/max *
(max -
output4)))) / max ; */\
+ a = max; \
break; \
} \
\
Overall, it is pretty easy to understand. Bug #3 resulted from the
line a = input4 > output4 ? input4 : output4;. Bug #1 was fixed by
multiplying by "output_weight" instead of "pixel_transparency". Not
only should the "pulled" transparency be taken into account... but the
current frame's alpha values should be as well (output4). Bug #2 I
don't understand... but the line a = max fixed it. :-) Well, sometimes
you just gotta go for it. And in this case, it somehow worked out. If
someone could figure out bug #2 for me, I would be most grateful :-) I
have my "first try" at the bug still there, but commented out. a =
(max_squared - (((max_squared-pixel_opacity)/max * (max - output4))))
/ max ;
As far as theoretical issues are concerned, I am somewhat worried
about overflow if 16-bit RGBA are used. However, in the current setup,
there is more accuracy in pixel_transparency when assuming 8-bit
values. Pixel_transparency already uses upto 2x the number of bits
than the currrent format, and doing (pixel_transparency * output4)
will use 3x the number of bits in the worst case. For 8-bit values,
thats fine if you use at least 24 bits, and for 16-bit values, thats
fine if you have at least 48 bits. If I'm reading the code correctly,
there shouldn't be any issues (temp_type is int32_t with 8-bit values,
and temp_type is int64_t with 16-bit values), but it is something to
keep in mind.
I am having problems with svn diff again, so I did a "svn diff"
inside the cinelerra/ directory. Apply the patch there.
--Percival "Dragontamer" Tiglao
Index: overlayframe.C
===================================================================
--- overlayframe.C (revision 1060)
+++ overlayframe.C (working copy)
@@ -204,13 +204,17 @@
{ \
temp_type r, g, b, a; \
temp_type pixel_opacity, pixel_transparency; \
+ temp_type max_squared; \
+ temp_type output_weight; \
temp_type output1 = output[0]; \
temp_type output2 = output[1]; \
temp_type output3 = output[2]; \
temp_type output4 = output[3]; \
\
pixel_opacity = opacity * input4; \
- pixel_transparency = (temp_type)max * max - pixel_opacity; \
+ max_squared = (temp_type) max*max; \
+ pixel_transparency = (temp_type)max_squared - pixel_opacity; \
+ output_weight = (pixel_transparency * output4) / max ; \
\
switch(mode) \
{ \
@@ -310,16 +314,17 @@
break; \
case TRANSFER_NORMAL: \
r = (input1 * pixel_opacity + \
- output1 * pixel_transparency) / max / max; \
+ output1 * output_weight) / max / max; \
g = ((input2 - chroma_offset) * pixel_opacity + \
- (output2 - chroma_offset) * pixel_transparency) \
+ (output2 - chroma_offset) * output_weight) \
/ max / max + \
chroma_offset; \
b = ((input3 - chroma_offset) * pixel_opacity + \
- (output3 - chroma_offset) * pixel_transparency) \
+ (output3 - chroma_offset) * output_weight) \
/ max / max + \
chroma_offset; \
- a = input4 > output4 ? input4 : output4; \
+/* a = (max_squared - (((max_squared-pixel_opacity)/max * (max - output4)))) / max ; */\
+ a = max; \
break; \
} \
\
