Mir

Merge lp:~mir-team/mir/android-input-transport-scatter-send-event into lp:mir

android-input-transport-scatter-send-event
Merge into development-branch

Proposed by Brandon Schaefer on 2015-12-03

Status:	Rejected
Rejected by:	Brandon Schaefer on 2016-01-14
Proposed branch:	lp:~mir-team/mir/android-input-transport-scatter-send-event
Merge into:	lp:mir
Diff against target:	316 lines (+165/-9) 2 files modified 3rd_party/android-input/android/frameworks/base/include/androidfw/InputTransport.h (+23/-2) 3rd_party/android-input/android/frameworks/base/services/input/InputTransport.cpp (+142/-7)
To merge this branch:	bzr merge lp:~mir-team/mir/android-input-transport-scatter-send-event
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Mir CI Bot	continuous-integration		Approve on 2016-01-08
Alan Griffiths			Needs Fixing on 2015-12-15
Daniel van Vugt			Needs Fixing on 2015-12-14
PS Jenkins bot (community)	continuous-integration		Approve on 2015-12-12
Alberto Aguirre (community)			Approve on 2015-12-11
Chris Halse Rogers		2015-12-03	Approve on 2015-12-07
Review via email: mp+279387@code.launchpad.net

Commit message

Move to using a scatter/gather when sending input message. This will make sending a uint8_t* mac + uint32_t mac_size much simpler (and better).

Description of the change

Move to using a scatter/gather when sending input message. This will make sending a uint8_t* mac + uint32_t mac_size much simpler (and better).

This is the beginning stages. All the casting to unit8_t* from a uint64_t location will be removed and replaced with a std::vector.data() call. Makes the core changes seen (with out bloating to much).

lp:~mir-team/mir/android-input-transport-scatter-send-event updated on 2015-12-04

3160. By Brandon Schaefer on 2015-12-04: * Merge trunk

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-12-04:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5778/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-12-04:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5779/rebuild

review: Approve (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2015-12-07:

+ /* Anything after the Pointer struct WILL BE CUT OFF if you send the entire
+ * struct. As the size() function returns only valid mouse pointers (leaving the rest off)
+ * So if you add to the struct insert before or manually re-construct the extra bits.
+ */

Could probably be reworded to be a bit clearer?

Maybe:
/* As an optimisation sendMessage() only sends the valid members of the pointers[]
* array. This is implemented by size() only counting pointerCount members of
* the pointers[] array.
*
* As a side effect, everything after the pointers[] array will NOT be reliably sent
* in sendMessage(). Any members placed after pointers[] will need to be reconstructed
* in recieveMessage().
*/

Otherwise seems fine.

review: Approve

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2015-12-07:

This is an input protocol break (remember last time you changed these?).

If we intentionally break part of the protocol then there are version macros that need updating. Although I would recommend not breaking it at all if you can help it.

review: Needs Fixing

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2015-12-07:

Hmm, last time we changed these structures we had to ask app developers to rebuild re-release their click packages to match the new input protocol.

This needs a deep full-team discussion, if not deferring/rejecting for now...

Revision history for this message

Chris Halse Rogers (raof) wrote on 2015-12-07:

On Monday, 7 December 2015 7:35:59 PM AEDT, Daniel van Vugt wrote:
> Hmm, last time we changed these structures we had to ask app
> developers to rebuild re-release their click packages to match
> the new input protocol.
>
> This needs a deep full-team discussion, if not
> deferring/rejecting for now...

My memory of the outcome last time was “embedding libmirclient is not
supported; when we break it you get to keep both pieces”. Has anything
changrd to make it different this time?

--
Sent using Dekko from my Ubuntu device

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2015-12-07:

I'm not sure we have yet convinced/coerced app developers to stop shipping their own libmirclient.

Assuming we haven't, we should probably pause for a moment to consider the plan of attack. Because most things are better than breaking apps already in the store (again).

Disapproved for safety, till we're absolutely sure the same pain isn't going to happen all over again.

review: Disapprove

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2015-12-07:

I'm not familiar with the term "scatter/gather" in this context. What is the motivation here? If we're changing the serialization, then it might make sense to switch to the protobuf serialization we use elsewhere.

review: Needs Information

Revision history for this message

Brandon Schaefer (brandontschaefer) wrote on 2015-12-07:

@duflu

From what I know we do not support the protocol. ie. We are free to break it, and make no guarantee it. From a public point of view.

Though if apps in the store are using it (against our guarantee). The yes we should approach a different plan of attack. Not sure the best one. As we *will* need to break the protocol again to support 160bit hash. How ever may it be.

@alan

The point of this scatter/gather is so we can send an arbitrary size hash over the protocol. As the 160bit hash is subject to being changed. We are looking at moving to a std::vector to hold the hash.

Issue being unions in InputMessage stop us from just using a vector. This is the approach around that. There will also be another issue in event_private, which we are moving MirEvent over to use boost::varaint so we can have a tagged union (which will then support std::vector!).

As far as moving to protobuf for serialization here... not sure. Im happy to do the best thing :). So Ill talk with RAOF. Though is protobuf the best for events? (Not really sure)

