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

✨ Add record / save path #893

Closed
2 tasks done
hirbod opened this issue Mar 12, 2022 · 54 comments · Fixed by #3103
Closed
2 tasks done

✨ Add record / save path #893

hirbod opened this issue Mar 12, 2022 · 54 comments · Fixed by #3103
Labels
✨ feature Proposes a new feature or enhancement Fund

Comments

@hirbod
Copy link
Contributor

hirbod commented Mar 12, 2022

What feature or enhancement are you suggesting?

I think it would be nice to define where the file should be saved. Currently it's being saved in the /tmp/ folder.
While this is not an issue but more a convenience and speed thing, (I am using FileSystem.copyAsync from expo to create a copy of it, since I am not allowed to "moveAsync" (permission issue?)

I would love to be able to define a save path, which Expo's FileSystem has access to.

Why I am asking for it? Because it's unreliable when the OS clears the tmp folder. I currently generated 16 GB of video files in my tests and there is no way for me to delete those. I think the end user will eventually delete our app if they can't free up space.

What Platforms whould this feature/enhancement affect?

iOS, Android

Alternatives/Workarounds

There are no workarounds. I tried to delete the files with FileSystem.deleteAsync but it always ends up with "file is not deleteable/moveable".

The copies of the files I create though, which live in FileSystems documentDirectory, are move- and deleteable.

I am using a video editor sdk which also has the ability to define the save path and its working pretty good and reliable.

Additional information

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@mrousavy
Copy link
Owner

Thanks for the feature request, that's a pretty valid enhancement. I'd imagine the user being responsible for creating all of the paths and making sure VisionCamera has permission to create a file there?

@hirbod
Copy link
Contributor Author

hirbod commented Mar 14, 2022

Here are the comments the other SDK I am using is making:

* The filename for the exported data .
     * The correct filename extension will be automatically added
     * based on the selected export format. It can be an absolute path or file URL or a relative path.
     * If some relative path is chosen it will be created in a temporary system directory and overwritten
     * if the corresponding file already exists. If the value is `null` an new temporary file will be
     * created for every export.
     * @note Please make sure that the provided `filename` is valid for the different devices and that
     * your application has the corresponding access rights to write to the desired location. For Android,
     * you will want to make sure to set this inside one of the directories conforming to scoped storage:
     * - DCIM/
     * - Pictures/
     * 
     * For Videos you can additionally use:
     *  - Movies/

And I am supplying something like this:

filename: VIDEO_TMP_DIR + uuidv4()

The SDK adds .mov/mp4 automatically based on filetype/codec.

And my VIDEO_TMP_DIR is

export const VIDEO_TMP_DIR = `${FileSystem.documentDirectory}myCoolApp/video-tmp-exports/`;

To your question:
If no filename/path supplied, it should behave like it does now: writing into the temp directory.
If the user does supply a path, RNVC should not be responsible for permissions.

@alexstanbury
Copy link
Contributor

alexstanbury commented Mar 21, 2022

+1 on this. I have faced issues in the past with losing pictures after a crash because React Native iOS clears the tmp directory upon each cold boot. Since then I have always moved every capture to a permanent directory immediately after capture, but this can add a slight delay which I'd rather avoid. Another reason is I'd like to generate my own file names instead of mrousavyXXXXXXXXX.jpg as they show up in certain file browsers, so currently I have to do another file operation to rename the file.

@garyhlusko
Copy link

@mrousavy I'm willing to help out with this, giving your approval and kind of a guide. I need this anyway :)

@mrousavy
Copy link
Owner

mrousavy commented Apr 4, 2022

Sure:

  1. Add path?: string in this interface:
    export interface RecordVideoOptions {
  2. [iOS]: Read options.path and if it exists, use it instead of tempURL here:
    let tempURL = URL(string: "file://\(tempFilePath)")!
  3. [Android]: Read options.path and if it exists, use it instead of tempURL here:
    val file = File.createTempFile("VisionCamera-${id}", ".mp4")

Test both with and without path to make sure everything still works, then create a PR and I can test :)

@mrousavy
Copy link
Owner

mrousavy commented Apr 4, 2022

We need some kind of error handling as well, so e.g.

  • Path does not exist
  • Path is not valid
  • Path is not writeable
  • Path is a folder, not a file
  • Path has invalid extension (not .mp4)

On Android, add here: https://github.com/mrousavy/react-native-vision-camera/blob/main/android/src/main/java/com/mrousavy/camera/Errors.kt
On iOS, add here: https://github.com/mrousavy/react-native-vision-camera/blob/main/ios/CameraError.swift
For JS, add here: https://github.com/mrousavy/react-native-vision-camera/blob/main/src/CameraError.ts

@alexstanbury
Copy link
Contributor

I'm happy to work on this next week sometime if @garyhlusko can't.

@GreenCurveDev
Copy link

Hi, did this enhancement ever get done? I am loving the vision camera but have hit a dead end re: using the results as its not possible to even read the photo/video files created using Expo FileSystem. FileSystem.getInfoAsync works but copyASync does not (file "isn't readable"). Any workaround's suggestions greatly appreciated.

@mortenmo
Copy link

mortenmo commented Oct 5, 2022

I'm having the same problem @GreenCurveDev has. I cannot even do copyASync on the file to keep it.

@mortenmo
Copy link

mortenmo commented Oct 5, 2022

@GreenCurveDev if you can use react-native-fs in your project, copying the file with that library does work.

@GreenCurveDev
Copy link

Thanks mortenmo, to work around this I prefixed the file paths with file:// but it definitely felt wrong - I will try your suggestion.

@Sid-Turner-Ellis
Copy link

Sid-Turner-Ellis commented Jan 20, 2023

Is this still in progress? Did either of you guys get to work on it? @alexstanbury @garyhlusko

I would use react-native-fs however i'm recording large videos which may be too large for the initial write to internal storage.

Another useful alternative would be to get some kind of access to chunks of data. Any thoughts on this kind of functionality @mrousavy ?

@mrousavy
Copy link
Owner

Yea, this is WIP - need V3 for this. If you wanna see this faster, consider sponsoring the V3 efforts :) [email protected]

@josh-thompson13
Copy link

Was this ever completed?

@emilje

This comment was marked as spam.

@gitn00b1337
Copy link

Same here, with expo I get:

"Caused by: File '///private/var/mobile/Containers/Data/Application/C998E08F-86F2-4413-BC87-FBB59CAC675E/tmp/9B75D8FA-2216-4E1D-B1A6-822806D3E4DC.jpeg' is not readable"

I'm unable to read the file in any way, other than getting info. Further, I cant use RNFS because it doesnt support expo. I tried prepending the file path with file:// like @GreenCurveDev mentioned but its not working for me at all. Does anyone have a working example for expo?

const fromPath = // tried `file:///{photo.path}` and file://{photo.path} as well as photo.path. 
await FileSystem.copyAsync({
      from: fromPath,
      to: destinationPath
    })

@gitn00b1337
Copy link

For those stuck on expo, the only way I've managed is to use fetch as per the docs. Unfortunately this means no immediate cleanup of the temp file but it'll do

const result = await fetch(photo.path);
    const blob = await result.blob();

    const reader = new FileReader();
    reader.readAsDataURL(blob);
    reader.onloadend = async () => {
      const base64data = (reader.result as string).split(',')[1];

      await FileSystem.writeAsStringAsync(destinationPath, base64data, {
        encoding: FileSystem.EncodingType.Base64,
      });
      
      console.log('File saved successfully to:', destinationPath);
      return destinationPath;
    };

@gwashburne
Copy link

@mrousavy Is this still WIP? The tmp and expo-file-system don't get along very well. Would this be something a donation page could help make happen?

@mrousavy
Copy link
Owner

Nope not WIP - I didn't implement this anymore because V3 (or now V4) was already huge and this was low priority for me since it always just worked for me with tmp files.

But yea I could create a donations page and start building this. I think the implementation plan for me would be to just add a path parameter to takePhoto, takeSnapshot and startRecording - file name itself is still generated randomly, and it will throw if the path is invalid.

@mrousavy mrousavy pinned this issue Jul 23, 2024
@polar-sh polar-sh bot added the Fund label Jul 23, 2024
Copy link

Guten Tag, Hans here.

Note

New features, bugfixes, updates and other improvements are all handled mostly by @mrousavy in his free time.
To support @mrousavy, please consider 💖 sponsoring him on GitHub 💖.
Sponsored issues will be prioritized.

@mrousavy
Copy link
Owner

mrousavy commented Jul 23, 2024

Added a polar bade to this issue - $400 is a fair goal for this, I don't think it takes too long. Could be done in less than a week

@gwashburne
Copy link

Added a polar bade to this issue - $400 is a fair goal for this, I don't think it takes too long. Could be done in less than a week

@mrousavy Sounds good! Perhaps I'm completely missing something, but could you post the link to the donations page? Thanks!

@gwashburne
Copy link

Android done now as well! A few refactors and two fixes later; we're ready to test - @gwashburne can you test if this works for you?

This might be user error on my end, but I cloned the repo so I could test locally, and npm install fails:

npm error code ERESOLVE
npm error ERESOLVE unable to resolve dependency tree
npm error
npm error While resolving: [email protected]
npm error Found: [email protected]
npm error node_modules/react
npm error dev react@"^18.3.1" from the root project
npm error
npm error Could not resolve dependency:
npm error peer react@"18.2.0" from [email protected]
npm error node_modules/react-native
npm error dev react-native@"^0.74.2" from the root project
npm error
npm error Fix the upstream dependency conflict, or retry
npm error this command with --force or --legacy-peer-deps
npm error to accept an incorrect (and potentially broken) dependency resolution.

@mrousavy
Copy link
Owner

You need to use yarn

@gwashburne
Copy link

You need to use yarn

Hmm. Tried to do that but still having issues; can't resolve the path to react-native-vision-camera package on pod install. If it's working for you and others, it's probably an issue on my end switching to yarn from npm. I'm not very familiar with yarn

@mrousavy
Copy link
Owner

To run VisionCamera example do this:

git clone https://....blablabla
cd react-native-vision-camera
cd package
yarn
cd example
yarn
yarn pods

Then open package/example/ios/VisionCameraExample.xcworkspace in Xcode, select your phone, and run

@gwashburne
Copy link

Thanks! I followed those steps, but unfortunately, at compile time, got these errors:
compile error

@mrousavy
Copy link
Owner

Are you sure you installed pods then? These two files have been added and will only be available after you installed pods. (cd package/example/ios && pod install or the yarn pods command)

@gwashburne
Copy link

Just ran pod install and tried again; different error this time. To be honest Marc, this may not be worth it; I'm fairly confident this is user error on my end. If it's working for you and passing the usual tests, we might want to skip me testing it haha.
Screenshot 2024-07-26 at 8 42 30 AM

@mrousavy
Copy link
Owner

Ah this error is because a .xcode.env.local file got generated which probably has a wrong path - I just always delete that file.

But yea sure I'll just try to release a beta for you to test in your app

@gwashburne
Copy link

Aha! Ok, after removing that file, it compiled and installed. Example app seems to be running fine!

@mrousavy
Copy link
Owner

Done and released in 4.5.1!! 💪 :)

Thanks for the sponsoring @gwashburne ❤️

@SarjuHansaliya
Copy link
Contributor

@mrousavy I just tried 4.5.1. what are the possible reason that this error comes

{ [capture/file-io-error: An unexpected File IO error occurred! Error: The file “28C67DDD-3528-49DF-9C1B-6F559EDD4836.jpg” doesn’t exist.]
  name: 'capture/file-io-error',
  _code: 'capture/file-io-error',
  _message: 'An unexpected File IO error occurred! Error: The file “28C67DDD-3528-49DF-9C1B-6F559EDD4836.jpg” doesn’t exist.',
  _cause: undefined }

My code is simple

const data = await camera.current.takePhoto({
  path: `${ReactNativeBlobUtil.fs.dirs.CacheDir}/image.jpeg`,
});

@mrousavy
Copy link
Owner

The path should be a directory, not a file url.
Remove the image.jpg suffix

@SarjuHansaliya
Copy link
Contributor

Thanks @mrousavy , yes that is working.

@gwashburne
Copy link

Done and released in 4.5.1!! 💪 :)

Thanks for the sponsoring @gwashburne ❤️

You're welcome! Thanks for getting this done so fast and helping me out with testing!

@mrousavy
Copy link
Owner

awesome! :)

@emilje
Copy link

emilje commented Jul 31, 2024

Can confirm this works nicely with expo too!

@mrousavy
Copy link
Owner

mrousavy commented Aug 1, 2024

great to hear!

@alexstanbury
Copy link
Contributor

Thanks so much for getting this done!

Would it be too much to ask that we are also able to provide the file name ourselves, as a separate option? It could be used in combination with the path parameter or standalone. I need my captured images to follow a certain naming convention, so I can't benefit from the performance/convenience benefit of this change because I still need to touch the file system to do the rename.

@mrousavy
Copy link
Owner

mrousavy commented Aug 1, 2024

Out of curiousity - why would you need your filenames to have a different name? How is that affecting the user of your app?

@alexstanbury
Copy link
Contributor

I use a timestamp for the filename to be able to quickly run certain analytical tasks on the images captured.

@gwashburne
Copy link

gwashburne commented Aug 7, 2024

@mrousavy Got a bit of a problem when running the release version: [capture/file-io-error: An unexpected File IO error occurred! Error: The file “AF0AED19-A636-495D-A3D6-3F1223D64BDF.jpg” doesn’t exist.
Which is a bit confusing, since I'm only giving it the directory path, not the file name

@gwashburne
Copy link

It's something about production vs debug compiling. Works as expected if I switch back to debug

@emilje
Copy link

emilje commented Aug 7, 2024

@gwashburne I had a similar error and then realized that the folder I was transferring to did not exist and it would not automatically create one just by specifying the path, so I now check and create the folder if it does not exist before taking a photo. Did not yet try in production build...

@gwashburne
Copy link

@gwashburne I had a similar error and then realized that the folder I was transferring to did not exist and it would not automatically create one just by specifying the path, so I now check and create the folder if it does not exist before taking a photo. Did not yet try in production build...

Thanks @emilje! That was exactly the issue. Got it fixed!

@gwashburne
Copy link

gwashburne commented Nov 13, 2024

Thanks so much for getting this done!

Would it be too much to ask that we are also able to provide the file name ourselves, as a separate option? It could be used in combination with the path parameter or standalone. I need my captured images to follow a certain naming convention, so I can't benefit from the performance/convenience benefit of this change because I still need to touch the file system to do the rename.

@alexstanbury @alexrififi @samuel-andres I have created a new fork/branch to add this feature. If you'd like to help out with testing it, I'd much appreciate it! I think it's ready, but was hoping for feedback before I make the PR. It's available here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature Proposes a new feature or enhancement Fund
Projects
None yet
Development

Successfully merging a pull request may close this issue.