-
Notifications
You must be signed in to change notification settings - Fork 607
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
Add little-endian and big-endian read functions for InputStreams #4038
Conversation
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
stream.ReadBytes(data, nbytes); | ||
return ReadValueImpl<nbytes, is_little_endian>(value, data); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also add the
template <int nbytes, bool is_little_endian>
void ReadValueImpl(float &value, InputStream &stream) {
version too, just like in the case of data ptr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we already (implicitly) have it: if we for example call ReadValueImpl<4, true>(someFloat, stream)
, then it will read stuff from stream
to data
array and call ReadValueImpl<4, true>(someFloat, data)
which will resolve to the specialized float
version. So as far as I understand adding a specialized wrapper wouldn't change anything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you are absolutely right. Sorry for the noise
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
741d784
to
eea37b0
Compare
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
dali/core/byte_io_test.cc
Outdated
@@ -59,6 +60,28 @@ TEST(byte_io, read_value_float) { | |||
EXPECT_EQ(1.0f, ReadValueLE<float>(data_le)); | |||
} | |||
|
|||
TEST(byte_io, read_value_unsigned_int_input_stream) { | |||
const uint8_t data[] = {0x04, 0xd2, 0x1e, 0x61}; // dec 7777 = hex 0x1E61 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const uint8_t data[] = {0x04, 0xd2, 0x1e, 0x61}; // dec 7777 = hex 0x1E61 | |
const uint8_t data_be[] = {0x04, 0xd2, 0x1e, 0x61}; // dec 7777 = hex 0x1E61 |
x86, ARM (and most computers, in fact) natively work with LE, so marking data as "LE" and not marking "BE" as such is misleading. You can adjust the rest of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Signed-off-by: Szymon Karpiński <skarpinski@nvidia.com>
CI MESSAGE: [5280907]: BUILD STARTED |
CI MESSAGE: [5280907]: BUILD PASSED |
Category:
New feature (non-breaking change which adds functionality)
Description:
This PR adds thin overloads of
ReadValueLE
/ReadValueBE
allowing to read data of different endianness directly fromInputStreams
. This will be needed for parsing TIFFs.Additional information:
Affected modules and functionalities:
core/byte_io.h
Key points relevant for the review:
Tests:
New tests are
byte_io_test
:read_value_unsigned_int_input_stream
,read_value_float_input_stream
which also check if the stream pos is moving forward onReadValueBE/LE
.Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: DALI-2862