Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Video assets which are iOS only should not end up in Android bundle #19987

Closed
siddarthkay opened this issue May 11, 2024 · 6 comments · Fixed by #20026
Closed

Video assets which are iOS only should not end up in Android bundle #19987

siddarthkay opened this issue May 11, 2024 · 6 comments · Fixed by #20026
Assignees

Comments

@siddarthkay
Copy link
Contributor

Problem

The video asset files used for onboarding animation are ~ 30 MB and they end up in the Android APK.
The component which uses those assets is not rendered in Android but maybe because its a runtime check those assets make their way into the bundle anyway.

We should find a way to make sure those video assets are included in iOS only so that we can shave off 30 MB from the Android APK.

Related links :

@siddarthkay
Copy link
Contributor Author

Checking my initial assumption that these mp4 files make their way into the bundle due to a runtime check.
I added stricter platform checks to only reference the video component which requires the mp4s only on iOS platform like this :

diff --git a/src/status_im/common/parallax/blacklist.cljs b/src/status_im/common/parallax/blacklist.cljs
index 3e9a1da5a..0b74d2586 100644
--- a/src/status_im/common/parallax/blacklist.cljs
+++ b/src/status_im/common/parallax/blacklist.cljs
@@ -1,6 +1,7 @@
 (ns status-im.common.parallax.blacklist
   (:require
-    [native-module.core :as native-module]))
+    [native-module.core :as native-module]
+    [react-native.platform :as platform]))

 (def ^:private device-id (:device-id (native-module/get-device-model-info)))

@@ -12,7 +13,4 @@

 (def ^:private minimum-device-code 11)

-(def blacklisted?
-  (-> device-id
-      get-model-code
-      (< minimum-device-code)))
+(def blacklisted? platform/android?)
diff --git a/src/status_im/contexts/onboarding/enable_biometrics/view.cljs b/src/status_im/contexts/onboarding/enable_biometrics/view.cljs
index a8f51f638..d8b0f746e 100644
--- a/src/status_im/contexts/onboarding/enable_biometrics/view.cljs
+++ b/src/status_im/contexts/onboarding/enable_biometrics/view.cljs
@@ -10,6 +10,7 @@
     [status-im.contexts.onboarding.enable-biometrics.style :as style]
     [status-im.navigation.state :as state]
     [utils.i18n :as i18n]
+    [react-native.platform :as platform]
     [utils.re-frame :as rf]))


@@ -55,9 +56,10 @@
     [rn/view
      {:position :absolute
       :top      12}
-     [parallax/video
-      {:layers  (:biometrics resources/parallax-video)
-       :stretch stretch}]]))
+     (when platform/ios?
+       [parallax/video
+        {:layers  (:biometrics resources/parallax-video)
+         :stretch stretch}])]))

 (defn enable-biometrics-simple
   []
diff --git a/src/status_im/contexts/onboarding/enable_notifications/view.cljs b/src/status_im/contexts/onboarding/enable_notifications/view.cljs
index f423944f9..5e6db2bae 100644
--- a/src/status_im/contexts/onboarding/enable_notifications/view.cljs
+++ b/src/status_im/contexts/onboarding/enable_notifications/view.cljs
@@ -10,6 +10,7 @@
     [status-im.contexts.onboarding.enable-notifications.style :as style]
     [status-im.contexts.shell.jump-to.utils :as shell.utils]
     [taoensso.timbre :as log]
+    [react-native.platform :as platform]
     [utils.i18n :as i18n]
     [utils.re-frame :as rf]))

