[native] Implement BlobUtils expo-module on Android
ClosedPublic
Actions

Authored by bartek on May 25 2023, 5:13 AM.

Details

Reviewers

atul
marcin

Commits

rCOMMca7dc26f16b4: [native] Implement BlobUtils expo-module on Android

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 created this revision.May 25 2023, 5:13 AM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptMay 25 2023, 5:13 AM

Harbormaster completed remote builds in B19765: Diff 27050.May 25 2023, 5:31 AM

bartek published this revision for review.May 25 2023, 5:53 AM

bartek added a child revision: D7976: [native] Use BlobUtils for media encryption/decryption.May 25 2023, 7:28 AM

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 ↗	(On Diff #27050)	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

marcin accepted this revision.May 29 2023, 10:33 AM

This revision is now accepted and ready to land.May 29 2023, 10:33 AM

marcin added inline comments.May 29 2023, 10:37 AM

native/expo-modules/comm-expo-package/android/src/main/java/app/comm/android/utils/BlobUtilsModule.kt
25 ↗	(On Diff #27050)	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.

marcin added inline comments.May 29 2023, 10:41 AM

native/expo-modules/comm-expo-package/android/src/main/java/app/comm/android/utils/BlobUtilsModule.kt
25 ↗	(On Diff #27050)	Also applies to the child diff where the usage of those methods suggests that they are not async.

bartek added inline comments.May 29 2023, 10:59 AM

native/expo-modules/comm-expo-package/android/src/main/java/app/comm/android/utils/BlobUtilsModule.kt
25 ↗	(On Diff #27050)	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)