numbersprotocol / capture-lite

A photo-sharing app with only verifiable photos and videos.
https://numbersprotocol.github.io/
GNU General Public License v3.0
25 stars 6 forks source link

[Sprint 2] Upload video(SHORT) #2660

Closed sync-by-unito[bot] closed 1 year ago

sync-by-unito[bot] commented 1 year ago

The user flow should be the same as ✓ [Sprint 2] Upload image

┆Issue is synchronized with this Asana task by Unito ┆Created By: Kenny Hung

sync-by-unito[bot] commented 1 year ago

➤ Tammy Yang commented:

Too many issues with a delay in the previous sprint, reschedule in sprint 23.04.24

sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Olga what is the maximum size asset we have in backend? The size of video in MegaBytes. Can you help to clarify this?

Because when user picks a video from gallery we need to limit maximum upload size.

sync-by-unito[bot] commented 1 year ago

➤ Olga commented:

Sam (cc Tammy YangJames Chien) The parameter description ( https://dia-backend-dev.numbersprotocol.io/api/v3/redoc/#operation/assets_create ) indicates that the file limit might be 1 GB. However, I couldn't find any specific implementation limitation regarding this. Although there is a limit of 1 GB on Nginx's client_max_body_size set to 1024M, it applies to the entire body size, not specifically to the file size. Therefore, it might be necessary for us to define a maximum asset file size.

sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Tammy YangBofu ChenJames Chien, should we re-consider how capture app works with files or we keep working with base64?

Currently capture app works ( https://github.com/numbersprotocol/capture-lite/blob/02ae4e74227cfc46f537ad1ead006ddf27d5eb18/src/app/shared/capture/capture.service.ts#L26 ) with base64. Meaning if we have movie.mp4 it reads it as base64 and then do the rest of the work.

Issue is when file is read as base64 it's loaded to memory and when the file is too big for user's device RAM it's crashing with error like:

Caused by: java.lang.OutOfMemoryError: Failed to allocate a 134217744 byte allocation with 25165824 free bytes and 37MB until OOM, target footprint 254595680, growth limit 268435456

I currently implemented feature to pick image/video from gallery and it's working for

but when video size is too big for device's memory it's crashing the app.

sync-by-unito[bot] commented 1 year ago

➤ James Chien commented:

As far as I know, using base64 is because of previous limitation of the Capacitor storage plugin, not an ideal design choice. As long as the data migration is smooth I believe we could move to other better implementations if there's any.

sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Kenny Hung, I think there one more reason why user camera crashes. I think when user take 15 seconds video and camera is high quality size of the captured video is also high.

And maybe app crashes not because of camera but because of file size is too big for device RAM when it's read as base64.

sync-by-unito[bot] commented 1 year ago

➤ Kenny Hung commented:

Sam Thanks for your feedback. QA will continue tracking issue from community.

It may be a potential issue for capture app.

Currently, We haven't received about crash issue when user is recording video.

sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Kenny Hung, I mean recording 15 second video with HD cameras like Redmi 10s. I didn't tested 15 seconds videos with Redmi 10s but it would be nice to test in next milestone release 🙏

sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Done but with limitation.

If user picks a video that is large it will crash the app.

But users can upload smaller video tested on

To fix limitation we need to do further testing and research.

sync-by-unito[bot] commented 1 year ago

➤ Kenny Hung commented:

Could we release first & have a pop-up or Informational text stop uploading when user upload too big let user know Capture App can't support this size.

sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Kenny Hung it crashes suddenly so we will not have a chance to inform user. I was thinking to show modal before user click next but file size limitation is different for each device

sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Kenny Hung I want to try one more experiment today and will report my reserach about this issue.

sync-by-unito[bot] commented 1 year ago

➤ Kenny Hung commented:

Could we implement a similar approach as the Dashboard, where if the video size exceeds 25MB, a dialog box will appear with the message "The video size is more than xx mb, please choose a lower one."?

sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Kenny Hung yes. I can check file size before upload.

But size limit is different for each device based on device memory.

For example iphone 6 was able to upload 30mb video. Galaxy M12 only 6mb video.

sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Kenny Hung let me try few experiments today and by end of day I will let you know what are final possible options.

I want to upgrade plugins and check native code if its possible to fix there.

sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Kenny Hung as I mentioned before

This limitation is because we load video/image to base64 format which 33% larger and when we convert file to base64 it loads to memory therefore might crash the app if user device has not enough memory (RAM)

For now we can dynamically check user available memory and show them dynamic popup.

For example for

So dynamically checking user available memory can be a temporary solution. In the future we should find a way to upload larger files like 1 minute video etc.

sync-by-unito[bot] commented 1 year ago

➤ Tammy Yang commented:

Agree with Sam that we should find a better way to upload the videos. For now, let's keep 5mb as the limitation for all App users since users can always use dashboard to upload bigger files. How do you think?

sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Tammy Yang, I was working on this task today.

First thing I found in docs Working with large files may require you to add ( https://capacitorjs.com/docs/apis/filesystem#android ) And doing so fixed Android OutOfMemory issue that I was mentioning before. Now my Galaxy M12 can do 17mb upload (before it could pick 6mb and was crashing when I picking same 17mb file)

Second is check device limitation dynamically.

  1. User capture photo/video or user pick photo/video from gallery
  2. When user click publish I call hasEnoughMemory()
  3. If hasEnoughMemory == true then continue normal process
  4. if hasEnoughMemory != true then show modal

This way we do not have to limit user to 5mb because some devices can do 20-30mb and some like my Galaxy M12 could do less. And if users device can not upload 30 mb we will let user know accordingly (see insufficient memory demo.mp4)

Tomorrow I will do more testing, code clean up and we should be good to go.

Just in case here is the GPT generated code for hasEnoughMemory just for details.

/**

*/

function hasEnoughMemory(fileSizeInBytes: number, availableMemory: number, buffer: number = fileSizeInBytes * 0.2): boolean {

const requiredMemory = fileSizeInBytes + buffer;

return requiredMemory <= availableMemory;

}

sync-by-unito[bot] commented 1 year ago

➤ Kenny Hung commented:

Sam Some device can’t record 15s SHORT by Capture camera. (Not using video uploading.) Because we block more than 30mb video to upload and too big video will let App crash.

https://app.claap.io/numbers-protocol/video-upload-limitation-issue-c-O35CsUM4Uy-uxaqbTpMYeky ( https://app.claap.io/numbers-protocol/video-upload-limitation-issue-c-O35CsUM4Uy-uxaqbTpMYeky )

Most issue will happened on android device

  1. Oppo

    1. previous version

      1. No issue
    2. 0.81.3

      1. Just accept less than 4 seconds video, over 5 seconds will display over 30 mb pop-up
  2. Redmi note 10s

    1. previous version (0.79.0)

      1. record 15s short will crash
    2. 0.81.3

      1. Just accept less than 5 seconds video, over 6 seconds will display over 30 mb pop-up
  3. Samsung A21

    1. previous version (0.79.0)

      1. record 15s short sometimes will crash.
    2. 0.81.3

      1. No issue
  4. Redmi Note 11 Pro

    1. previous version (0.79.0)

      1. No issue
    2. 0.81.3

      1. Just accept less than 12 seconds video, over 13 seconds will display over 30 mb pop-up
sync-by-unito[bot] commented 1 year ago

➤ Sam commented:

Kenny Hung, thank you for testing and summary. As mentioned before Comment by @Sam on [Sprint 2] Upload video(SHORT) ( https://app.asana.com/0/0/1204202515424193/1204802442705457/f ) I suggest to open new asana card to discuss this limitation and propose new solution.