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

FFmpeg Improvements #906

Merged
merged 3 commits into from
Apr 27, 2024
Merged

FFmpeg Improvements #906

merged 3 commits into from
Apr 27, 2024

Conversation

VendorAttestation
Copy link
Contributor

@VendorAttestation VendorAttestation commented Apr 14, 2024

Bump ffmpeg-kit -> 6.0-2.LTS
Default -hwaccel mediacodec
Change libx264 -> h264_mediacodec

https://developer.android.com/reference/android/media/MediaCodec

Tested here is a sample video https://gofile.io/d/LV1s5H (Download Raw File for Improvements)

Tested on Iphone / Android / Windows video played on all

MediaCodec is androids codec I've seen many great improvements for quality

Bump guava -> 33.1.0-jre
Bump compose-compiler->1.5.11
Bump kotlin -> 1.9.23
Bump compose-bom -> 2024.04.00
Bump ffmpeg-kit -> 6.0.LTS
Bump material3 -> 1.2.1
Default -hwaccel mediacodec
Change libx264 -> h264_mediacodec
@VendorAttestation
Copy link
Contributor Author

Before we push i need ffmpeg-kit testers for the new encoding as this is a major LTS upgrade

Copy link
Contributor

@CanerKaraca23 CanerKaraca23 left a comment

Choose a reason for hiding this comment

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

There is more recent version of FFmpeg kit but developer says it break "stuff". Don't know exactly what it is.

bcprov-jdk18on = "1.77"
dexlib2 = "3.0.5"
ffmpeg-kit = "5.1.LTS" # DO NOT UPDATE FFMPEG-KIT TO "5.1" it breaks stuff :3
ffmpeg-kit = "6.0.LTS" # DO NOT UPDATE FFMPEG-KIT TO "v6.0.3" it breaks stuff :3
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ffmpeg-kit = "6.0.LTS" # DO NOT UPDATE FFMPEG-KIT TO "v6.0.3" it breaks stuff :3
ffmpeg-kit = "6.0-2.LTS"

- Revert Everything non FFMPEG related
* Bump ffmpeg-kit -> 6.0-2.LTS
@VendorAttestation VendorAttestation changed the title FFmpeg Improvements + Dependencies Changes FFmpeg Improvements Apr 14, 2024
Remove old comment
@VendorAttestation
Copy link
Contributor Author

@CanerKaraca23 It doesn't break anything tbh removed that old comment its just better practice to use LTS (Long Time Support)

@authorisation
Copy link
Collaborator

IIRC @CanerKaraca23 already made a pull request before and updating ffmpeg-kit broke something.

@VendorAttestation
Copy link
Contributor Author

VendorAttestation commented Apr 14, 2024

IIRC @CanerKaraca23 already made a pull request before and updating ffmpeg-kit broke something.

I've had a few people test this new LTS build with FFMPEG stuff seems to work? Where did i break so i can test

Also arthenica/ffmpeg-kit#789

we need 6 for HW Encoding sadly so maybe we can try to resolve bugs?

@authorisation
Copy link
Collaborator

I think merging this for v2.1.0 while everything else gets tested first would fit nicely. I want to make sure v2.0.x doesn't have any major flaws first.

@authorisation authorisation added the accepted This will be actively worked on. label Apr 14, 2024
@authorisation
Copy link
Collaborator

Also maybe a stripped down ffmpeg-kit custom build would be better, the current kit makes the APKs really huge.

https://github.com/SnapEnhance/ffmpeg

@VendorAttestation
Copy link
Contributor Author

Also maybe a stripped down ffmpeg-kit custom build would be better, the current kit makes the APKs really huge.

https://github.com/SnapEnhance/ffmpeg

any reason you killed --enable-android-media-codec we kinda need that :)

@CanerKaraca23
Copy link
Contributor

CanerKaraca23 commented Apr 14, 2024

IIRC @CanerKaraca23 already made a pull request before and updating ffmpeg-kit broke something.

I didn't see anything broken, i use with latest version on my own fork.

Developer reverted changes without any comments on my old PR, so i don't know the reason.

@authorisation
Copy link
Collaborator

Also maybe a stripped down ffmpeg-kit custom build would be better, the current kit makes the APKs really huge.
https://github.com/SnapEnhance/ffmpeg

any reason you killed --enable-android-media-codec we kinda need that :)

My bad, I'm trying to get it to compile in the first place, I'll add it in a second

@VendorAttestation
Copy link
Contributor Author

Also maybe a stripped down ffmpeg-kit custom build would be better, the current kit makes the APKs really huge.
https://github.com/SnapEnhance/ffmpeg

any reason you killed --enable-android-media-codec we kinda need that :)

My bad, I'm trying to get it to compile in the first place, I'll add it in a second

Also --enable-opus probably not needed as i don't think windows PC / other platforms can even play it by default so very rare use cases because they'll have to open in VLC also we should enable --enable-x265 (https://en.wikipedia.org/wiki/X265) for people who want to compress video's more with no quality loss just my opinion tho

@VendorAttestation
Copy link
Contributor Author

Also @authorisation

https://github.com/arthenica/ffmpeg-kit/wiki/Building-FFmpegKit-with-Custom-Libraries

https://github.com/mstorsjo/fdk-aac

We should build with libfdk-aac (arthenica/ffmpeg-kit#581) says we need to edit the config

libfdk-aac compiles audio faster (on android) then normal aac and is generally more stable just putting this here for future reference if we decide to add.

@authorisation authorisation added the next_minor Will be merged or worked on in the next MINOR iteration. label Apr 21, 2024
@authorisation authorisation merged commit 638dba0 into rhunk:dev Apr 27, 2024
5 checks passed
@authorisation authorisation removed the next_minor Will be merged or worked on in the next MINOR iteration. label Apr 27, 2024
sn-o-w pushed a commit to sn-o-w/SnapEnhance that referenced this pull request Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This will be actively worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants