Skip to content

Make avdevice library optional again#1246

Open
dimula73 wants to merge 1 commit into
mltframework:masterfrom
dimula73:kazakov/make-avdevice-optional
Open

Make avdevice library optional again#1246
dimula73 wants to merge 1 commit into
mltframework:masterfrom
dimula73:kazakov/make-avdevice-optional

Conversation

@dimula73

Copy link
Copy Markdown
Contributor

It is not used by avformat when performing playback via SDL2, the only thing it does is initializing avdevice and not using it later.

This is a follow-up of the previous rejected MR: #1239

@ddennedy ddennedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

working for me with avdevice included 👍

@ddennedy ddennedy added this to the v7.40.0 milestone May 12, 2026
@jlskuz

jlskuz commented May 18, 2026

Copy link
Copy Markdown
Contributor

Making the dependency optional is in principle fine for me, but I would prefer to have it explicitly managed with a WITH_AVDEVICE switch. I know we still have this implicit behavior in other places (like for ladaspa.h) still, but my goal would be to have them all controlled explicitly in the future. See #1115 for more context.

Comment thread src/modules/avformat/CMakeLists.txt Outdated
Comment thread src/modules/avformat/factory.c Outdated
Comment thread src/modules/avformat/factory.c Outdated
It is not used by avformat when performing
playback via SDL2, the only thing it does is
initializing avdevice and not using it later.
@dimula73 dimula73 force-pushed the kazakov/make-avdevice-optional branch from b41eb38 to 516b077 Compare May 21, 2026 11:22
@dimula73

Copy link
Copy Markdown
Contributor Author

Hi, @jlskuz!

I have applied your USE_AVDEVICE suggestions. Does this patch need any other changes before it could be merged?

@ddennedy

ddennedy commented Jun 10, 2026

Copy link
Copy Markdown
Member

@jlskuz prefers that options be explicit instead of discovered by what is available. For example, for the jackrack module that originally provided only ladspa we added in the top level CMakeLists.txt

option(USE_LV2 "Enable LV2 features" ON)  
option(USE_VST2 "Enable LV2 features" ON)

I think the same is desired for avformat:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 36c8d82f..ea7745f3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -20,6 +20,7 @@ option(CLANG_FORMAT "Enable Clang Format" ON)
 option(BUILD_TESTS_WITH_QT6 "Build test against Qt 6" ON)

 option(MOD_AVFORMAT "Enable avformat module" ON)
+option(USE_AVDEVICE "Integrate FFmpeg libavdevice for capture" ON)
 option(MOD_DECKLINK "Enable DeckLink module" ON)
 option(MOD_FREI0R "Enable Frei0r module" ON)
 option(MOD_GDK "Enable GDK module" ON)
diff --git a/src/modules/avformat/CMakeLists.txt b/src/modules/avformat/CMakeLists.txt
index aebb4637..3b197daa 100644
--- a/src/modules/avformat/CMakeLists.txt
+++ b/src/modules/avformat/CMakeLists.txt
@@ -51,7 +51,7 @@ if(WIN32)
   endif()
 endif()

-if(AVDEVICE_FOUND)
+if(USE_AVDEVICE AND AVDEVICE_FOUND)
   target_compile_definitions(mltavformat PRIVATE USE_AVDEVICE)
 endif()

Beyond that, I tested this branch again, and what you have still works for me.

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.

3 participants