AmusementClub / vapoursynth-classic

A production video processing framework with simplicity and backward compatibility in mind. We strive to keep 99% R54 API compatibility at the vpy script level while providing better performance & stability than the upstream. API4 compatibility is preserved whenever possible as well.
https://amusementclub.github.io/doc/
GNU Lesser General Public License v3.0
37 stars 4 forks source link

std.MaskedMerge: broken for RGB clips when premultiplied=1 #4

Closed AkarinVS closed 3 years ago

AkarinVS commented 3 years ago

Recent issue 766 is caused by incorrect MaskedMerge for RGB48 clips when premultiplied=1. The issue is workarounded by not using premultiplied MaskedMerge.

However, the underlying issue remains. Here is an analysis of the bug by tracing its history.

Commit ca83ff59f reorganizes MaskedMerge, and it left a bug: https://github.com/AmusementClub/vapoursynth-classic/blob/ca83ff59fd61955a92e93952d2e7abaf4da50638/src/core/mergefilters.c#L468-L475 functions vs_mask_merge_premul_byte_uv_c and vs_mask_merge_premul_word_uv_c were never defined.

Let's compare that with the original code it replaced: https://github.com/AmusementClub/vapoursynth-classic/blob/ba00928bc291ac262265b40aee1672434ddf7a4d/src/core/mergefilters.c#L459-L486

MaskedMerge clearly should behave differently when processing U/V planes as compared to Y/R/G/B planes. Evidently, he code for vs_mask_merge_premul_{byte,word}_c implements the U/V flavor.

Eventually, the missing functions vs_mask_merge_premul_{byte,word}_uv_c were discovered and "fixed" in rev f598bfe51, leaving these strange looking code: https://github.com/AmusementClub/vapoursynth-classic/blob/f598bfe515210fecf3e3b498134a32c18f8210a8/src/core/mergefilters.c#L467-L474 Notice that yuvhandling no longer does anything here.

The code is kept essentially the same until issue 766 was filed and bug isolated to MaskedMerge(premultiplied=1) behavior on RGB48 clips.

AkarinVS commented 3 years ago

Hopefully fixed in R55.A2 release. Only ran some simple test cases and full test suite run still pending.