ZSTD_error_dstSize_tooSmall when stream-decompressing with ZSTD_d_stableOutBuffer

[fgenesis]

Related to #2094 and maybe #2723, regarding ZSTD_d_stableOutBuffer. Using zstd 1.5.1 on win10 64bit, MSVC 2019.

Context:
I'm trying to use streaming zstd in a use case very similar to https://fgiesen.wordpress.com/2011/11/21/buffer-centric-io/ (Ctrl+F cool thing for extra context). So my primary goal is to stream-decompress small-ish blocks of data to keep memory usage low.

The TL;DR of the link above is that no user output buffer (and thus, no extra copy from the window) is needed when you can directly expose the LZ77 window and give the user a pointer to that.

I allocate a buffer of frameHeader.windowSize bytes for the window, set ZSTD_d_stableOutBuffer, pin the output + size and then start decompressing. A small file works fine but as soon as the decompressed data is too big to fit into the window, it fails with ZSTD_error_dstSize_tooSmall just after parsing the header internally.

The error is set here (in zstd_decompress.c)

            /* Check output buffer is large enough for ZSTD_odm_stable. */
            if (zds->outBufferMode == ZSTD_bm_stable
                && zds->fParams.frameType != ZSTD_skippableFrame
                && zds->fParams.frameContentSize != ZSTD_CONTENTSIZE_UNKNOWN
                && (U64)(size_t)(oend-op) < zds->fParams.frameContentSize) {
                RETURN_ERROR(dstSize_tooSmall, "ZSTD_obm_stable passed but ZSTD_outBuffer is too small");
            }

My window size is ~2MB (as taken from the header; was compressed with level 3) and frameContentSize is 18 MB.
Why is this check there? Shouldn't it it check the window size instead? And the window is large enough to handle any back-references it enounters so that shouldn't be an issue.
The way the code works now effectively breaks chunked streaming with ZSTD_d_stableOutBuffer when the output size is known.

Since my buffer is exactly window-sized and externally read-only, it should be possible to decompress one window worth of data, hand a pointer to the user for reading directly from the window, then resume decompression, overwrite the window, hand it to the user again, and so on.

I'm doing this -- pseudo-ish code, simplified, based on the streaming example:

// this function is called whenever the user requests new decompressed data:
decompSome(z) { // z = ... context, pointers, output, etc. heap-allocated. z->dc is the zstd stream
    N = 0; // # of bytes decompressed
    for (;;)
    {
        p = pointer to some compressed data; streamed in
        avail = # bytes available in p; at least 1 byte.

        ZSTD_inBuffer input = { p, avail, 0 };
        for (;;)
        {
            size_t const oldpos = z->output.pos == z->output.size ? 0 : z->output.pos; // handle window wraparound
            size_t const ret = ZSTD_decompressStream(z->dc, &z->output, &input);
            if (ZSTD_isError(ret))
               return fail(....):
            N += z->output.pos - oldpos;
            p += input.pos;
            if (!ret) // all done?
                goto eof;
            if (z->output.pos == z->output.size) // window full? store state for next time
                goto out;
            if (input.pos == input.size) // input consumed, get more
                break;
        }
    }
eof: ...
out: store state, hand the user (z->output.dst, N)
}

Am i overlooking something, mis-using the API, or is this a bug?

I have this working with an inflater using miniz and also wrote a quick'n'dirty streaming LZ4 decoder that exposes the window like this.

Would be nice to be able to do the same with zstd.

[Cyan4973]

Taken from the documentation :

Tells the decompressor that the ZSTD_outBuffer will ALWAYS be the same
between calls, except for the modifications that zstd makes to pos (the
caller must not modify pos)

Effectively, the way ZSTD_d_stableOutBuffer works
is that it requires the entire output to remain stable all the time.
It's not allowed to move the data around.
As a consequence, the caller must have enough room to contain the entire frame content, not just the window size.

It seems what you are expecting is outside the current contract of ZSTD_d_stableOutBuffer.
cc @terrelln

[fgenesis]

Yeah, i figured as much by now after reading into the decompressor a bit more. I think i've found a nice way to expose the LZ window to the caller:
In a nutshell, i've renamed ZSTD_d_stableOutBuffer to ZSTD_d_outBufferMode, made that take an enum (buffered|stable|expose), and adjusted the inner decomp loop so that instead of calling memcpy when flushing it'll just set output->{buf,size} and exits afterwards. Pretty minimal code changes. Didn't expect it to be so hackable, very nice.

The API contract now is that with the new flag value set the user has to accept whatever ends up in the output struct if output->size > 0, ie. output->dst stays valid until the next call to decompress.
Looks rather clean this way, imho.

If there's any interest in this i'll clean this up a bit, test it properly, and make it a PR.

(Might also want to rename ZSTD_c_stable{In|Out}Buffer to ZSTD_c_{in|out}BufferMode for consistency if this goes in)

Thanks!