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

getSafParameterForWrite doesn't work with -movflags faststart on Android #167

Closed
gazlaws-dev opened this issue Oct 3, 2021 · 3 comments
Closed
Assignees
Labels
android Affect Android platform enhancement New feature or request fixed-in-v4.5.1 library Affects the library saf Issue about storage access framework v4.4 Affects v4.4 release v4.5 Affects v4.5 release

Comments

@gazlaws-dev
Copy link

gazlaws-dev commented Oct 3, 2021

Description
Creating an output video (.mp4) with the-movflags faststart doesn't work with getSafParameterForWrite (using Android's Storage Access Framework), presumably because it's not able to read the file for the second pass.

Expected behavior
An option to create the output file with read and write permissions. Maybe getSafParameter can be made public so we can pass 'rw' as openMode.

Current behavior
Output file is not created properly, error shown:

2021-10-03 15:27:24.849 24442-24657/x I/ffmpeg-kit: Output #0, mp4, to 'saf:105.mp4':
2021-10-03 15:27:28.770 24442-24657/x E/ffmpeg-kit: Error writing trailer of saf:105.mp4: Bad file descriptor

To Reproduce

inputVideoUrl = FFmpegKitConfig.getSafParameterForRead(requireContext(), inputVideoUri);
outputFilePath = FFmpegKitConfig.getSafParameterForWrite(requireContext(), resultVideoUri);

ffmpeg command (shortened):
  -y -i "saf:90.mp4" -vcodec libx264  -movflags faststart  saf:105.mp4

Logs

2021-10-03 15:27:18.726 24442-24442/x I/EditFragment: getContent: Input video absolute file path: saf:90.mp4
2021-10-03 15:27:24.547 24442-24657/x I/ffmpeg-kit: Input #1, mov,mp4,m4a,3gp,3g2,mj2, from 'saf:90.mp4':
2021-10-03 15:27:24.849 24442-24657/x I/ffmpeg-kit: Output #0, mp4, to 'saf:105.mp4':
2021-10-03 15:27:28.770 24442-24657/x E/ffmpeg-kit: Error writing trailer of saf:105.mp4: Bad file descriptor

Environment

  • Platform: Android
  • Architecture: arm64-v8a
  • Version: v4.5
@tanersener tanersener added the bug Something isn't working label Oct 4, 2021
@tanersener tanersener self-assigned this Oct 4, 2021
@tanersener tanersener added v4.5 Affects v4.5 release android Affect Android platform library Affects the library saf Issue about storage access framework labels Oct 4, 2021
@tanersener
Copy link
Collaborator

Thanks for reporting this issue. Yeah, it seems like there is a problem there. Existing flags used internally ("r" and "w") are not enough to move the moov atom to the beginning of the file. We need to provide another method for it.

@tanersener tanersener added this to To Do in Project Roadmap via automation Oct 4, 2021
@tanersener tanersener removed this from To Do in Project Roadmap Oct 6, 2021
@tanersener tanersener added this to Needs triage in Next GA Release via automation Oct 6, 2021
@tanersener tanersener added enhancement New feature or request and removed bug Something isn't working labels Oct 7, 2021
@tanersener tanersener added the v4.4 Affects v4.4 release label Oct 21, 2021
@tanersener tanersener moved this from Needs triage to High priority in Next GA Release Nov 5, 2021
@tanersener tanersener moved this from High priority to Closed in Next GA Release Nov 7, 2021
@tanersener
Copy link
Collaborator

My tests show that we have this problem on v4.4 and v4.5 because saf urls created by FFmpegKitConfig.getSafParameter methods are single-use. They cannot be used more than once. And, in your case, when -movflags faststart is added, ffmpeg tries to open the file second time to move the header to the beginning. But it fails and prints the standard error we have for re-using the same saf url: Bad file descriptor.

I fixed this on the development branch by re-implementing the whole saf protocol logic. SAF urls are reusable now. So, opening the file again to move the header is no longer a problem. I also changed the access modifier of FFmpegKitConfig.getSafParameter method. You can use it to pass your own open mode.

@tanersener tanersener added the fixed-on-development Fixed on the development branch. Not released yet. label Nov 7, 2021
@tanersener
Copy link
Collaborator

The changes are released on v4.5.1. However, apps that use this feature must test it well. I saw two issues during my tests.

  • Storage Access Framework handles open modes differently on old API levels. w mode that is enough to write an mp4 file on API Level 29+ devices doesn't work as expected on API Level 24 and 21. You need to use rw on them.
  • Some Document providers do not support rw mode, e.g. Google Drive provider

@tanersener tanersener added fixed-in-v4.5.1 and removed fixed-on-development Fixed on the development branch. Not released yet. labels Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android Affect Android platform enhancement New feature or request fixed-in-v4.5.1 library Affects the library saf Issue about storage access framework v4.4 Affects v4.4 release v4.5 Affects v4.5 release
Projects
No open projects
Next GA Release
  
Closed
Development

No branches or pull requests

2 participants