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 a simple audio / video trimmer. #178

Merged
merged 4 commits into from
Aug 12, 2023
Merged

Conversation

SuhasDissa
Copy link
Member

Hey! I was looking through the issue history of the app and saw that you closed issue #54 saying that it's not supported by Android system APIs. But I found a system API that does support simple video trimming, and the AOSP gallery app uses it without any third-party libraries.

I added that feature to the app, and it seems to work fine. There are a few minor front-end UI/UX issues that I'll fix soon. But in the meantime, I'd love for you to take a look and let me know what you think.

Thanks!

@Bnyro
Copy link
Member

Bnyro commented Aug 11, 2023

This looks great at first view!
I haven't yet tested this, but such a functionality by using system APIs only is really awesome!

Copy link
Member

@Bnyro Bnyro left a comment

Choose a reason for hiding this comment

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

I think we should place the play button below the seekbar, between start: ... and stop: ... to make it look less squashed. (Though it's fine if we do that somewhen later)

I also noticed the recordings tab doesn't get updated when the trimming is done (since we run it on Dispatchers.IO now), but that can be changed in a different PR.

So it's good to be merged by you from my side :)
Works great, thank you for working on that!

@SuhasDissa
Copy link
Member Author

TrimmerModel has some problem. It doesn't reset the timestamps when opening a new file

@SuhasDissa
Copy link
Member Author

image
This is the only solution I could come up with

@Bnyro
Copy link
Member

Bnyro commented Aug 12, 2023

image This is the only solution I could come up with

That should be fine in my opinion.
That way there's no edge case where startTimeStamp and endTimeStamp are not being resetted, so it's the probably the most safe option :)

@SuhasDissa
Copy link
Member Author

@Bnyro What do you prefer? Merge or Squash and Merge

@Bnyro
Copy link
Member

Bnyro commented Aug 12, 2023

I combined the latest three commits since to clean up the commit history, now you can Merge the PR if you want :)

@SuhasDissa SuhasDissa merged commit a496c2f into you-apps:main Aug 12, 2023
1 check passed
@SuhasDissa SuhasDissa deleted the trimmer branch January 11, 2024 14:13
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.

None yet

2 participants