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: Initial work on OpenFX module support #1051

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ option(MOD_SDL2 "Enable SDL2 module" ON)
option(MOD_SOX "Enable SoX module" ON)
option(MOD_SPATIALAUDIO "Enable SpatialAudio module" ON)
option(MOD_VIDSTAB "Enable vid.stab module (GPL)" ON)
option(MOD_OPENFX "Enable OpenFX module (GPL)" ON)
option(MOD_VORBIS "Enable Vorbis module" ON)
option(MOD_XINE "Enable XINE module (GPL)" ON)
option(MOD_XML "Enable XML module" ON)
Expand Down Expand Up @@ -171,6 +172,7 @@ if(NOT GPL)
set(MOD_RESAMPLE OFF)
set(MOD_RUBBERBAND OFF)
set(MOD_VIDSTAB OFF)
set(MOD_OPENFX OFF)
set(MOD_XINE OFF)
endif()

Expand Down Expand Up @@ -633,6 +635,7 @@ add_feature_info("Module: SDL2" MOD_SDL2 "")
add_feature_info("Module: SoX" MOD_SOX "")
add_feature_info("Module: SpatialAudio" MOD_SPATIALAUDIO "")
add_feature_info("Module: vid.stab" MOD_VIDSTAB "")
add_feature_info("Module: OpenFX" MOD_OPENFX "")
add_feature_info("Module: Vorbis" MOD_VORBIS "")
add_feature_info("Module: XINE" MOD_XINE "")
add_feature_info("Module: XML" MOD_XML "")
Expand Down
4 changes: 4 additions & 0 deletions src/modules/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ if(MOD_VIDSTAB)
add_subdirectory(vid.stab)
endif()

if(MOD_OPENFX)
add_subdirectory(openfx)
endif()

if(MOD_VORBIS)
add_subdirectory(vorbis)
endif()
Expand Down
20 changes: 20 additions & 0 deletions src/modules/openfx/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
add_library(mltopenfx MODULE
factory.c
mlt_openfx.c mlt_openfx.h
filter_openfx.c
)

file(GLOB YML "*.yml")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather be explicit about the YAML files, because I think there won't be that many services here.

add_custom_target(Other_openfx_Files SOURCES
${YML}
)

target_compile_options(mltopenfx PRIVATE ${MLT_COMPILE_OPTIONS})

target_link_libraries(mltopenfx PRIVATE mlt m)
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes that every platform has libm though that might not be the case, just link it if it's needed.


