Add wcclient.get_bytes() to Berry

[btsimonh]

(cherry picked from commit 5903b21)

Description:

Add webclient.get_bytes() to Berry to allow binary files to be read from the web.

My first time modifying the Berry native code, so pls confirm I did it right?

Checklist:

• The pull request is done against the latest development branch
• Only relevant files were touched
• Only one feature/fix was added per PR and the code change compiles without warnings
• The code change is tested and works with Tasmota core ESP8266 V.2.7.4.9
• The code change is tested and works with Tasmota core ESP32 V.2.0.9
• I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

[btsimonh]

I was looking at multipart upload too, but it's too complex, so just the above is enough.

[s-hadinger]

Very nice, thanks.

[s-hadinger]

This is an excellent start. It could be possible to optimize further and to have the bytes() buffer allocated first and the stream writing into it. This would avoid a double allocation of memory.

[btsimonh]

Is that not what be_pushbuffer does? I'd assumes this allocated the bytes, and that be_pushbytes just confirmed it on the stack.
But maybe we don't even want the be_pushbytes at all? Can you check what the two functions do?
I have a nasty feeling that we're adding two things to the stack...

[s-hadinger]

We're indeed pushing and allocating twice the buffer on the heap.

To make it only once is a little trickier:

• instanciate a new bytes() object and pass the size to force allocation of the buffer
• call resize() to force the internal buffer to be the right size
• call _buffer() to get the pointer of the buffer in memory
• fill the buffer with the data
• (opt) call resize() again if the data is actually shorter than expected

One example is here:

static int i_readbytes(bvm *vm)
{
    int argc = be_top(vm);
    be_getmember(vm, 1, ".p");
    if (be_iscomptr(vm, -1)) {
        void *fh = be_tocomptr(vm, -1);
        size_t size = readsize(vm, argc, fh);
        if (size) {
            /* avoid double allocation, using directly the internal buffer of bytes() */
            be_getbuiltin(vm, "bytes");
            be_pushint(vm, size);
            be_call(vm, 1);  /* call bytes() constructor with pre-sized buffer */
            be_pop(vm, 1);  /* bytes() instance is at top */

            be_getmember(vm, -1, "resize");
            be_pushvalue(vm, -2);
            be_pushint(vm, size);
            be_call(vm, 2); /* call b.resize(size) */
            be_pop(vm, 3);  /* bytes() instance is at top */

            char *buffer = (char*) be_tobytes(vm, -1, NULL); /* we get the address of the internam buffer of size 'size' */
            size = be_fread(fh, buffer, size);

            /* resize if something went wrong */
            be_getmember(vm, -1, "resize");
            be_pushvalue(vm, -2);
            be_pushint(vm, size);
            be_call(vm, 2); /* call b.resize(size) */
            be_pop(vm, 3);  /* bytes() instance is at top */
        } else {
            be_pushbytes(vm, NULL, 0);
        }
        be_return(vm);
    }
    be_return_nil(vm);
}

[btsimonh]

Theoretically, we could cope with content-length -1 by expanding the byte buffer - not efficient, but maybe better than not allowing?

[s-hadinger]

I don't understand. Expanding a buffer always results in a new allocation. So it's better to allocate the right size in the first place

[btsimonh]

yep, but if there is no content-length header, we don't know how much we're going to get :). Obviously it's better to have a content-length.... but we should cope without, even if it's more expensive.

[s-hadinger]

I wouldn't support binary read if Content-Length is not present. Unless you have a specific use case for it?

[sfromis]

Except for images, most binary reads could be well-served by having a fixed initial buffer of a "reasonable" size, as an argument to get_bytes, and then throw an error if exceeded.

Of course, well-behaved servers "should" provide a length, but I did observe that webcam snapshot.jpg (which I some time ago contemplated using from another Tasmota device) is not among those, instead having ContentLength=-1.

[btsimonh]

haha :). I will fix that! The new driver is coming on nicely... I'm now considering whether to make a true berry class for it, or continue to work with evil Tas commands that return native addresses and lengths in json. But I don't really know where to start on adding a Berry class to the webcam driver. (I DO call functions on a berry class if it exists, and that works nicely if you create the class in a berry script)..

[btsimonh]

see new PR based on your advice and some research :).
The StreamBeBytesWriter(vm, incr) may be useful for other things as well?

[s-hadinger]

Always handy to have it. Thanks for this.