> LGTM. 

Great!

> Let's also take the opportunity to clean up the current shared
> memory thing. Will it be needed? I'm struggling to make it work properly
> (including the test).

Binary messaging depends on shared memory IPC to deliver the best performance 
when using external extension.

Thanks for your efforts to fix it.

-ningxin

> 
> On Mon, Jul 20, 2015 at 12:15 AM, Huo, Halton <[email protected]> wrote:
> > LGTM. Thanks for satisfy high performance needs like RealSense.
> >
> >> -----Original Message-----
> >> From: Crosswalk-dev
> >> [mailto:[email protected]] On Behalf
> >> Of Hu, Ningxin
> >> Sent: Friday, July 17, 2015 7:06 PM
> >> To: Liang, Zhenyu; '[email protected]'
> >> Subject: Re: [Crosswalk-dev] Intent to Implement: Binary Messaging
> >> Interface for Crosswalk External Extension
> >>
> >> > Yes, certainly.
> >>
> >> I assume it means LGTM from you. :) Thanks!
> >>
> >> >
> >> > Since I'm neither architect nor owner, you need to get LGTMs from
> >> > them
> >> > :-)
> >>
> >> Ping @Halton for extension, @Lin for Android and @Alexis for overall.
> >> Please take a look.
> >>
> >> Thanks,
> >> -ningxin
> >>
> >> >
> >> > On 7/17/15 17:44 , "Hu, Ningxin" <[email protected]> wrote:
> >> >
> >> > >Hi Zhenyu,
> >> > >
> >> > >>
> >> > >> My suggestion is to introduce a new version of current messaging
> >> > >>interface  instead of a whole new interface.
> >> > >>
> >> > >> struct XW_MessagingInterface_2 {
> >> > >>
> >> > >>   void (*Register)(XW_Extension extension,
> >> > >> XW_HandleMessageCallback handle_message);
> >> > >>   void (*PostMessage)(XW_Instance instance, const char*
> >> > >> message);
> >> > >>
> >> > >>   void (*RegisterBinaryMessageCallback)(XW_Extension extension,
> >> > >> XW_HandleBinaryMessageCallback handle_message);
> >> > >>   void (*PostBinaryMessage)(XW_Instance instance, const char*
> >> > >> message, const size_t size); };
> >> > >>
> >> > >
> >> > >Thanks for your suggestion.
> >> > >
> >> > >XW_MessagingInterface_2 looks better than
> >> > >XW_BinaryMessagingInterface_1, as the binary messaging API is an
> >> > >enhancement to the async messaging interface. And it looks
> >> > >consistent to
> >> > Java API proposal.
> >> > >
> >> > >I will integrate this change into my design.
> >> > >
> >> > >With this change, does it look good you?
> >> > >
> >> > >Thanks,
> >> > >-ningxin
> >> > >
> >> > >> Thanks,
> >> > >>
> >> > >> Zhenyu
> >> > >>
> >> > >> On 7/16/15 15:25 , "Hu, Ningxin" <[email protected]> wrote:
> >> > >>
> >> > >> >Intent to Implement: Binary Messaging Interface for Crosswalk
> >> > >> >External Extension
> >> > >> >
> >> > >> >Description:
> >> > >> >Today's Crosswalk external extension API only provides text
> >> > >> >based messaging interface. However, coming use cases require
> >> > >> >the external extension native code to exchange binary data with
> >> > >> >JavaScript
> >> code.
> >> > >> >For example, an external extension wants to transfer the images
> >> > >> >stream from native to JavaScript in real-time (30FPS at least).
> >> > >> >With today's external extension messaging API, the extension
> >> > >> >has to encode the binary image data into text in native first,
> >> > >> >posts to JavaScript and then decodes from the text to binary
> >> > >> >data in JavaScript. The encoding, decoding (in
> >> > >> >JavaScript!) and increased message size leads to low performance.
> >> > >> >Experiment shows by using base64 to transfer VGA (640x480)
> >> > >> >images, this approach can only deliver less 10 FPS on mainstream
> PC.
> >> > >> >The binary messaging interface is proposed to added into
> >> > >> >Crosswalk external extension API. It allows native code to
> >> > >> >exchange binary data (a byte array) to JavaScript without
> >> > >> >unnecessary
> >> > encoding/decoding.
> >> > >> >JavaScript receives an ArrayBuffer object and can access the
> >> > >> >binary data via different Typed Array View. Vice versa,
> >> > >> >JavaScript can send the binary data in an ArrayBuffer object to
> >> > >> >native, native code receives a byte array.
> >> > >> >Prototype shows 3X-8X (binary messaging vs. text messaging)
> >> > >> >performance speedup when transferring different size of data on
> >> > >> >both PC and mobile device. Initial data shows with binary
> >> > >> >messaging interface, an external extension is able to transfer
> >> > >> >VGA images at
> >> > 60FPS on PC.
> >> > >> >
> >> > >> >Affected component: Crosswalk External Extension
> >> > >> >
> >> > >> >Related feature:
> >> > >> >https://crosswalk-project.org/jira/browse/XWALK-4615
> >> > >> >
> >> > >> >Target release: Crosswalk-17
> >> > >> >
> >> > >> >Implementation details:
> >> > >> >1. Interface change:
> >> > >> >For Crosswalk External Extension C API, a new interface
> >> > >> >XW_BinaryMessagingInterface_1 will be added. To keep API  and
> >> > >> >ABI compatibility with existing external extension, a new file
> >> > >> >(extensions/public/XW_Extension_BinaryMessage.h) will be added
> >> > >> >to contain the XW_BinaryMessagingInterface_1 interface. The
> >> > >> >XW_BinaryMessagingInterface_1 looks like:
> >> > >> >
> >> > >> >///////////////////////////////////////////////////////////////
> >> > >> >///
> >> > >> >//
> >> > >> >///
> >> > >> >///
> >> > >> >/////////////////
> >> > >> >#ifdef __cplusplus
> >> > >> >extern "C" {
> >> > >> >#endif
> >> > >> >
> >> > >> >#define XW_BINARY_MESSAGING_INTERFACE_1
> >> > "XW_BinaryMessagingInterface_1"
> >> > >> >#define XW_BINARY_MESSAGING_INTERFACE
> >> > >> >XW_BINARY_MESSAGING_INTERFACE_1
> >> > >> >
> >> > >> >typedef void (*XW_HandleBinaryMessageCallback)(XW_Instance
> >> > >> >instance,
> >> > >> >
> >> > >> >                               const char* message,
> >> > >> >
> >> > >> >                               const size_t size);
> >> > >> >
> >> > >> >struct XW_BinaryMessagingInterface_1 {
> >> > >> >   void (*Register)(XW_Extension extension,
> >> > >> >
> >> XW_HandleBinaryMessageCallback
> >> > >> >handle_message);
> >> > >> >
> >> > >> >   void (*PostMessage)(XW_Instance instance, const char*
> >> > >> >message, const size_t size); };
> >> > >> >
> >> > >> >typedef struct XW_BinaryMessagingInterface_1
> >> > >> >XW_BinaryMessagingInterface;
> >> > >> >
> >> > >> >#ifdef __cplusplus
> >> > >> >}  // extern "C"
> >> > >> >#endif
> >> > >> >///////////////////////////////////////////////////////////////
> >> > >> >///
> >> > >> >//
> >> > >> >///
> >> > >> >///
> >> > >> >////////////////
> >> > >> >
> >> > >> >If the C extension wants to use binary messaging, it needs to
> >> > >> >query the "XW_BinaryMessagingInterface_1" interface and use the
> >> > >> >PostMessage method to post binary data to JavaScript. On other
> >> > >> >hand, it needs to register a XW_HandleBinaryMessageCallback to
> >> > >> >receive binary data from
> >> > >> JavaScript.
> >> > >> >
> >> > >> >For Crosswalk external extension Android Java API, two methods
> >> > >> >will be added into XWalkExtensionClient interface:
> >> > >> >
> >> > >> >///////////////////////////////////////////////////////////////
> >> > >> >///
> >> > >> >//
> >> > >> >///
> >> > >> >///
> >> > >> >///////////////
> >> > >> >
> >> > >> >public void onBinaryMessage(int extensionInstanceID, byte[]
> >> > >> >message);
> >> > >> >
> >> > >> >public final void postBinaryMessage(int instanceID, byte[]
> >> > >> >message);
> >> > >> >
> >> > >> >///////////////////////////////////////////////////////////////
> >> > >> >///
> >> > >> >//
> >> > >> >///
> >> > >> >///
> >> > >> >//////////////
> >> > >> >
> >> > >> >Java extension which inherits XWalkExtensionClient can use
> >> > >> >postBinaryMesssage to post binary data to JavaScript and
> >> > >> >implements onBinaryMessage to receive binary data from JavaScript.
> >> > >> >
> >> > >> >No new APIs will be added into Crosswalk extension JavaScript API.
> >> > >> >When native external extension posts binary message, the
> >> > >> >JavaScript message listener receives an ArrayBuffer object.
> >> > >> >When JavaScript invokes extension.postMessage with an
> >> > >> >ArrayBuffer object as argument, the native binary message
> listener is invoked.
> >> > >> >
> >> > >> >This proposal doesn't cover binary messaging support in
> >> > >> >SyncMessage, as there is no obvious use case.
> >> > >> >
> >> > >> >Sample code:
> >> > >> >An external extension wants to transfer image data to JavaScript.
> >> > >> >To transfer structured data, the extension can use simple
> >> > >> >custom binary message format as this sample shows. Or it can
> >> > >> >leverage more sophisticated binary format, e.g. Google's
> >> > >> >Protocol Buffer (https://developers.google.com/protocol-buffers/).
> >> > >> >C++:
> >> > >> >///////////////////////////////////////////////////////////////
> >> > >> >///
> >> > >> >//
> >> > >> >///
> >> > >> >///
> >> > >> >/////////////
> >> > >> >// image binary format
> >> > >> >// uint32_t width;
> >> > >> >// uint32_t height;
> >> > >> >// char data[width * height * 4];
> >> > >> >
> >> > >> >static const XW_BinaryMessagingInterface* g_binary_messaging =
> >> > >> >    get_interface(XW_BINARY_MESSAGING_INTERFACE);
> >> > >> >.....
> >> > >> >size_t buffer_size = sizeof(uint32_t) + sizeof(uint32_t) +
> >> > >> >width*height*4*sizeof(char);
> >> > >> >char* buffer = new char[buffer_size ];
> >> > >> >uint32_t* uint32_array = reinterpret_cast<uint32_t*>(buffer);
> >> > >> >uint32_array[0] = image.width(); uint32_array[1] =
> >> > >> >image.height();
> >> > >> >char* data = buffer + 2 * sizeof(uint32_t);
> >> > >> >image.CopyImageData(data);
> >> > >> >g_binary_messaging->postMessage(instance,
> >> > >> >buffer, buffer_size); delete[] buffer;
> >> > >> >///////////////////////////////////////////////////////////////
> >> > >> >///
> >> > >> >//
> >> > >> >///
> >> > >> >///
> >> > >> >/////////////
> >> > >> >
> >> > >> >JavaScript:
> >> > >> >///////////////////////////////////////////////////////////////
> >> > >> >///
> >> > >> >//
> >> > >> >///
> >> > >> >///
> >> > >> >/////////////
> >> > >> >extension.setMessageListener(function(msg) {
> >> > >> >  if (msg instaceof ArrayBuffer) {
> >> > >> >     var uint32_view = new Uint32Array(msg);
> >> > >> >     var width = uint32_view[0];
> >> > >> >     var height = uint32_view[1];
> >> > >> >     var data = new Uint8Array(msg, 8, width*height*4);
> >> > >> >     ....
> >> > >> >  }
> >> > >> >}
> >> > >> >///////////////////////////////////////////////////////////////
> >> > >> >///
> >> > >> >//
> >> > >> >///
> >> > >> >///
> >> > >> >////////////
> >> > >> >
> >> > >> >Details:
> >> > >> >It doesn't require to modify Crosswalk Core Extension code
> >> > >> >(XWalkExtensionModule), as V8ValueConverter already supports to
> >> > >> >convert a base::BinaryValue object to V8 ArrayBuffer object and
> >> > >> >vice
> >> > versa.
> >> > >> >
> >> > >> >It mainly requires to modify the external extension components,
> >> > >> >to expose the binary interface and convert the binary data to
> >> > >> >base::BinaryValue and vice versa.
> >> > >> >
> >> > >> >For C external extension, files to be modified:
> >> > >> >extensions/common/xwalk_external_adapter.cc
> >> > >> >extensions/common/xwalk_external_adapter.h
> >> > >> >extensions/common/xwalk_external_extension.cc
> >> > >> >extensions/common/xwalk_external_extension.h
> >> > >> >extensions/common/xwalk_external_instance.cc
> >> > >> >extensions/common/xwalk_external_instance.h
> >> > >> >extensions/extensions.gyp
> >> > >> >
> >> > >> >new file:
> >> > >> >extensions/public/XW_Extension_BinaryMessage.h
> >> > >> >
> >> > >> >For Java external extension, files to be modified:
> >> > >> >app/android/runtime_client/src/org/xwalk/app/runtime/extension/
> >> > >> >XWa
> >> > >> >lk
> >> > >> >Cor
> >> > >> >eEx
> >> > >> >tensionBridge.java
> >> > >> >app/android/runtime_client/src/org/xwalk/app/runtime/extension/
> >> > >> >XWa
> >> > >> >lk
> >> > >> >Ext
> >> > >> >ens
> >> > >> >ionClient.java
> >> > >> >app/android/runtime_client/src/org/xwalk/app/runtime/extension/
> >> > >> >XWa
> >> > >> >lk
> >> > >> >Ext
> >> > >> >ens
> >> > >> >ionContextClient.java
> >> > >> >app/android/runtime_client/src/org/xwalk/app/runtime/extension/
> >> > >> >XWa
> >> > >> >lk
> >> > >> >Run
> >> > >> >tim
> >> > >> >eExtensionBridge.java
> >> > >> >app/android/runtime_client/src/org/xwalk/app/runtime/extension/
> >> > >> >XWa
> >> > >> >lk
> >> > >> >Run
> >> > >> >tim
> >> > >> >eExtensionManager.java
> >> > >> >extensions/android/java/src/org/xwalk/core/internal/extensions/
> >> > >> >XWa
> >> > >> >lk
> >> > >> >Ext
> >> > >> >ens
> >> > >> >ionAndroid.java
> >> > >> >extensions/common/android/xwalk_extension_android.cc
> >> > >> >extensions/common/android/xwalk_extension_android.h
> >> > >> >runtime/android/core_internal/src/org/xwalk/core/internal/XWalk
> >> > >> >Ext
> >> > >> >en
> >> > >> >sio
> >> > >> >nIn
> >> > >> >ternal.java
> >> > >> >
> >> > >> >
> >> > >> >Thanks,
> >> > >> >-ningxin
> >> > >> >
> >> > >> >_______________________________________________
> >> > >> >Crosswalk-dev mailing list
> >> > >> >[email protected]
> >> > >> >https://lists.crosswalk-project.org/mailman/listinfo/crosswalk-
> >> > >> >dev
> >> > >
> >>
> >> _______________________________________________
> >> Crosswalk-dev mailing list
> >> [email protected]
> >> https://lists.crosswalk-project.org/mailman/listinfo/crosswalk-dev
> > _______________________________________________
> > Crosswalk-dev mailing list
> > [email protected]
> > https://lists.crosswalk-project.org/mailman/listinfo/crosswalk-dev
_______________________________________________
Crosswalk-dev mailing list
[email protected]
https://lists.crosswalk-project.org/mailman/listinfo/crosswalk-dev

Reply via email to