Skip to content
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

Fix ne_read_float on big-endian machines. #65

Merged
merged 1 commit into from
Aug 21, 2019
Merged

Fix ne_read_float on big-endian machines. #65

merged 1 commit into from
Aug 21, 2019

Conversation

kinetiknz
Copy link
Collaborator

@kinetiknz kinetiknz commented Aug 19, 2019

Closes #64.

Verified fix via qemu-user with a MIPS build. make check passes.

r? SingingTree please

@kinetiknz kinetiknz self-assigned this Aug 20, 2019
@kinetiknz
Copy link
Collaborator Author

Oops, I forgot the @.

r? @SingingTree please :)

@@ -834,7 +834,15 @@ ne_read_float(nestegg_io * io, double * val, uint64_t length)
{
union {
uint64_t u;
float f;
struct {
#if __FLOAT_WORD_ORDER__ == __ORDER_BIG_ENDIAN__
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears there's broad support for these macros. I've tested a toy program with clang/gcc/msvc and they work -- but I haven't worked with these much, so wanted to highlight them in case you have any further thoughts.

Copy link
Collaborator Author

@kinetiknz kinetiknz Aug 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Yeah, it's possible this won't be defined on some particular platform/compiler but the most common/likely ones are covered. In the event that this fix breaks for a big-endian system with a compiler that doesn't define __FLOAT_WORD_ORDER__, it is covered by the included regression tests (e.g. detodos.webm will fail) so it can, at least, be found and fixed quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WebM Opus rate is 0 on big endian machines
2 participants