[$500] Chat - The video is reloaded every time you open it

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Found when validating PR : https://github.com/Expensify/App/pull/37036

Version Number: v1.4.43-18 Reproducible in staging?: Y Reproducible in production?: N Email or phone of affected tester (no customers): [email protected] Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause-Internal Team Slack conversation:

Action Performed:

1. Open the app
2. Login with any account
3. Go to any chat
4. Plus button -> Add attachment -> Send a video
5. Open the video that you just sent
6. Tap the back button
7. Open the video again

Expected Result:

Loading spinner is displayed only when you open the video for the first time

Actual Result:

The video is reloaded every time you open it

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [ ] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/ed75e85e-dca4-4385-ad16-d4d8c69816ed

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018db53b84762ad824
• Upwork Job ID: 1761342164490567680
• Last Price Increase: 2024-03-30

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~018db53b84762ad824

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @DylanDylann (External)

[melvin-bot[bot]]

Triggered auto assignment to @greg-schroeder (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[github-actions[bot]]

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot[bot]]

Triggered auto assignment to @Gonals (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[izarutskaya]

We think that this bug might be related to #vip-vsb CC @quinthar

[webbdays]

This shows no caching in the app.

[melvin-bot[bot]]

📣 @webbdays! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[brandonhenry]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The problem we are trying to solve is that the video attachment in the chat is reloaded every time it is opened, instead of being cached after the first load. This results in unnecessary network requests and a suboptimal user experience. This only happens on the mobile version of the app.

What is the root cause of that problem?

The root cause of the problem is that the video is not being cached on the device after the first load in the AttachmentViewVideo component. Every time the video is opened, it is fetched from the server again, causing the reload. This is a common issue for react native and caching videos.

https://github.com/Expensify/App/blob/699e3025924e5484bfd0433d6f037dfde230b2bb/src/components/Attachments/AttachmentView/AttachmentViewVideo/index.js#L24

What changes do you think we should make in order to solve the problem?

To solve the problem, we should implement a caching mechanism for the video files. When a video is opened for the first time, it should be downloaded and cached on the device. Subsequent openings of the video should use the cached version instead of fetching it from the server again.

Thankfully, since we are already using expo, we can just make use of expo's file system.

1. Import the necessary functions from expo-file-system:

import * as FileSystem from 'expo-file-system';

2. Create a utility function to check if the video is cached and fetch/cache it if necessary:

const MAX_CACHE_SIZE = 500 * 1024 * 1024; // 500MB
const CACHE_DIR = `${FileSystem.cacheDirectory}videos/`;

const fetchAndCacheVideo = async (videoUri) => {  
  const fileName = videoUri.split('/').pop();
  const videoPath = `${CACHE_DIR}${fileName}`;

  // Check if the video is already cached
  const isCached = await FileSystem.getInfoAsync(videoPath).then(
    (fileInfo) => fileInfo.exists
  );

  if (isCached) {
    // Load the cached video
    return videoPath;
  } else {
    // Ensure the cache directory exists
    await FileSystem.makeDirectoryAsync(CACHE_DIR, { intermediates: true });
    
    // Check available space and clean cache if needed
    await cleanCacheIfNeeded();

    // Fetch and cache the video
    const downloadResult = await FileSystem.downloadAsync(
      videoUri,
      videoPath
    );

    if (downloadResult.status === 200) {
      return videoPath;
    } else {
      throw new Error('Failed to fetch video');
    }
  }
};

const getCacheSize = async () => {
  const files = await FileSystem.readDirectoryAsync(CACHE_DIR);
  const sizes = await Promise.all(
    files.map(file => FileSystem.getInfoAsync(`${CACHE_DIR}${file}`))
  );
  
  return sizes.reduce((total, {size}) => total + size, 0); 
};

const cleanCacheIfNeeded = async () => {
  const cacheSize = await getCacheSize();
  
  if (cacheSize > MAX_CACHE_SIZE) {
    const files = await FileSystem.readDirectoryAsync(CACHE_DIR);
    const fileInfos = await Promise.all(
      files.map(file => FileSystem.getInfoAsync(`${CACHE_DIR}${file}`))  
    );
    
    // Sort by modificationTime ascending to remove oldest first
    fileInfos.sort((a,b) => a.modificationTime - b.modificationTime);
    
    let sizeToClear = cacheSize - MAX_CACHE_SIZE;
  
    for (const {uri, size} of fileInfos) {
      await FileSystem.deleteAsync(uri);
      sizeToClear -= size;
      
      if (sizeToClear <= 0) {
        break;
      }
    }
  }
};

The fetchAndCacheVideo function checks if the video is already cached in the app's cache directory (FileSystem.cacheDirectory). If the video is cached, it returns the cached file path. If not, it creates a videos/ directory in the cache directory (if it doesn't exist), fetches the video from the provided videoUri, and caches it in the videos/ directory.

UPDATE:

Two new utility functions are added:

• getCacheSize recursively gets the total size of all files in the video cache directory
• cleanCacheIfNeeded checks if the cache exceeds MAX_CACHE_SIZE (set to 500MB here) and if so, deletes the oldest videos until the size is below the limit. The check is performed before caching a new video.

This approach limits the video cache to a maximum size and automatically removes the oldest cached videos when that limit is reached. The specific size limit and cache location can be adjusted as needed.

[webbdays]

Note: Streaming the video is the best thing to do than download whole video at a time. Its best to achieve this by doing things like when video plays for the first time we get the stream segments. As we get more and more segments we join them to form a video. This helps us to get the video and users sees the video without waiting long time instead of waiting for full long video to download at first.

Second when we add caching, cache invalidation is very important. Adding functionality like ask the server "is the video/file modified?" before asking for video. (This needs some server side changes(very minor)).

[Skalakid]

@izarutskaya @DylanDylann @greg-schroeder I'm not sure if it is a bug, I think it is more like a new feature for the video player. The presented behavior is correct according to the video player design doc. Video caching wasn't planned during the predesign. I think it might be a great improvement! What do you think about it @francoisl?

@brandonhenry video player work differently depending on the screen size. On small screens, they are only usable inside the attachment modal. On large screens video players are available inside chat. When extending the video we are sharing the component to the attachment carousel to optimize video loadings (currently it is broken on main but I created a fix for it). Because of that, I think we should cache only videos that have shouldUseSharedVideoElement set to false. Also, videos may have bigger sizes, so I think it would be nice to limit the amount of videos that are currently cached so a user won't run out of memory. What's your thoughts on that?

[brandonhenry]

@Skalakid that makes sense to me and good insight. ++ to limiting the amount of videos currently cache

[webbdays]

Another approach might be to load first few segments (5-10 sec) of video when page loads, which allow the user to play fast and as the video plays the remaining parts arrives the browser.

[greg-schroeder]

Discussion ongoing

[DylanDylann]

I agree with @Skalakid at this point

I'm not sure if it is a bug, I think it is more like a new feature for the video player. The presented behavior is correct according to the video player design doc. Video caching wasn't planned during the predesign. I think it might be a great improvement

@francoisl @Gonals Should we make an improvement to have a caching mechanism here?

[francoisl]

Yes, that would make sense to me and sounds like a good improvement overall.

Are there any other libs than react-native-cached-image out there though? The GH repo for that project looks dead, the latest release is 7 years old.

[brandonhenry]

@francoisl looks like react-native-cached-image might be the only lib 🤔 - it was updated 2 months ago though. I think we'd probably want to either make use of this or create our own system

[webbdays]

I think expensify already have the codebase for that. Storage providers code in react-native-onyx repo. For storage. For caching we need to implement logic using those storage providers.

[DylanDylann]

@webbdays Could you detail your idea in a proposal?

[DylanDylann]

@brandonhenry Could you give a video after applying your change? And pushing your change to a branch then I can test your solution

[brandonhenry]

yes, will get you video today @DylanDylann

[brandonhenry]

was having some issue with my android build tonight, should have an update tomorrow

[brandonhenry]

@DylanDylann applied change and things are working (albeit needs a little tweaking)

PR: https://github.com/Expensify/App/pull/37574

https://github.com/Expensify/App/assets/15656774/3dc03277-dde6-488b-9f08-4a7d472357b7

[DylanDylann]

I am not in favor of adding react-native-cached-image to our repo

[brandonhenry]

@DylanDylann understandable. How about https://github.com/itinance/react-native-fs

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@Gonals, @greg-schroeder, @DylanDylann Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[webbdays]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Problem Statement: Video in chat should play without load the second time. Which mean we need caching.

What is the root cause of that problem?

The reason for the video still loading the second time is that there is no caching for the videos in chat. User need to wait until the browser gets video from the backend again.

What changes do you think we should make in order to solve the problem?

Implementing cache mechanism for attachments (like videos) in the chat at AttachmentViewVideo component or even early or later in base component (BaseVideoPlayer).

I have implemented it using existing code in react-native-onyx codebase. The code used is in "src/libs/Storage" folder. Cleaned it a bit to make it suitable for this case. Functionality remains the same. But use case is different.

The code provides an Storage Api to store key value pairs in both native and web platforms. It uses "idb-keyval" package for web. And "react-native-quick-sqlite" for native.

Moved this code to "src/libs/Storage" folder in Expensify/App repo and made some changes required. Added some cacheUtils code to help in cache implementation. Also changes to clear cache when sign out.

But there is some points here (not related to implementation but for using it inside a component) :

1. Cache not happening in wide screen. Video is playing without attachment view popup. Maybe we need to move this cache mechanism to appropriate component. I am not fully aware of the codebase. Working when minimise the window. Tested on Edge Browser in windows os.
2. Not tested for android, ios. But still the changes will solve for android and ios too.

I am ready to make a pr.

What alternative solutions did you explore?

We can use onyx with some modifications/by exposing storage API to allow create custom cache DB without default., but its intended use is different. This is another approach.

Note:

1. Still user need to wait until the video downloads for the first time. No fast play here. Still didn't found a proper way for this.
2. Cache Invalidation need backend changes for sure.

Thanks for reading.

[webbdays]

@DylanDylann @brandonhenry

[webbdays]

"idb-keyval" and "react-native-quick-sqlite" i think these are already being used here in both the repos (App, react-native-onyx).