@@ -58,9 +59,10 @@
 (defn enable-notifications-parallax
   []
   (let [stretch (if rn/small-screen? -40 -25)]
-    [parallax/video
-     {:layers  (:notifications resources/parallax-video)
-      :stretch stretch}]))
+    (when platform/ios?
+      [parallax/video
+         {:layers  (:notifications resources/parallax-video)
+          :stretch stretch}])))

 (defn enable-notifications-simple
   []
diff --git a/src/status_im/contexts/onboarding/generating_keys/view.cljs b/src/status_im/contexts/onboarding/generating_keys/view.cljs
index f15a96d68..5b2a6a888 100644
--- a/src/status_im/contexts/onboarding/generating_keys/view.cljs
+++ b/src/status_im/contexts/onboarding/generating_keys/view.cljs
@@ -126,13 +126,14 @@
 (defn parallax-page
   [insets]
   [:<>
-   [parallax/video
-    {:stretch           -20
-     :container-style   {:top  40
-                         :left 20}
-     :layers            (:generate-keys resources/parallax-video)
-     :disable-parallax? true
-     :enable-looping?   false}]
+   (when platform/ios?
+     [parallax/video
+      {:stretch           -20
+       :container-style   {:top  40
+                           :left 20}
+       :layers            (:generate-keys resources/parallax-video)
+       :disable-parallax? true
+       :enable-looping?   false}])
    [:f> f-page-title insets]])

 (defn f-simple-page

However after analysing a release APK from this build I could still see the mp4s as part of the res folder.
Which means that as long as there is a js/require in the code that pulls in these resources they will make it to both the android and iOS bundles.
ref ->

{:biometrics [(js/require "../resources/videos2/biometrics_01.mp4")
(js/require "../resources/videos2/biometrics_02.mp4")
(js/require "../resources/videos2/biometrics_03.mp4")
(js/require "../resources/videos2/biometrics_04.mp4")]
:generate-keys [(js/require "../resources/videos2/generating_keys_01.mp4")
(js/require "../resources/videos2/generating_keys_02.mp4")
(js/require "../resources/videos2/generating_keys_03.mp4")]
:notifications [(js/require "../resources/videos2/notifications_01.mp4")
(js/require "../resources/videos2/notifications_02.mp4")
(js/require "../resources/videos2/notifications_03.mp4")
(js/require "../resources/videos2/notifications_04.mp4")]})

@siddarthkay
Copy link
Contributor Author

One idea which i fiddled with a little bit was to store the video assets in native directories of iOS and Android respectively.
The iOS one would have the original video but the Android one would have a fake mp4 file of 4bytes.
so when we'd require them on Android we'd get the savings we want since we don't use those UI components which require videos anyways.

This isn't very straightforward since react-native-transparent-video library only accepts a source attribute and doesn't accept uris -> https://github.com/status-im/react-native-transparent-video/blob/main/src/index.tsx#L6-L9
cc @briansztamfater

I did write a patch to include uri as part of props but I did not get the video to show up on iOS
so more research is indeed needed in this space to achieve the goal of these heavy video assets not showing up in Android bundle

@OmarBasem
Copy link
Member

@siddarthkay I think 30MB just for onboarding animations (regardless of platform) is a bit too much

Maybe we can also try to optimize the size of these files

@siddarthkay
Copy link
Contributor Author

siddarthkay commented May 14, 2024

a bit too much

indeed 30 MB of animations are a lot.

in other news : I managed to get rid of this 30 MB on Android side.

credits to @jakubgs for this recommendation
I wrote a gradle task which parses the release bundle and swaps the heavy mp4 files with fake ones.

note : The fake ones would contain the same name as the original one to make sure that the js bundle does not complain of missing assets.

So now the android APK would contain mp4 files which would weight 4 bytes each.

This should land us savings of ~ 30 MB on Android APK.

I'm still testing things out, will send a PR for this today.

@siddarthkay
Copy link
Contributor Author

This approach should have no impact on iOS which then requires no change on Clojurescript side as well which is nicer considering the goal here is to reduce APK size.

@siddarthkay siddarthkay linked a pull request May 14, 2024 that will close this issue
siddarthkay added a commit that referenced this issue May 14, 2024
I recently discovered that we have `mp4` video assets as part of `Android` bundle which are never used on `Android`.
The animations are hidden only shown on certain `iOS` devices and this is done via a runtime check which includes only certain `iOS` devices.
Because of this logic and the way `react-native` bundles assets into binaries we get these assets inside the Android `APK` as well.

This commit swaps the `mp4` files with fake ones of `4 bytes` each after a release build is generated in a nix phase.
This step would only happen for `nightly`, `PR` and `release` builds, since we care about final `APK` size.

This helps us to shave off roughly `30 MB` on the `APK` size

not needed since we just optimise the size of final `APK`.

- Android

fixes: #19987

status: ready
@siddarthkay
Copy link
Contributor Author

wrote a gradle task which parses the release bundle and swaps the heavy mp4 files with fake ones.

I later realised that this logic should live in one of the phases and it was better if it was executed right after the buildPhase which prepares the APK in our existing nix derivation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants