-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhancement: add extra encoding/decoding half precision floating point functions to public API #149
Comments
Right, half-float is often used for storage only, while all the manipulation is done on single- or double-precision. I agree on adding those functions, but let's put each pair on a separate .c file from cborparser.c and cborencoder.c. That way, those two remain floating-point-free. |
Partially implemented. Is this OK. or some amendment is needed? |
Yes, it looks very good. I have one overall comment and two very minor. Overall, I think the Thanks for the tests, they look nice. The two minor comments:
|
Look at the improvement made so far. But I'm in doubt now: I have used ldexpf function, which is not accessible in plain old C89 and some antique compilers. |
Let's do bit manipulation directly. I wonder if there's a |
Do you mean to replace the |
static inline float decode_halff(unsigned short half)
{
int exp = (half >> 10) & 0x1f;
int mant = half & 0x3ff;
float mantf, expf, val;
if (exp == 0) {
mantf = mant;
expf = 1.0f / (1 << 24);
val = mantf * expf;
} else if (exp != 31) {
mantf = mant + 1024.0f;
expf = exp >= 25 ? 1 << (exp - 25) : 1.0f / (1 << (25 - exp));
val = mantf * expf;
} else {
val = mant == 0 ? INFINITY : NAN;
}
return half & 0x8000 ? -val : val;
} |
If that produces proper results, that's fine. I was thinking of converting the |
So, there are 4 ways with their pro-s and contra-s:
#ifndef CBOR_NO_PLATFORM_IEEE754_FLOATING_POINT
CBOR_INLINE_API CborError cbor_value_get_float(CborEncoder *encoder, float *value);
CBOR_INLINE_API CborError cbor_encode_float(CborEncoder *encoder, float value);
// ... for half and double
#else
// Caveat lector: IEEE754 floating point representation is assumed
CBOR_INLINE_API CborError cbor_value_get_float(CborEncoder *encoder, uint32_t *value);
CBOR_INLINE_API CborError cbor_encode_float(CborEncoder *encoder, uint32_t value);
// ... for half and double
#endif What is the preferable way? |
CBOR is defined as carrying IEEE754 binary64, binary32 and binary16 content. So I like your idea. I don't think we need to provide the |
OK, but I think, this is the task for separate feature request. As for now, let's roll back to plain old doubles (ldexp, not ldexpf) both for cbor_value_get_half_float_as_float and cbor_value_get_half_float_as_double. |
I don't think we need the |
Well, cbor_value_get_half_float_as_float needs a pointer to float, so user should do extra effort. Hope, nobody could try something like: double x;
/* Oops... I know a guy, who can try it without any doubt */
cbor_value_get_half_float_as_float(cbor_value, (float*)&x); But in name of API brevity |
If someone wants to shoot themselves in the foot, who are we to say no? :-) |
Implemented. I'll squash the branch into the single commit before pull request. |
…nt data as single float Motivation: half-precision floating point format is used to minimize storage and traffic mostly. Application level manipulates with single and double precision usually. So, two routines added to public API to encode/decode given single precision value in the half precision format
…ta as single float Motivation: half-precision floating point format is used to minimize storage and traffic mostly. Application level manipulates with single and double precision usually. So, two routines added to public API to encode/decode given single precision value in the half precision format Signed-off-by: S.Phirsov Signed-off-by: Thiago Macieira <[email protected]>
Hello @x448 |
@thiagomacieira Oops! Deleted. Thanks for letting me know. |
…ta as single float Motivation: half-precision floating point format is used to minimize storage and traffic mostly. Application level manipulates with single and double precision usually. So, two routines added to public API to encode/decode given single precision value in the half precision format Signed-off-by: S.Phirsov Signed-off-by: Thiago Macieira <[email protected]>
It's uncommon to operate half precision floating point data on application level, while such format is widely used on transport level to minimize traffic. In such case one can need API for encoding/decoding half-precision floating point data AS more common single or double. Something like this:
There is some related stuff in the private area, and I think, I can make it public.
The text was updated successfully, but these errors were encountered: