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

Feat:add ffmpeg sidecar for ffmpeg installation #231

Conversation

Mubashirshariq
Copy link

/claim #194
/closes #194

@louis030195
Copy link
Collaborator

@Mubashirshariq hey thanks for this!

this is a good start, but this is just a function unused in a corner

here's what's missing:

  • instructions to test and see screenpipe works without having manually ffmpeg installed beforehand
  • running this somewhere, say in the CLI screenpipe-server.rs
  • i can run the install instructions from README without the ffmpeg part and it works
  • i can remove the ffmpeg installation in brew and it works
  • i can remove the ffmpeg installation from app pre build and it works

i'd add tips for things like (but optional):

  • h265 encoding which should reduce storage required
  • well tested on all OSes and i don't have to test myself
  • good unit tests that check in CI that ffmpeg is usable on all OSes using different runners

@Mubashirshariq
Copy link
Author

sure i will test it and share the video

@Mubashirshariq
Copy link
Author

@louis030195 i am getting the following error while running the cli installation,i am not sure what is the reason ,could you please have a look into this one?

Screenshot from 2024-08-29 17-55-41

@louis030195
Copy link
Collaborator

@Mubashirshariq are you on Linux wayland ?

ATM screenpipe vision does not work with Linux wayland (but does with X)

@Mubashirshariq
Copy link
Author

yeah i am Linux Wayland,so i think i need to wait for this one ,i am getting a new machine next week

@Mubashirshariq
Copy link
Author

Mubashirshariq commented Sep 3, 2024

@louis030195 i did comment the ffmpeg installation code in prebuild and pushed some changes and it is now working perfect ,attaching the recording for the same

https://drive.google.com/file/d/16BROsTSFbuMbSA7LAumNpqkxMMtSqH2T/view?usp=drive_link

@louis030195
Copy link
Collaborator

@Mubashirshariq cannot access the drive link, you can also drop the video in github comment fyi

will give a try

@Mubashirshariq
Copy link
Author

Mubashirshariq commented Sep 3, 2024

The size of file is big so was'nt able to upload it as cmnt.I have given you the access

@Mubashirshariq
Copy link
Author

@louis030195, have you looked into this video and the overall PR?

@louis030195
Copy link
Collaborator

@Mubashirshariq i just think this is missing lot of things, if i merge i will have to remove all the rest of the things that install ffmpeg myself like pre_build.js script, CI/CD, etc etc

@Mubashirshariq
Copy link
Author

Mubashirshariq commented Sep 9, 2024

@louis030195 if it is working fine and you tested it ,i will remove those that is not an issue

Copy link

vercel bot commented Sep 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
screenpipe ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 30, 2024 11:09am

@Mubashirshariq
Copy link
Author

Mubashirshariq commented Sep 28, 2024

@louis030195 can you take a look i have removed the ffmpeg installation from prebuild and workflows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

switch to ffmpeg-sidecar
10 participants