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

Fix MP4 videos saving result in 3GP on Android API <=28 #1188

Merged
merged 9 commits into from
Oct 30, 2024

Conversation

yishangfei
Copy link
Contributor

Fixed the issue that the video format changes to 3gp when saving mp4 videos to the album on Android 9

@AlexV525
Copy link
Member

Could you provide a test case in which saving 3gp will fail? Your changes don't tell the idea and why it will fix the issue well.

@yishangfei
Copy link
Contributor Author

yishangfei commented Sep 19, 2024

pubspec.yaml

photo_manager: ^3.3.0
photo_manager_image_provider: ^2.1.1

image
image

use my code

e0bd912a05e01fcca51f557d9eb0d26

device:

Redmi Note5 android 9
MI 8 Lite android 9
MEIZU Note9 android 9

@AlexV525
Copy link
Member

It is still unclear why the PR fixes the issue. Could you provide a minimal reproducible example?

@yishangfei
Copy link
Contributor Author

example code

var appCacheDir = await getApplicationCacheDirectory();
String filePath = '${appCacheDir.path}/aaa.mp4';
File result = File(filePath);
var entity = await PhotoManager.editor.saveVideo(
      result,
     title: path.basename(result.path),
     );  
print('assetEntity===${entity!.title}===${await entity.file}');

On Android 9, the source file is mp4 and converted to 3gp format through saveVideo

@AlexV525
Copy link
Member

Is this happening on all videos or just one specific video? Could you file an issue to summarize all the details? And provide a minimal runnable example so we can run to reproduce.

@yishangfei
Copy link
Contributor Author

yishangfei commented Sep 21, 2024

All videos on android9

demo: https://github.com/yishangfei/test1

image

test1.mp4

@AlexV525 AlexV525 changed the title Update IDBUtils.kt Fix MP4 videos saving result in 3GP on Android API <=28 Oct 3, 2024
@@ -428,11 +429,6 @@ interface IDBUtils {
}
refreshStream()

val shouldKeepPath = if (!isAboveAndroidQ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also keep the condition? The fix should work without removing it, correct?

folderName: String?,
): String {
var albumFolderPath: String = Environment.getExternalStorageDirectory().path
if (android.os.Build.VERSION.SDK_INT < 29) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What standard do we following here? Should we use MOVIE as much as possible?

return albumFolderPath
}

private fun createDirIfNotExist(dirPath: String): String? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem to already have a helper method checkDir

@yishangfei
Copy link
Contributor Author

I added it in line 455 according to your latest code.

// check if the directories exist
"$albumDir${File.separator}$title".checkDirs()
val timestamp = System.currentTimeMillis().toString()
// Create a video file. If you use file.name, assetEntity may be empty.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we cannot use the filename?

@AlexV525
Copy link
Member

Still working on this?

@yishangfei
Copy link
Contributor Author

now there is no problem. I only modified the version below Android 10. It’s just that the timestamp needs to be used when saving. If the original name is used a second time, the entity returned by PhotoManager.editor.saveVideo will be null.

@AlexV525
Copy link
Member

If the original name is used a second time, the entity returned by PhotoManager.editor.saveVideo will be null.

We need to figure out why.

Also please do another rebase on the latest changes and remove unnecessary changes.

@yishangfei
Copy link
Contributor Author

The automatic renaming function (for example, video (1).mp4) is implemented in Android 11 and above. In earlier versions, if you repeat cr.insert in the insertUri method, an exception will be thrown. "Cannot insert the new asset."

 val uri = cr.insert(contentUri, values) ?: throw RuntimeException("Cannot insert the new asset.")

Just like the pictures I provided before,versions below Android Q use timestamp naming to insert, or write a renaming logic that is the same as that of higher versions.

@AlexV525
Copy link
Member

AlexV525 commented Oct 26, 2024

The automatic renaming function (for example, video (1).mp4) is implemented in Android 11 and above.

TIL. Are there any docs referencing this?

EDIT: Could you also write it down as comment in the code?

@yishangfei
Copy link
Contributor Author

yishangfei commented Oct 26, 2024

I have not been able to find any official documentation.
I found the solution on stackoverflow.
ContentResolver.insert always returning null

add the following comment to the code. Is it OK?

When using the ContentResolver.insert() method to insert a file with the same name, an exception will indeed be thrown if the file with the same name already exists. This is because in Android 10 and below, MediaStore does not have the automatic renaming function.

@AlexV525
Copy link
Member

add the following comment to the code. Is it OK?

How about:

Using a duplicate filename that already exists on the device will cause the insertion to fail on Android API 30-.

Using a duplicate file name that already exists on the device will cause inserts to fail on less than Android API 30.
@yishangfei
Copy link
Contributor Author

add the following comment to the code. Is it OK?

How about:

Using a duplicate filename that already exists on the device will cause the insertion to fail on Android API 30-.

i submitted

@AlexV525
Copy link
Member

I've also added some format changes and fixed the nullable saving result.

Copy link
Member

@AlexV525 AlexV525 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution.

@AlexV525 AlexV525 merged commit ea4c635 into fluttercandies:main Oct 30, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants