bug: make_kernel("binomial", ...) indexes its tap arrays past their allocation for even or fractional extents

Start in make_kernel in src/libOpenImageIO/imagebufalgo.cpp, where the binomial branch sizes wfilter and hfilter from width and height but indexes them over the odd-rounded w and h. The header comment at imagebufalgo.h:1849-1850 documents the odd-size intent. Decide whether even requests should zero-fill or grow to odd, since the issue leaves that open. Done means the tap arrays cover the full image, and a --kernel binomial 4x4 test and a 1x4 test pass with the agreed maximum.

Describe the bug

make_kernel's binomial branch sizes its tap arrays from the extent the caller
asked for, and then indexes them over the odd-rounded image size.

src/libOpenImageIO/imagebufalgo.cpp, at identical line numbers on v3.2.2.0,
v3.1.19.0 and main:

    int w = std::max(1, (int)ceilf(width));                           // :910
    ...
    w |= 1;                                                           // :914
    ...
    } else if (name == "binomial") {                                  // :934
        float* wfilter = OIIO_ALLOCA(float, width);                   // :936
        for (int i = 0; i < width; ++i)
            wfilter[i] = binomial(width - 1, i);                      // :938
        float* hfilter = (height == width) ? wfilter
                                           : OIIO_ALLOCA(float, height);  // :940
        ...
        float* dfilter = OIIO_ALLOCA(float, depth);                   // :944
    ...
        for (ImageBuf::Iterator<float> p(dst); !p.done(); ++p)
            p[0] = wfilter[p.x() - spec.x] * hfilter[p.y() - spec.y]  // :951
                   * dfilter[p.z() - spec.z];

The image is w wide, where w is width rounded up and then forced odd. The
tap array is width long. The loop at :951 indexes it with
p.x() - spec.x, which runs over [0, w).

So for a width of 4 the image is 5 wide and the array holds 4 floats, and the
last column reads wfilter[4], one float past the allocation. For a fractional
width the fill overruns as well: OIIO_ALLOCA(float, 4.5) reserves 18 bytes
and for (int i = 0; i < 4.5; ++i) writes five floats into them, so that case
is a write past the allocation and then a read past it.

height has the same problem through hfilter, and the two share one array
when they are equal, so a square even request gets it on both axes.

Two regimes, and they look different from the outside

This matters for reproducing it, because only one of the two varies:

• An even or fractional width overruns wfilter, which is the last of the
  three allocations, so the stray tap comes from unrelated stack and the kernel
  differs from run to run.
• An even or fractional height with an odd width overruns hfilter, whose
  overrun lands in a neighbouring allocation, so the kernel is stably wrong
  rather than unstable. --kernel binomial 1x4 returns a maximum of
  0.333333 on every run where the intended 4 tap binomial [1,3,3,1]/8 has a
  maximum of 0.375.

Both are the same defect. The second is the one unsharp_mask reaches.

OpenImageIO version and dependencies

OIIO 3.2.2.0 | Windows/x86_64
    Build compiler: MSVS 1951 | C++17/199711
    HW features enabled at build: sse2,sse3,ssse3,sse41,sse42
    No CUDA support (disabled / unavailable at build time)
Dependencies: BZip2 1.0.8, DCMTK NONE, FFmpeg NONE, fmt 12.1.0, Freetype 2.13.3, GIF 5.2.2, Imath 3.2.2, JXL NONE, libdeflate 1.25, Libheif NONE, libjpeg-turbo 3.1.3, LibRaw NONE, libuhdr NONE, OpenColorIO 2.5.1, OpenCV NONE, OpenEXR 3.4.7, OpenJPEG 2.5.4, openjph 0.26.3, PNG 1.6.55, Ptex NONE, Ptex NONE, Robinmap 1.4.1, TBB NONE, TIFF 4.7.1, WebP 1.6.0, ZLIB 1.3.1, zstd 1.5.7

Nothing unusual about the environment and no nonstandard build options. The
cited lines are identical on v3.2.2.0, v3.1.19.0 and main.

To Reproduce

oiiotool only, no input files. Run the even case several times, because a
single run is as likely as not to look correct:

for i in $(seq 1 20); do
    oiiotool --kernel binomial 4x4 --printstats 2>&1 | grep "Stats Max"
done

# The stable half of the same defect:
oiiotool --kernel binomial 1x4 --printstats     # max 0.333333, every run
                                                # intended [1,3,3,1]/8 -> 0.375

# The control. An odd extent is well behaved:
oiiotool --kernel binomial 3x3 --printstats     # min 0.062500 max 0.250000
                                                # avg 0.111111, every run

Expected: the same numbers every time, with 4x4 giving a maximum of
0.140625, since a correct 4 tap binomial kernel is the outer product of
[1,3,3,1] with itself normalised by its own sum of 64, whose largest element
is 9/64.

Evidence

A summary of the Stats Min and Stats Max lines from 20 fresh oiiotool
processes, same command each time, on the 3.2.2.0 build above. Full
--printstats output per run is available if it is wanted; these are the two
lines that move.

maximum       runs
0.140625        12     correct
1.000000         5     one cell near 1, the rest near 0: a delta, not a blur
-0.000000        2     the whole kernel is zero or negative
0.125335         1     quietly wrong, with minimum 0.003128

Two of those rule out an innocent explanation.

The two runs whose maximum is -0.000000 cannot be a kernel at all. A
binomial kernel is a product of binomial coefficients, every one of which is
positive, so a kernel whose largest value is not positive did not come from the
intended inputs. Convolving with it returns a black image.

The run at 0.125335 is the quiet case. The stray tap happened to be small,
the normalisation absorbed most of it, and the result is wrong in the second
digit while looking plausible.

The 3x3 control is stable across every run.

The fix

Size the arrays by the rounded extents rather than the requested ones, and fill
the whole of them:

    float* wfilter = OIIO_ALLOCA(float, w);
    for (int i = 0; i < w; ++i)
        wfilter[i] = binomial(w - 1, i);
    float* hfilter = (h == w) ? wfilter : OIIO_ALLOCA(float, h);
    if (h != w)
        for (int i = 0; i < h; ++i)
            hfilter[i] = binomial(h - 1, i);
    float* dfilter = OIIO_ALLOCA(float, d);

depth needs the same treatment for consistency, though d is 1 in every
in-tree use.

This changes what an even request returns: a 4 by 4 becomes the 5 tap binomial
[1,4,6,4,1] centred in its 5 by 5 image, rather than a 4 tap one sitting off
centre with a zero column. That seems the better answer given
imagebufalgo.h:1849-1850, which says the size is "rounded up to odd
resolution so that the center of the kernel can be at the center of the middle
pixel", but it is a judgement rather than a correctness matter, and the
alternative is to keep the current intent by zero filling [width, w). Either
way, nobody can be relying on the even-width result today, because it is not
the same twice.

Where unsharp_mask reaches it

unsharp_mask builds its blur with make_kernel(kernel, 1, width) above a
width of three and make_kernel(kernel, width, width) at or below it. So
--unsharp:kernel=binomial:width=4 goes through the 1xN shape, which is the
stable regime: the kernel is deterministically wrong rather than variable.
--unsharp:kernel=binomial:width=2 takes the square path and is variable.

Test coverage

There is none. git grep binomial testsuite/ returns nothing, so no test asks
for a binomial kernel at any extent, even or odd. A regression test would be
--kernel binomial 4x4 asserting whichever maximum the chosen fix produces,
next to the 3x3 case and a 1x4 case.

Provenance

The binomial branch was added in e92a191c4 (2013-04-26), merged as #585, and
the sizing mismatch dates from that commit: the branch sizes its tap arrays
from the unrounded request while indexing them over an image that the
surrounding code had already rounded to odd. The odd rounding itself is older
and was only respelled as w |= 1 by that commit. The defective lines have
changed only cosmetically since, by 400e2340c (clang-format, 2018-11-14) and
0bc7e2bbe (ALLOCA to OIIO_ALLOCA, 2019-10-31).