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

Draft: Feat add clear individual session #1067

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bawahakim
Copy link

@bawahakim bawahakim commented Nov 18, 2024

Description

Fixes #1066

Type of Change

  • New feature

Checks

  • Changes support all platforms (Android, iOS, Linux, macOS, tvOS)
  • Does not break existing functionality
  • Implementation is completed, not half-done
  • Is there another PR already created for this feature/bug fix

Tests

  1. Create any ffmpeg session
  2. Confirm the session exists with FFmpegKitConfig.getSessions(); or equivalent
  3. At any point, call FFmpegKitConfig.clearSession(sessionId); or equivalent
  4. Call FFmpegKitConfig.getSessions(); again
  5. Confirm the session is not there anymore

@bawahakim
Copy link
Author

@tanersener Coincidentally, I just started working on this right now, and see you working on it as well :D

I've got the part working for iOS/flutter. However, I named the method "clearSession" rather than "deleteSession" to keep it consistent with "clearSessions". What are your thoughts?

@bawahakim bawahakim marked this pull request as draft November 18, 2024 10:55
@tanersener
Copy link
Collaborator

Thanks for the PR!

After reviewing the method name, I felt that clearSession() might suggest clearing resources without fully deleting the session. To avoid confusion, I opted for deleteSession() instead.

This method is already implemented for the native platforms (Android, Apple, Linux) on the development branch. If you'd like to contribute to the Flutter implementation, please submit a PR against development-flutter as we outlined in the CONTRIBUTING guide.

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.

Allow to clear individual sessions
2 participants