Page MenuHomePhabricator

[native] Implement BlobUtils expo-module on Android
ClosedPublic

Authored by bartek on May 25 2023, 5:13 AM.
Tags
None
Referenced Files
F2034595: D7974.id.diff
Tue, Jun 18, 7:17 AM
Unknown Object (File)
Sun, Jun 16, 4:32 PM
Unknown Object (File)
Thu, Jun 13, 1:33 AM
Unknown Object (File)
Wed, Jun 12, 3:57 AM
Unknown Object (File)
Tue, Jun 11, 8:59 PM
Unknown Object (File)
Mon, Jun 3, 3:20 PM
Unknown Object (File)
Wed, May 29, 10:25 AM
Unknown Object (File)
Wed, May 29, 12:47 AM
Subscribers

Details

Summary

The core part of ENG-3718. The issue description is important to understand the context of this diff.
In short: Implemented two functions that provide a way to directly convert Blob <--> ArrayBuffer without going through base64 encoding/decoding which is extremely inefficient.

Other helpful resources:

  • The library that does the same using Java and raw JSI: C++, Java

Depends on D7973

Test Plan

Built the Android, tested using the following snippet:

const array1 = new Uint8Array([1, 2, 3, 4, 5]);
const blob1 = blobFromArrayBuffer(array1.buffer);
console.log('blob1', blob1);

const decoded = new Uint8Array(arrayBufferFromBlob(blob1));
console.log('decoded', decoded);

const blob2 = blobFromArrayBuffer(decoded.buffer);
console.log('blob2', blob2);
// line below requires amending exports in native/media/blob-utils.js
console.log('Another way:', await blobToDataURI(blob2).then(dataURIToIntArray));

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.May 25 2023, 5:53 AM
atul added 1 blocking reviewer(s): marcin.

Looks good to me, but adding @marcin as blocking to take another look since he's more familiar w/ Kotlin/Android APIs/etc.

native/expo-modules/comm-expo-package/android/src/main/java/app/comm/android/utils/BlobUtilsModule.kt
16

Same nit as iOS version of this diff, we would usually name this blobID instead of blobId.

Lack of syntax highlighting for Kotlin was annoying... solved it with this. Let me know if any other languages have similar problems

This revision is now accepted and ready to land.May 29 2023, 10:33 AM
native/expo-modules/comm-expo-package/android/src/main/java/app/comm/android/utils/BlobUtilsModule.kt
25

Are those methods async when used in JS? If not can we make them so? I recall from Java and Objective-C that it wasn't always the caseThis module appears to be a general purpose tool so it should support ability to process many independent blobs at once.
This question applies to iOS diff as well.

native/expo-modules/comm-expo-package/android/src/main/java/app/comm/android/utils/BlobUtilsModule.kt
25

Also applies to the child diff where the usage of those methods suggests that they are not async.

native/expo-modules/comm-expo-package/android/src/main/java/app/comm/android/utils/BlobUtilsModule.kt
25

They're synchronous. Unfortunately, Typed arrays aren't supported yet in async expo-module functions. The problem is that arraybuffer/typedArray operations must be performed on the JS thread.
This is a known issue, we even have a task to track this.

In this particular case, I'd argue that these should be async because it's only pointer logic here + one copy operation (for contiguous memory) so they should be fast.

Anyway, at this moment we can't do much about it (except using C++ and JSI which is more effort than it's worth)