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.

Sign up for GitHub

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

Jump to bottom

Add support for decoding input with ffmpeg (Linux) #2133

Merged
merged 1 commit into from May 21, 2024
Merged

Add support for decoding input with ffmpeg (Linux) #2133

merged 1 commit into from May 21, 2024
+574 −2

Conversation

WilliamTambellini
Copy link
Contributor

WIP: for early review only. Do not merge.

  • add cmake option to build with ffmpeg
  • search for ffmpeg at cmake time
  • include ffmpegs/libav headers in main.cpp
    Remaining todos:
  • if needed convert input file to wav 16khz in main.cpp

@WilliamTambellini
Copy link
Contributor Author

@ggerganov @slaren could you please early review before I move on ? Best, WT.

@ggerganov
Copy link
Owner

We can add the FindFFmpeg.cmake script in the cmake folder and use it to find ffmpeg libs

Probably the conversion functionality should be implemented in common.cpp so that it can be reused by all examples, not just main

@WilliamTambellini
Copy link
Contributor Author

WilliamTambellini commented May 13, 2024 •
edited

We can add the FindFFmpeg.cmake script in the cmake folder and use it to find ffmpeg libs

done

Probably the conversion functionality should be implemented in common.cpp so that it can be reused by all examples, not just main

ok

@ggerganov ggerganov marked this pull request as draft May 14, 2024 06:42
@WilliamTambellini
Copy link
Contributor Author

@ggerganov
reready for review, tks

@WilliamTambellini
Copy link
Contributor Author

@petterreinholdtsen review please

@WilliamTambellini WilliamTambellini changed the title Add support for decoding input with ffmpeg in main (Linux) Add support for decoding input with ffmpeg (Linux) May 16, 2024
@WilliamTambellini
Copy link
Contributor Author

@arthw review please

ggerganov
ggerganov approved these changes May 19, 2024
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
examples/CMakeLists.txt Outdated Show resolved Hide resolved
examples/common.cpp Outdated Show resolved Hide resolved
examples/ffmpeg_transcode.cpp Outdated
Copy link
Owner

Choose a reason for hiding this comment

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

Change the underscore in the filename to a dash for consistency: ffmpeg-transcode.cpp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Owner

Choose a reason for hiding this comment

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

Hm, still uses underscore -> ffmpeg_transcode.cpp

@WilliamTambellini WilliamTambellini marked this pull request as ready for review May 20, 2024 17:01
@WilliamTambellini
Copy link
Contributor Author

@ggerganov retouched. Reready for final review.

@ggerganov
Copy link
Owner

Hm, I think you didn't push the correct revision - I don't see any changes since last time

@WilliamTambellini
Add support for transcoding input with ffmpeg libs (Linux)
5d16555 
- search for ffmpeg libs/headers at cmake time
- added ffmpeg-transcode.cpp into libcommon if ffmpeg on
- hooked ffmpeg trancoding in common read_wav(...)
- passed test:
./main -m ggml-base.en.bin -f samples/jfk.mp3
@WilliamTambellini
Copy link
Contributor Author

WilliamTambellini commented May 21, 2024 •
edited

oops indeed @ggerganov . Just pushed the latest retouches.

@ggerganov ggerganov merged commit 1b51fdf into ggerganov:master May 21, 2024
48 checks passed
@WilliamTambellini
Copy link
Contributor Author

tks @ggerganov
Any way to do a new minor release soon (eg 1.6.1) ?

@ggerganov
Copy link
Owner

done

@data-man
Copy link

Unfortunately cannot be built with FFmpeg 7.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ggerganov ggerganov ggerganov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@WilliamTambellini @ggerganov @data-man