Bugs in ieee754.c about qcbor HOT 12 CLOSED

Yes, this is true. Here's the comment from the ieee754.c
// This will not convert to half-precion or single-precision
// subnormals. Values that could be converted will be output as
// the double they are or occasionally to a normal single. This
// could be implemented, but it is more code and would rarely be
// used and rarely reduce the output size.

QCBOR is still producing a correct value, just not the smallest possible for a few cases.

One could say that QCBOR is not producing correct deterministic encoding of floats because of this. (Another area of concern for deterministic floats encoding is NaN payloads; not even sure there
is enough of a standard for them).

Did your d-cbor code use PLATFORM_SUPPORTS_FLOAT_CAST? If so it relies on the floating point HW to do the work and will of course be less code.

Thanks for reporting. I'm not sure if I'll fix this or when as it will take some time to fix, but I will leave the issue open.

from qcbor.

Hi @laurencelundblade
I have read the docs :)

AFAICT, my take on IEEE754_DoubleToSmallestInternal generates short(er) code also when not relying on floating point HW. BTW, doesn't your code actually depend on that as well?
https://github.com/laurencelundblade/QCBOR/blob/master/src/ieee754.c#L491

Regarding NaN, RFC8949 is a bit unclear on that. In the Java platform it is not technically possible setting any kind of payload to NaN so using this feature is asking for problems. In my take on the matter NaN is f97e00, everything else is considered an error.

If you want, I could imagine integrating my code in ieee754.c.

from qcbor.

Hi Anders,

One problem with your code is that it is the Apache license which is not really compatible with QCBOR's MIT license.

Maybe we should see clarification on NaN deterministic encoding in the WG? There's also the quiet NaN and non-quiet NaN thing.

from qcbor.

Hi Laurence,
If I would port the code it would have to follow QCBOR licensing. There is also a minor formatting difference. I have used K&R brace style.

NaN clarifications is probably not going to happen because that is too late. The D-CBOR specification is supposed to address issues that are unclear and some that doesn't fit at all like using floating point numbers as keys. Well, actually the latter works perfect but RFC8949 destroyed it by requiring numeric comparisons which is an unnecessary complication since CBOR objects are just strings of bytes and map key comparisons only have to treat them as such.

from qcbor.

FWIW, the floating point reduction algorithm has just been updated and doesn't rely on loops anymore:
https://github.com/cyberphone/openkeystore/blob/90736d4420aafff9aea5b8cfab19bc863ab8a1d0/library/src/org/webpki/cbor/CBORFloat.java#L117

BTW, given Carsten't recent upgrade of https://cbor.me, deterministic CBOR seems to be a deal done. QCBOR would fit very nicely in this scheme.

from qcbor.

Hi Anders,

Been busy with COSE these days. Hope to get back to QCBOR after IETF 117 in July.

Hopefully some of the deterministic stuff and other can get done then.

from qcbor.

Hi Anders,

Finally getting around to looking at this carefully. I like your approach to numeric reduction for double to float to half. It seems like less object code than what I have. I'd like to try integrating it into QCBOR.

It's all code you wrote, right? You have the full rights to change the license to BSD, right? If so, maybe you can put it in GitHub with the BSD 3 clause license?

For now this is primarily about implementing RFC 8949, not CDE or dCBOR, though I expect to move on to CDE and dCBOR.

Thx

LL

from qcbor.

Hi Laurence,
I wrote the code from scratch (but looked a little bit into other implementations to get "inspiration"), so I can provide it to you using whatever license you want. I have recently upgraded the algorithm so that there are no loops anymore. Shifts does the same job. I will look into this next week,

Java version of the updated algorithm:
https://github.com/cyberphone/openkeystore/blob/dcc068256034239536fee5b6c2b2ab5af98b7f07/library/src/org/webpki/cbor/CBORFloat.java#L52

Regards,
Anders

from qcbor.

Updated algorithm in C:
https://github.com/cyberphone/D-CBOR/blob/main/lib/d-cbor-ieee754.c
https://github.com/cyberphone/D-CBOR/blob/main/ieee754-test/ieee754-test.c

from qcbor.

This might be the right time for "gearing up" for CDE/dCBOR. Since I have already implemented the CDE versus "Legacy" decoder switch in both Java and JavaScript, I could easily do that for QCBOR. However, I think we need to talk a bit before I take any further steps. Would WhatsApp be OK?

For decoding I have "cheated" (well...) by first converting F16, F32, and F64 to double and then run the double-to-shortest encoder and see if they match on a binary level. I'm in favor of shortest possible (source) code :)
https://github.com/cyberphone/CBOR.js/blob/2b9dcde6da61d19f40f3ac892730ca7f5a62147c/src/cbor.js#L880

from qcbor.

Hi Anders, what I'd like is just a simple posting of your C code with the BSD license. https://github.com/cyberphone/D-CBOR/blob/main/lib/d-cbor-ieee754.c

I like the way it works and handles subnormals and want to work it into QCBOR.

Thx

from qcbor.

Fixed by #192. I ended up writing all the code on my own from scratch.

from qcbor.