So right now the goal of this is:
1) Support sending a uint8_t* through the android socket to the client (this branch)
2) Move to using boost::variant in place of MirEvent (an attempt tagged union atm). This will allow us to store non-trivial objects in the MirEvent structs. As well as makes things type-safe (yay for us).
3) Move to using std::vector as a way to hold the MAC hash so we dont hard code 160bits. (subject to change)
4) Move to actually pushing the 160bit SHA-1 hash through to the vectors (which at this point will be support!)

Possibly best I write up a proposal?

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2015-12-07:

> As far as moving to protobuf for serialization here... not sure. Im happy to
> do the best thing :). So Ill talk with RAOF. Though is protobuf the best for
> events? (Not really sure)

On the basis of our testing protobuf is unlikely to be "best" for performance, it would just be consistent (and does have, for example, some support for compatibility between versions). I'm sure RAOF will give you the background story.

lp:~mir-team/mir/android-input-transport-scatter-send-event updated on 2015-12-07

3161. By Brandon Schaefer on 2015-12-07: * Change comment (reads much better)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-12-07:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5791/rebuild

review: Approve (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2015-12-08:

Hey don't shoot the messenger. I'm just the poor bugger who untangled the problem last time so I remember it probably more than others.

That said, input protocol changes might be OK now providing (and we should verify) that the offending click packages:

1. use libmirclient9 (and only nine); and
2a. do not contain their own copy of libmirclient9; or
2b. will use the newer system version of libmirclient9 we provide instead of their own one.

Sounds feasible so we might be free to break the input protocol now.

review: Needs Information

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2015-12-08:

I don't have reason to block, but I've not the time to look closer (I'm on vacation the rest of this week).

review: Abstain

Revision history for this message

Brandon Schaefer (brandontschaefer) wrote on 2015-12-08:

Not trying to shoot :). So first thing first (before we look at approving this). Is to test if we would offend any click packages.

We also talked about rejecting click packages that bundle their own libmirclient. Which if that were the case, we can break away?

Ill try to talk with you later to try and figure out if this is OK to do atm, or figure out what will be needed to be done!

Thanks!

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2015-12-09:

I just had to find the history of the problem again. It's all in --> bug 1507982

Sounds like it probably won't happen again so long as no packages are using libmirclient8 or earlier. The affected apps of bug 1507982 have been rebuilt and re-released against libmirclient9. So that should be OK. The only remaining concern is that the problem could still happen in theory even if people don't package their own libmirclient, because bug 1498281 is still open.

In summary, it sounds like we are free to break the input protocol now. Just so long as we don't bump to libmirclient10 in the foreseeable future. This way all existing native apps that use libmirclient9 will just get the new libmirclient9 and protocol changes will be transparent.

review: Abstain

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2015-12-11:

Comments inline.

review: Needs Fixing

lp:~mir-team/mir/android-input-transport-scatter-send-event updated on 2015-12-11

3162. By Brandon Schaefer on 2015-12-11: * Address reviewer
3163. By Brandon Schaefer on 2015-12-11: * Rework the using of the empty_mac so we set the correct size

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2015-12-11:

LGTM

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-12-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5827/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-12-12:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5828/rebuild

review: Approve (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2015-12-14:

(1) Is 1000 a sensible number? That would allow up to 8000 bits which is a strange number. You might want something that is (2^n/8) bytes to avoid wastage.

(2) I just realized; having a (relative) pointer in an over-the-wire structure adds unnecessary complexity and risk:
+ uint8_t* mac;
The structure is meant to represent what goes over the wire, but it can't now and you have an unnecessary dance to support it.

Please just change it to a fixed size like "uint8_t mac[512]; // supports up to 4096 bits".

That will nicely restore clear over-the-wire opaque copy support, and so simplify the code a lot too.

review: Needs Fixing

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2015-12-14:

(2) Or if you really do need variable size support, make it clearer using "ptrdiff_t mac_offset" and add a getter method that is "uint8_t* mac() const".

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2015-12-15:

> (2) I just realized; having a (relative) pointer in an over-the-wire structure
> adds unnecessary complexity and risk:
> + uint8_t* mac;
> The structure is meant to represent what goes over the wire, but it can't now
> and you have an unnecessary dance to support it.
>
> Please just change it to a fixed size like "uint8_t mac[512]; // supports up
> to 4096 bits".
>

Umm but that's the whole point of this MP, to use the scatter/gather support in the kernel so that you don't have to copy the mac into the structure when sending.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2015-12-15:

OK codewise, but note this is a protocol break that isn't a client ABI bump.

Before release we need to update mir::protobuf::oldest_compatible_protocol_version() and possibly mir::protobuf::current_protocol_version() (the latter only if we don't add any client API functions and bump MIR_CLIENT_MINOR_VERSION).

mir::protobuf::oldest_compatible_protocol_version() should return protocol_version(MIR_CLIENT_MAJOR_VERSION, 3);

Otherwise we'll accept connections from older libmirclient binaries but send input events they cannot parse and (if we don't bump MIR_CLIENT_MINOR_VERSION) successfully connect new clients to 0.18 servers which do not supply the correct events.

We can either make these changes here, or accept a promise elsewhere but we need to be sure of the approach before landing this branch.

review: Needs Information

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2015-12-15:

> OK codewise, but note this is a protocol break that isn't a client ABI bump.

Actually, the easiest approach would be to bump the epoch default value in mir::protobuf::protocol_version() - and that can be done here very simply.

review: Needs Fixing

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2015-12-16:

> Umm but that's the whole point of this MP, to use the scatter/gather support
> in the kernel so that you don't have to copy the mac into the structure when
> sending.

Maybe so, but I think my suggestion is significantly simpler. Smaller code and easier to follow and maintain.

Revision history for this message

Brandon Schaefer (brandontschaefer) wrote on 2016-01-04:

Im fine for using either. If we using a uint8_t[N] vs a uint8_t*. After all it looks we will want to replace with proper serialization anyway. So, the reason for supporting scatter/gather was with the idea that it would be useful for the future. If we want to rip this out, then maybe the simplest smallest way would be the best?

Though im not sure how long it would take to move to proper serialization :). It would be on my list of goals, and looking at it *shouldnt* be to hard.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-01-04:

Ok. Since we apparently intend to do this correctly in the not-too-distant future, and we're capable of breaking protocol at this level at will, we might as well shove a simple
uint8_t mac[20];
into the relevant structures and preserve the blittability at the cost of mac size changes being a protocol break.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-01-08:

PASSED: Continuous integration, rev:3163
https://mir-jenkins.ubuntu.com/job/mir-ci/19/
Executed test runs:
None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/19/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/19/rebuild

review: Approve (continuous-integration)

Unmerged revisions

3163. By Brandon Schaefer on 2015-12-11: * Rework the using of the empty_mac so we set the correct size
3162. By Brandon Schaefer on 2015-12-11: * Address reviewer
3161. By Brandon Schaefer on 2015-12-07: * Change comment (reads much better)
3160. By Brandon Schaefer on 2015-12-04: * Merge trunk
3159. By Brandon Schaefer on 2015-12-04: * Switch to using recvmsg so we can check if we get a truncated message! (We cant have those)
* Also added a comment to why we need to put values before the pointer[] if we want to copy those values.
the mac* is handled in its own buffer (the address is copied) soo its fine after the pointer[]
3158. By Brandon Schaefer on 2015-12-04: * Move to using a unit8_t* for the mac + a uint32_t mac_size to pass a variable length mac hash
through the fd
3157. By Brandon Schaefer on 2015-12-03: * Move to using a scatter/gather method for sending events.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file '3rd_party/android-input/android/frameworks/base/include/androidfw/InputTransport.h'
 --- 3rd_party/android-input/android/frameworks/base/include/androidfw/InputTransport.h	2015-08-17 18:33:38 +0000
 +++ 3rd_party/android-input/android/frameworks/base/include/androidfw/InputTransport.h	2015-12-11 21:17:18 +0000
@@ -45,6 +45,8 @@
   * Intermediate representation used to send input events and related signals.
   */
  struct InputMessage {
++    static uint32_t const MAC_PADDING = 1000;
++
      InputMessage();
      enum {
@@ -62,7 +64,6 @@
          struct Key {
              uint32_t seq;
              int64_t eventTime;
--            uint64_t mac;
              int32_t deviceId;
              int32_t source;
              int32_t action;
@@ -72,6 +73,8 @@
              int32_t metaState;
              int32_t repeatCount;
              int64_t downTime;
++            uint32_t mac_size;
++            uint8_t* mac;
              inline size_t size() const {
                  return sizeof(Key);
@@ -81,7 +84,6 @@
          struct Motion {
              uint32_t seq;
              int64_t eventTime;
--            uint64_t mac;
              int32_t deviceId;
              int32_t source;
              int32_t action;
@@ -95,10 +97,20 @@
              float xPrecision;
              float yPrecision;
              size_t pointerCount;
++            uint32_t mac_size;
++            /* As an optimisation sendMessage() only sends the valid members of the pointers[]
++             * array. This is implemented by size() only counting pointerCount members of
++             * the pointers[] array.
++             *
++             * As a side effect, everything after the pointers[] array will NOT be reliably sent
++             * in sendMessage(). Any members placed after pointers[] will need to be reconstructed
++             * in recieveMessage().
++             */
              struct Pointer {
                  PointerProperties properties;
                  PointerCoords coords;
              } pointers[MAX_POINTERS];
++            uint8_t* mac;
              int32_t getActionId() const {
                  uint32_t index = (action & AMOTION_EVENT_ACTION_POINTER_INDEX_MASK)
@@ -120,6 +132,15 @@
                  return sizeof(Finished);
+             }
          } finished;
++
++        /* Use a 1KB padding for the MAC
++         *
++         * Reserve enough space to receive a MAC in
++         * receiveMessage(...)
++         */
++        struct Padding {
++            uint8_t mac_padding[MAC_PADDING];
++        };
      } body;
      bool isValid(size_t actualSize) const;
 === modified file '3rd_party/android-input/android/frameworks/base/services/input/InputTransport.cpp'
 --- 3rd_party/android-input/android/frameworks/base/services/input/InputTransport.cpp	2015-10-20 03:30:00 +0000
 +++ 3rd_party/android-input/android/frameworks/base/services/input/InputTransport.cpp	2015-12-11 21:17:18 +0000
@@ -27,7 +27,9 @@
  #include <unistd.h>
  #include <sys/types.h>
  #include <sys/socket.h>
++#include <sys/uio.h>
  #include <math.h>
++#include <stdexcept>
  namespace android {
@@ -38,6 +40,8 @@
  // behind processing touches.
  static const size_t SOCKET_BUFFER_SIZE = 32 * 1024;
++static const uint64_t EMPTY_MAC = 0;
++
  // Nanoseconds per milliseconds.
  static constexpr const std::chrono::nanoseconds NANOS_PER_MS = std::chrono::nanoseconds(1000000);
@@ -61,6 +65,19 @@
      return a + alpha * (b - a);
+ }
++size_t getMacSizeForMessage(const InputMessage* msg)
++{
++    switch(msg->header.type)
++    {
++        case InputMessage::TYPE_KEY:
++            return msg->body.key.mac_size;
++        case InputMessage::TYPE_MOTION:
++            return msg->body.motion.mac_size;
++    }
++
++    return 0;
++}
++
  // --- InputMessage ---
  InputMessage::InputMessage()
@@ -69,7 +86,8 @@
+ }
  bool InputMessage::isValid(size_t actualSize) const {
--    if (size() == actualSize) {
++    size_t const mac_size = getMacSizeForMessage(this);
++    if (size() + mac_size == actualSize) {
          switch (header.type) {
          case TYPE_KEY:
              return true;
@@ -140,11 +158,74 @@
      return OK;
+ }
++bool messageHasMac(const InputMessage* msg)
++{
++    switch(msg->header.type) {
++        case InputMessage::TYPE_KEY:
++        case InputMessage::TYPE_MOTION:
++            return true;
++    }
++
++    return false;
++}
++
++uint8_t* macFromMessage(const InputMessage* msg)
++{
++    switch(msg->header.type) {
++        case InputMessage::TYPE_KEY:
++            return msg->body.key.mac;
++        case InputMessage::TYPE_MOTION:
++            return msg->body.motion.mac;
++    }
++
++    throw std::runtime_error("Invalid message type: " + msg->header.type);
++}
++
++size_t fillMac(const InputMessage* msg, iovec* vec)
++{
++    auto const mac  = static_cast<void*>(macFromMessage(msg));
++    auto const size = getMacSizeForMessage(msg);
++
++    if (mac && size == sizeof(uint64_t))
++    {
++        vec->iov_base = mac;
++        vec->iov_len  = size;
++    }
++    else
++    {
++        vec->iov_base = static_cast<void*>(const_cast<uint64_t*>(&EMPTY_MAC));
++        vec->iov_len  = sizeof(EMPTY_MAC);
++    }
++
++    return vec->iov_len;
++}
++
  status_t InputChannel::sendMessage(const InputMessage* msg) {
++    std::array<iovec, 2> msg_parts;
      size_t msgLength = msg->size();
++    size_t msg_parts_size = 1;
++
++    msg_parts[0].iov_base = static_cast<void*>(const_cast<InputMessage*>(msg));
++    msg_parts[0].iov_len  = msgLength;
++
++    if (messageHasMac(msg)) {
++        msgLength += fillMac(msg, &msg_parts[1]);
++        msg_parts_size++;
++    }
++
++    msghdr const message = {
++        nullptr,
++        0,
++        msg_parts.data(),
++        msg_parts_size,
++        nullptr,
++        0,
++        0
++    };
++
      ssize_t nWrite;
      do {
--        nWrite = ::send(mFd, msg, msgLength, MSG_DONTWAIT | MSG_NOSIGNAL);
++        nWrite = ::sendmsg(mFd, &message, MSG_DONTWAIT | MSG_NOSIGNAL);
      } while (nWrite == -1 && errno == EINTR);
      if (nWrite < 0) {
@@ -176,12 +257,64 @@
      return OK;
+ }
++uint8_t* getMacArrayFromMessageOffset(const InputMessage* msg, uint32_t mac_size)
++{
++    if (mac_size == sizeof(uint64_t)) {
++        return reinterpret_cast<uint8_t*>(const_cast<InputMessage*>(msg)) + msg->size();
++    }
++
++    return reinterpret_cast<uint8_t*>(const_cast<uint64_t*>(&EMPTY_MAC));
++}
++
++void setMacAddressFromMessage(InputMessage* msg)
++{
++    switch (msg->header.type)
++    {
++        case InputMessage::TYPE_KEY:
++        {
++            msg->body.key.mac = getMacArrayFromMessageOffset(msg, msg->body.key.mac_size);
++            break;
++        }
++        case InputMessage::TYPE_MOTION:
++        {
++            msg->body.motion.mac = getMacArrayFromMessageOffset(msg, msg->body.motion.mac_size);
++            break;
++        }
++    }
++}
++
  status_t InputChannel::receiveMessage(InputMessage* msg) {
++    iovec recv_msg;
++
++    recv_msg.iov_base = msg;
++    recv_msg.iov_len  = sizeof(InputMessage);
++
++    msghdr message = {
++        nullptr,
++        0,
++        &recv_msg,
++        1,
++        nullptr,
++        0,
++        0
++    };
++
      ssize_t nRead;
      do {
--        nRead = ::recv(mFd, msg, sizeof(InputMessage), MSG_DONTWAIT);
++        nRead = ::recvmsg(mFd, &message, MSG_DONTWAIT);
      } while (nRead == -1 && errno == EINTR);
++    if (messageHasMac(msg)) {
++        setMacAddressFromMessage(msg);
++    }
++
++    if (message.msg_flags & MSG_TRUNC) {
++#if DEBUG_CHANNEL_MESSAGES
++        ALOGD("channel '%s' ~ received message was truncated undefined behaviour", c_str(mName));
++#endif
++        return BAD_VALUE;
++    }
++
      if (nRead < 0) {
          int error = errno;
  #if DEBUG_CHANNEL_MESSAGES
@@ -264,9 +397,10 @@
      msg.body.key.scanCode = scanCode;
      msg.body.key.metaState = metaState;
      msg.body.key.repeatCount = repeatCount;
--    msg.body.key.mac = mac;
      msg.body.key.downTime = downTime.count();
      msg.body.key.eventTime = eventTime.count();
++    msg.body.key.mac = reinterpret_cast<uint8_t*>(&mac);
++    msg.body.key.mac_size = sizeof(uint64_t);
      return mChannel->sendMessage(&msg);
+ }
@@ -325,7 +459,6 @@
      msg.body.motion.yOffset = yOffset;
      msg.body.motion.xPrecision = xPrecision;
      msg.body.motion.yPrecision = yPrecision;
--    msg.body.motion.mac = mac;
      msg.body.motion.downTime = downTime.count();
      msg.body.motion.eventTime = eventTime.count();
      msg.body.motion.pointerCount = pointerCount;
@@ -333,6 +466,8 @@
          msg.body.motion.pointers[i].properties.copyFrom(pointerProperties[i]);
          msg.body.motion.pointers[i].coords.copyFrom(pointerCoords[i]);
+     }
++    msg.body.motion.mac = reinterpret_cast<uint8_t*>(&mac);
++    msg.body.motion.mac_size = sizeof(uint64_t);
      return mChannel->sendMessage(&msg);
+ }
@@ -882,7 +1017,7 @@
              msg->body.key.scanCode,
              msg->body.key.metaState,
              msg->body.key.repeatCount,
--            msg->body.key.mac,
++            *(reinterpret_cast<uint64_t*>(msg->body.key.mac)),
              std::chrono::nanoseconds(msg->body.key.downTime),
              std::chrono::nanoseconds(msg->body.key.eventTime));
+ }
@@ -908,7 +1043,7 @@
              msg->body.motion.yOffset,
              msg->body.motion.xPrecision,
              msg->body.motion.yPrecision,
--            msg->body.motion.mac,
++            *(reinterpret_cast<uint64_t*>(msg->body.motion.mac)),
              std::chrono::nanoseconds(msg->body.motion.downTime),
              std::chrono::nanoseconds(msg->body.motion.eventTime),
              pointerCount,