Skip to content

Conversation

@dnachbaur
Copy link
Contributor

No description provided.

@dnachbaur dnachbaur requested a review from rdumusc August 3, 2017 13:00
// and fulfill the promise already to reduce load in the send thread
std::promise<bool> promise;
promise.set_value(true);
return promise.get_future();
Copy link

Choose a reason for hiding this comment

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

OK, not sure why though? Is it because promise.set_value(true); takes too much time on KNL?

  • move "_requests.enqueue({nullptr, tasks, false});" below the comment
  • return make_ready_future(true); ?

unsigned int y;
};
std::vector<Segment> segments;
for (unsigned int i = 0; i < width + segmentSize - 1; i += segmentSize)
Copy link

Choose a reason for hiding this comment

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

Hmmm, these termination condition look complicated, I think it should be just i < width and j < height. Maybe add:
BOOST_REQUIRE_EQUAL(numSegments, std::ceil((float)width / segmentSize) * std::ceil((float)height / segmentSize));
Also, it see that it does not matter too much here, but if you had checked the size of the received frame it's height would not have been correct because (1080 % 64) != 0 (the tile of the last row are too high)


const unsigned int byte = segmentSize * segmentSize * 4;
std::unique_ptr<uint8_t[]> pixels(new uint8_t[byte]);
::memset(pixels.get(), 0, byte);
Copy link

Choose a reason for hiding this comment

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

std::vector<uint8_t> pixels(bytes, 0); ?

Copy link

@rdumusc rdumusc left a comment

Choose a reason for hiding this comment

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

All good!

@dnachbaur dnachbaur merged commit c3cb80b into BlueBrain:master Aug 4, 2017
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.

2 participants