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.

Sign up for GitHub

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

Jump to bottom

feat: move to big endian for network communications #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: move to big endian for network communications #45

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions src/dmdServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,21 @@ void DMDUTILCALLBACK LogCallback(DMDUtil_LogLevel logLevel, const char* format,
fprintf(stderr, "%s\n", buffer);
}

void convertFromNetwork(DMDUtil::DMD::Update* o) {
// use case ?
Copy link
Collaborator

Choose a reason for hiding this comment

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

The Update struct contains "bigger" data types as well. So I think a conversion would be required here, too:

libdmdutil/include/DMDUtil/DMD.h

Lines 84 to 100 in fc29cd7

struct Update
{
Mode mode;
AlphaNumericLayout layout;
int depth;
uint8_t data[256 * 64 * 3];
uint16_t segData[256 * 64]; // RGB16 or segment data
uint16_t segData2[128];
bool hasData;
bool hasSegData;
bool hasSegData2;
uint8_t r;
uint8_t g;
uint8_t b;
uint16_t width;
uint16_t height;
};

But not all fields are used for all modes. So maybe we can optimize the conversion per mode.

}

void convertFromNetwork(DMDUtil::DMD::StreamHeader* o) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the counterpart is missing. Don't we need to ensure big endian when sending data?

libdmdutil/src/DMD.cpp

Line 339 in fc29cd7

m_pDMDServerConnector->write_n(&streamHeader, sizeof(StreamHeader));

o->mode = (DMDUtil::DMD::Mode) ntohl((uint32_t)o->mode);
o->width = ntohs(o->width);
o->height = ntohs(o->height);
o->length = ntohl(o->length);
}

void convertFromNetwork(DMDUtil::DMD::PathsHeader* o) {
// nothing to do
}

void run(sockpp::tcp_socket sock, uint32_t threadId)
{
uint8_t buffer[sizeof(DMDUtil::DMD::Update)];
Expand All @@ -95,9 +110,9 @@ void run(sockpp::tcp_socket sock, uint32_t threadId)

if (n == sizeof(DMDUtil::DMD::StreamHeader))
{
// At the moment the server only listens on localhost.
// Therefore, we don't have to take care about litte vs. big endian and can use memcpy.
memcpy(pStreamHeader, buffer, n);
convertFromNetwork(pStreamHeader);

if (strcmp(pStreamHeader->header, "DMDStream") == 0 && pStreamHeader->version == 1)
{
if (opt_verbose)
Expand Down Expand Up @@ -125,6 +140,7 @@ void run(sockpp::tcp_socket sock, uint32_t threadId)
{
DMDUtil::DMD::PathsHeader pathsHeader;
memcpy(&pathsHeader, buffer, n);
convertFromNetwork(&pathsHeader);

if (strcmp(pathsHeader.header, "Paths") == 0 &&
(n = sock.read_n(buffer, sizeof(DMDUtil::DMD::Update))) == sizeof(DMDUtil::DMD::Update) &&
Expand All @@ -136,6 +152,7 @@ void run(sockpp::tcp_socket sock, uint32_t threadId)
pathsHeader.name, pathsHeader.altColorPath, pathsHeader.pupVideosPath);
DMDUtil::DMD::Update data;
memcpy(&data, buffer, n);
convertFromNetwork(&data);

if (data.width <= DMDSERVER_MAX_WIDTH && data.height <= DMDSERVER_MAX_HEIGHT)
{
Expand Down Expand Up @@ -166,9 +183,6 @@ void run(sockpp::tcp_socket sock, uint32_t threadId)
threadId == currentThreadId && pStreamHeader->width <= DMDSERVER_MAX_WIDTH &&
pStreamHeader->height <= DMDSERVER_MAX_HEIGHT)
{
// At the moment the server only listens on localhost.
// Therefore, we don't have to take care about litte vs. big endian and can use the buffer as uint16_t as
// it is.
pDmd->UpdateRGB16Data((uint16_t*)buffer, pStreamHeader->width, pStreamHeader->height,
pStreamHeader->buffered == 1);
}
Expand Down