License/usage conditions of LZ4 depacker seems incompatible to your GNU GPL v3-only

[ecm-pushbx]

Your readme states:

License

The code is licensed under the terms of the GPL, version 3.

It uses Jim Leonard's lz4_8088, which is under the the Demoscene License.

That license on their webpage states:

License

The LZ4 library is provided under the BSD License. However, my code was not derived from any of the original LZ4 code so I can provide it via any license I choose. So, I am providing my code under what I am calling the Demoscene License. The Demoscene License grants you the following rights:

You are free to use this code in any production, commercial or otherwise, without providing remuneration to the author. If you use this code, you must greet "Trixter/Hornet" if used in a demoscene production, or "Jim Leonard" if used in a normal program. Also, you must send email to trixter@oldskool.org telling him you used the code so he can marvel at your result.

Emphasis mine. I believe the "greet" can be considered a request for attribution, which you do provide. However, the email request seems nonfree to me. I don't think it is compatible to the GNU GPL.

(Plug: I do happen to have also written an 8086 assembly LZ4 depacker, which much like the one you use is based on the documentation and not a derivative of any original LZ4 sources. I have provided it under the Fair License as yet, which I believe to be GPL-compatible.)

[foone]

Good point. I was actually thinking of removing the LZ4 code entirely because of issues I'm having with it in my local branch, but I'll try substituting your code to see if it functions equivalently or better. You don't have any example code for interfacing it with C code, do you? That was the main issue I had with Jim Leonard's LZ4 code.

[ecm-pushbx]

You don't have any example code for interfacing it with C code, do you? That was the main issue I had with Jim Leonard's LZ4 code.

No, I don't, but if you have the calling convention you could create a wrapper that calls depack to do its job. However, the code does assume that the destination buffer is below the source buffer. It wouldn't be a major rewrite to allow a different layout however.

If you give me the function protocol you want to have (including what registers to change, what to keep unchanged, buffer layout, and code section name) I could prepare something for you. It should be possible to assemble it into an OMF object using NASM, though you will have to copy my macro collection too.

[ecm-pushbx]

I prepared a repo with all changes needed to make my depacker a drop-in replacement for the other one, hopefully. I did not try building VGAPride with this. Get it at https://hg.pushbx.org/ecm/lz4depak/file/ec11d489af00 - assemble as nasm lz4.asm -f obj -o lz4.obj then use _lz4_depack. It should be called the same way as the prior depacker, or using this declaration:

extern "C" unsigned int far lz4_depack(const void * inbuffer, void * outbuffer);

Because the original depacker didn't accept any buffer size inputs I simply hardcoded both the source and destination buffers as 64 KiB each.

[ecm-pushbx]

Nevermind, this won't work correctly just yet. I need to change how the end of the compressed data is detected.

[ecm-pushbx]

Hacked it a bit, you can try https://hg.pushbx.org/ecm/lz4depak/rev/557731bc70e9

[ecm-pushbx]

The return value in ax is wrong too, though your code doesn't seem to use it.

[ecm-pushbx]

https://hg.pushbx.org/ecm/lz4depak/rev/9acbd3a456b8 fixed the return value (also using dx which was unused otherwise).

[ecm-pushbx]

Ping, do you still want to try using my depacker?

[ecm-pushbx]

I successfully built VGAPride both in the current git head and with modifications to use my depacker instead. Haven't tested it yet because the app I'm using to connect to our server doesn't support graphical display, so will have to test later at home.

These modifications are needed:

• Clone https://hg.pushbx.org/ecm/lz4depak/
• Assemble as nasm -f obj lz4.asm -o lz4.obj -l lz4.lst
• Copy lz4.obj to VGAPride directory
• https://github.com/foone/VGAPride/blob/main/display.cpp#L84 replace lz4_decompress by lz4_depack
• https://github.com/foone/VGAPride/blob/main/display.cpp#L115 same change
• In the TC IDE open the menu Window and select Project
• Select the prior LZ4 decompression assembly file
• Open the menu Project and select "Delete item"
• Open the same menu again and select "Add item..."
• Choose LZ4.OBJ
• Build using menu Compile then "Build all" like usual

[ecm-pushbx]

It seems to run as expected without crashing if I load the program like in ldebug vgapride.exe crab-pride and set a breakpoint at my routine (eg bp new 40C1:CA70 if PSP is at 20ADh), then run it. It breaks in lz4_depack four times, then waits for a keypress.

(This is all testing on the server without graphical output, so I cannot yet verify the correct display.)

[ecm-pushbx]

Tested vgapride crab-pride using a remote diskette on https://www.pcjs.org/machines/pcx86/ibm/5170/vga/cdrom/ and it doesn't seem to work. I will debug it at a later time.

[ecm-pushbx]

Also tested your release executable and that one works, so something is wrong with my depacker certainly.

[ecm-pushbx]

As reported in https://github.com/foone/VGAPride/issues/6 there is some confusion over the LZ4 version to use. I now patched planize.py to use ['lz4', '-f', 'stdin', TEMPFILE], with the version "LZ4 command line interface 64-bits v1.9.4, by Yann Collet" used, and recreated my patched VGAPride, using my depacker (as described in one of the prior comments). It works!

Apparently whatever prior LZ4 version was used created data in the "Legacy frame" format. This did not work with my depacker which expects a current format.

[ecm-pushbx]

Ideally I'd like to modify VGAPride in the following files:

• planize.py (record length of compressed data in a variable per image array)
• graphics.h (add field to the class to track the compressed data length, maybe re-use one of the existing fields?)
• crabs.cpp (as per prior changes, use and pass on the compressed data length)
• display.cpp (pass length of source & destination buffer to LZ4 depacker)

Another nice-to-have feature would be for display.cpp to check the status returned by the depacker. Are you interested in these features?

[ecm-pushbx]

To be fair, the depacker that you originally used is noticeably faster. Loading https://pushbx.org/ecm/test/20221124/diskette.img on a pcjs machine as remote diskette, old.exe crab-pride is faster to draw the planes than new.exe crab-pride.

(I've been trying to get my debugger also included on that diskette to connect to one of pcjs's debugger or terminal windows, which is possible. Goal is to debug VGAPride with serial I/O. Not much luck for now.)

[ecm-pushbx]

I modified the lz4depak repo with a revision picked from its original repo (inicomp), at https://hg.pushbx.org/ecm/lz4depak/rev/ec1486b65a79 This is a slight speed optimisation.

I also made planize.py pass the --best switch to the LZ4 command. This ends up with a smaller executable than your last release. (239_229 bytes with my depacker with lz4 without --best, 224_589 bytes with --best, 226_122 bytes for the last release.) This is in new2.exe of https://pushbx.org/ecm/test/20221124/disknew2.img