set_target_properties(mltopenfx PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${MLT_MODULE_OUTPUT_DIRECTORY}")

install(TARGETS mltopenfx LIBRARY DESTINATION ${MLT_INSTALL_MODULE_DIR})

install(FILES filter_openfx.yml DESTINATION ${MLT_INSTALL_DATA_DIR}/openfx)
285 changes: 285 additions & 0 deletions src/modules/openfx/factory.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,285 @@
/*
* factory.c -- the factory method interfaces
* Copyright (C) 2024 Meltytech, LLC
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

#include "mlt_openfx.h"

extern OfxHost MltOfxHost;

static OfxStatus (*OfxSetHostFn)(const OfxHost *host);
static OfxPlugin *(*OfxGetPluginFn) (int nth);
static int (*OfxGetNumberOfPluginsFn) (void);
Comment on lines +24 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

Add typedefs for these.

static int NumberOfPlugins;
mlt_properties mltofx_context;
mlt_properties mltofx_dl;
Comment on lines +28 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

You know, using properties as sort of a dictionary seems kinda wrong to me, if I were to do this I'd rather use std::map from C++ or any implementation that has a way to store structured data by key.


#if defined (__linux__) || defined (__FreeBSD__)

#define OFX_DIRLIST_SEP_CHARS ":;"
#define OFX_DIRSEP "/"
#include <dirent.h>

static const char *getArchStr()
{
if(sizeof(void *) == 4) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that there's a precompiler definition out there that describes the architecture bits, look for it if there's one.

#if defined(__linux__)
return "Linux-x86";
#else
return "FreeBSD-x86";
#endif
}
else {
#if defined(__linux__)
return "Linux-x86-64";
#else
return "FreeBSD-x86-64";
#endif
}
}

#define OFX_ARCHSTR getArchStr()

#elif defined (__APPLE__)

#define OFX_DIRLIST_SEP_CHARS ";:"
#if defined(__x86_64) || defined(__x86_64__)
#define OFX_ARCHSTR "MacOS-x86-64"
#else
#define OFX_ARCHSTR "MacOS"
#endif
#define OFX_DIRSEP "/"
#include <dirent.h>

#elif defined (WINDOWS)
#define OFX_DIRLIST_SEP_CHARS ";"
#ifdef _WIN64
#define OFX_ARCHSTR "win64"
#else
#define OFX_ARCHSTR "win32"
#endif
#define OFX_DIRSEP "\\"

#include "shlobj.h"
#include "tchar.h"
#endif

extern mlt_filter filter_openfx_init(mlt_profile profile,
mlt_service_type type,
const char *id,
char *arg);

static void plugin_mgr_destroy(mlt_properties p)
{
int cN = mlt_properties_count(mltofx_context);

int j;
for (j = 0; j < cN; ++j)
{
char *id = mlt_properties_get_name (mltofx_context, j);
mlt_properties pb = (mlt_properties) mlt_properties_get_data(mltofx_context, id, NULL);

char *dli = mlt_properties_get (pb, "dli");
dli[0] = 'g';
int index = mlt_properties_get_int (pb, "index");

OfxPlugin *(*GetPluginFn) (int nth) = mlt_properties_get_data(mltofx_dl, dli, NULL);

if (GetPluginFn != NULL)
{
OfxPlugin *pt = GetPluginFn (index);
if (pt == NULL) continue;
pt->mainEntry (kOfxActionUnload, NULL, NULL, NULL);
}
}

int N = mlt_properties_get_int (p, "N");

int i;
for (i = 0; i < N; ++i)
{
char tstr[12] = {'\0',};
sprintf (tstr, "%d", i);
void *current_dlhandle = mlt_properties_get_data(mltofx_dl, tstr, NULL);
dlclose (current_dlhandle);
}
}

static mlt_properties metadata(mlt_service_type type, const char *id, void *data)
{
char file[PATH_MAX];
snprintf(file, PATH_MAX, "%s/openfx/%s", mlt_environment("MLT_DATA"), (char *) data);
mlt_properties result = mlt_properties_parse_yaml(file);

mlt_properties pb = (mlt_properties) mlt_properties_get_data(mltofx_context, id, NULL);

char *dli = mlt_properties_get (pb, "dli");
int index = mlt_properties_get_int (pb, "index");

OfxPlugin *(*GetPluginFn) (int nth) = mlt_properties_get_data(mltofx_dl, dli, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Use a OfxGetPlugin typedef here.

OfxPlugin *pt = GetPluginFn (index);

dli[0] = 's';
OfxStatus (*OfxSetHost)(const OfxHost *host) = mlt_properties_get_data(mltofx_dl, dli, NULL);
Comment on lines +136 to +137
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you trying to prepend an s? Also, please use a OfxSetHost typedef.

if (OfxSetHost != NULL) OfxSetHost (&MltOfxHost);

mlt_properties_set (result, "identifier", id);
mlt_properties_set (result, "title", id);

/* parameters */
mlt_properties params = mlt_properties_new();
mlt_properties_set_data(result,
"parameters",
params,
0,
(mlt_destructor) mlt_properties_close,
NULL);

mltofx_fetch_params (pt, params);
return result;
}

MLT_REPOSITORY
{
MltOfxHost.host = (OfxPropertySetHandle) mlt_properties_new ();
mltofx_init_host_properties (MltOfxHost.host);

char *dir, *openfx_path = getenv("OFX_PLUGIN_PATH");
Copy link
Contributor

Choose a reason for hiding this comment

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

Brilliant that you've thought ahead about multiple paths, though, is there a way to set a default?

size_t archstr_len = strlen (OFX_ARCHSTR);

mltofx_context = mlt_properties_new ();
mltofx_dl = mlt_properties_new ();

if (openfx_path)
{
int dli = 0;

for (dir = strtok(openfx_path, MLT_DIRLIST_DELIMITER); dir;
dir = strtok(NULL, MLT_DIRLIST_DELIMITER))
Comment on lines +171 to +172
Copy link
Contributor

Choose a reason for hiding this comment

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

Can strtok_r or strtok_s be considered instead?

{
size_t dir_len = strlen (dir);

DIR *d = opendir(dir);
if (!d) continue;

struct dirent *de = readdir(d);
while (de)
{
char *name = de->d_name;

char *bni = NULL;
if ((bni = strstr (name, ".ofx.bundle")) != NULL)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will grab *.ofx.bundle.bak, which might not be desirable.

{
char *barename = strndup (name, (int) (bni - name) + 4);
size_t name_len = (size_t) (bni - name) + 4 + 7;
/* 12b sizeof `Contents` word, 1 sizeof null byte */
char *binpath = malloc(dir_len + name_len + 12 + (name_len-7) + archstr_len + 1);
sprintf(binpath, "%s/%s/Contents/%s/%s", dir, name, OFX_ARCHSTR, barename);
Copy link
Contributor

Choose a reason for hiding this comment

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

If OFX_ARCHSTR is defined to a string literal then it could be placed inside this format literal.

Suggested change
sprintf(binpath, "%s/%s/Contents/%s/%s", dir, name, OFX_ARCHSTR, barename);
sprintf(binpath, "%s/%s/Contents/" OFX_ARCHSTR "/%s", dir, name, barename);


void *dlhandle = dlopen (binpath, RTLD_LOCAL | RTLD_LAZY);

OfxSetHostFn = dlsym (dlhandle, "OfxSetHost");

OfxGetPluginFn = dlsym (dlhandle, "OfxGetPlugin");
OfxGetNumberOfPluginsFn = dlsym (dlhandle, "OfxGetNumberOfPlugins");
Comment on lines +193 to +198
Copy link
Contributor

Choose a reason for hiding this comment

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

You should take into account that dlopen and dlsym can fail to do their job because either there was no loadable library to begin with or the symbols aren't present. Besides that, this is not going to work on Windows, though we can let this slide ... (just use what is in the WinAPI or push to someone else that job).

NumberOfPlugins = OfxGetNumberOfPluginsFn ();

char dl_n[16] = {'\0',};
sprintf (dl_n, "%d", dli);

mlt_properties_set_data(mltofx_dl,
dl_n,
dlhandle,
0,
NULL,
NULL);
dl_n[0] = '\0';
Copy link
Contributor

Choose a reason for hiding this comment

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

You are going to sprintf on it regardless ...

sprintf (dl_n, "gn%d", dli);
mlt_properties_set_data(mltofx_dl,
dl_n,
OfxGetNumberOfPluginsFn,
0,
(mlt_destructor) mlt_properties_close,
NULL);
dl_n[0] = '\0';
sprintf (dl_n, "s%d", dli);
mlt_properties_set_data(mltofx_dl,
dl_n,
OfxSetHostFn,
0,
(mlt_destructor) mlt_properties_close,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are dlsym symbols supposed to be destroyed?

NULL);

dl_n[0] = '\0';
sprintf (dl_n, "g%d", dli);
mlt_properties_set_data(mltofx_dl,
dl_n,
OfxGetPluginFn,
0,
(mlt_destructor) mlt_properties_close,
NULL);

dli++;

if (OfxGetPluginFn == NULL) goto parse_error;

int i;
for (i = 0; i < NumberOfPlugins; ++i)
{
OfxPlugin *plugin_ptr = OfxGetPluginFn (i);

char *s = NULL;
size_t pluginIdentifier_len = strlen (plugin_ptr->pluginIdentifier);
s = malloc (pluginIdentifier_len + 8);
sprintf (s, "openfx.%s", plugin_ptr->pluginIdentifier);

mlt_properties p;
p = mlt_properties_new ();
mlt_properties_set_data(mltofx_context,
s,
p,
0,
(mlt_destructor) mlt_properties_close,
NULL);

mlt_properties_set(p, "dli", dl_n);
mlt_properties_set_int(p, "index", i);

/* WIP: this is only creating them as filter I should find a way to see howto detect producers
if they exists in OpenFX plugins
*/
MLT_REGISTER(mlt_service_filter_type, s, filter_openfx_init);
MLT_REGISTER_METADATA(mlt_service_filter_type, s, metadata, "filter_openfx.yml");

}


parse_error:

free (binpath);
free (barename);
}

de = readdir(d);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

You should call closedir here or somewhere else.

}
Copy link
Contributor

Choose a reason for hiding this comment

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

You are mixing tabulation characters here, please run clang-format or ninja astyle.


mlt_properties_set_int(mltofx_dl, "N", dli);
mlt_factory_register_for_clean_up(mltofx_dl, (mlt_destructor) plugin_mgr_destroy);
}
}
Loading