Hi all, I'd like to propose yet another light NMS API change before to get dirty into builiding last missing pieces for 1.1.0 release.
The situation ------------- Until now, the assumption that a single instance of module can be used for just ONE processing step. Take as example a not-yet-implemented MPEG module that can do BOTH demuxer and decoding. The implicit assumption for NMS that there is two different instances (= two different TCModule s), one for demux and one for decoding. In other words one CANNOT use the same instance both for demuxing and for decoding. Strange things can happen (= code and interfaces wasn't designed to allow so). Another example: encode_lavc can be encode both audio and video. Well, is NOT possible to reuse the same instance. We need TWO instances, one for encoding audio and one for encoding video. Until now that's just implicit or not explicitely checked in code. Reason is fairly simple: there isn't modules that can do more than one thing (i.e., encoding both audio and video). Yet. Well, in facts some filters and import_pv3 (at least) can do more that one thing, but filters and import modules are still used in back-compatible way, so new interface isn't stressed yet :) Why we need so? --------------- Take again encode_lavc as example. Problems arising at configure() step. If module instance can distinguish if it's supposed to encode audio, video or both, it will configure() just everything (since this the most conservative way to go). That will easily lead to troubles: transcode -y lavc,lame,avi -N mpeg4,mp3 [...] is a legal commandline but causes trouble since 1) encode_lavc (incorrectly) try to initialize BOTH video and audio encoders 2) this will fail since mp3 encoding is NOT supported by encode_lavc (and it will never be since encode_lame exists...) 3) anyway, configuring both video and audio and allocating necessary Contexts, buffer and data is resource waste at least. So, we need a way to tell a module what is supposed to do. This save resources, avoid troubles, and causes very little or no pain with existing modules. The proposal ------------ We need another parameter to specify expected features (in TC_MODULE_FEATURE_* sense) from it. We can 1) add a `shadow' implicit parameter to const char *options in configure(), something like "enable_audio:other_params" that's quite ugly and I think will lead to trouble in feature 2) add another parameter to specify features using TC_MODULE_FEATURE_* flags. since this change API/ABI, I think it's better, following this way 3) add a parameter at init() doing so allow module to save more resources, since if, say, audio buffers aren't needed, they cannot be allocated early. To complete the picture, we need some code to ensure that a given module can honour requested features, and that feature request is coherent. Attached patch address both issues outlined above. Feel free to comment. Of course I'm very open to further suggestions to address above issues in a better way, even if proposed solution (see patch) looks quite clean and acceptable to me. Current status -------------- I'd like to commit attached patch as well as other patches that adapt to new API *all* existing NMS-enabled code. That will happen in the next few days (this weekend most probably) unless someone objects or propose something better. Feel free to ask comments if some pieces aren't clear enough. Thoughts? Best regards, -- Francesco Romani - Ikitt ['people always complain, no matther what you do'] IM contact : (email first, Antispam default deny!) icq://27-83-87-867 tiny homepage : http://fromani.exit1.org (see IDEAS if you want send code!) known bugs : http://tcfoundry.hostme.it/mantis (EXPERIMENTAL)
? libtc/extradata.h Index: libtc/tcmodule-core.h =================================================================== RCS file: /cvstc/transcode/libtc/tcmodule-core.h,v retrieving revision 1.16 diff -u -r1.16 tcmodule-core.h --- libtc/tcmodule-core.h 3 Feb 2007 11:55:23 -0000 1.16 +++ libtc/tcmodule-core.h 7 Feb 2007 20:50:16 -0000 @@ -280,10 +280,11 @@ * method on given module. You should NOT do by yourself. * * Parameters: - * factory: use this factory instance to build the new module. + * factory: use this factory instance to build the new module. * modclass: class of module requested (filter, encoding, * demultiplexing...). - * modnale: name of module requested. + * modname: name of module requested. + * media: media type for new module (TC_VIDEO, TC_AUDIO, TC_EXTRA) * * Return value: * NULL: an error occurred, and notified via tc_log_*() @@ -306,7 +307,7 @@ * TCModule my_module = tc_new_module("filter", "foobar"); */ TCModule tc_new_module(TCFactory factory, - const char *modclass, const char *modname); + const char *modclass, const char *modname, int media); /* * tc_del_module: Index: libtc/tcmodule-data.h =================================================================== RCS file: /cvstc/transcode/libtc/tcmodule-data.h,v retrieving revision 1.15 diff -u -r1.15 tcmodule-data.h --- libtc/tcmodule-data.h 24 Oct 2006 08:14:31 -0000 1.15 +++ libtc/tcmodule-data.h 7 Feb 2007 20:50:17 -0000 @@ -64,6 +64,7 @@ struct tcmoduleinstance_ { int id; /* instance id; */ const char *type; /* packed class + name of module */ + uint32_t features; /* subset of enabled features for this instance */ void *userdata; /* opaque to factory, used by each module */ @@ -91,7 +92,7 @@ const TCModuleInfo *info; /* mandatory operations: */ - int (*init)(TCModuleInstance *self); + int (*init)(TCModuleInstance *self, uint32_t features); int (*fini)(TCModuleInstance *self); int (*configure)(TCModuleInstance *self, const char *options, vob_t *vob); int (*stop)(TCModuleInstance *self); @@ -129,7 +130,8 @@ * is requested. Request an operation in a initialized but unconfigured * module will result in an undefined behaviour. * Parameters: - * self: pointer to module instance to initialize. + * self: pointer to module instance to initialize. + * features: select feature of this module to initialize. * Return Value: * 0 succesfull. * -1 error occurred. A proper message should be sent to user using Index: libtc/tcmodule-info.h =================================================================== RCS file: /cvstc/transcode/libtc/tcmodule-info.h,v retrieving revision 1.7 diff -u -r1.7 tcmodule-info.h --- libtc/tcmodule-info.h 11 Nov 2006 20:51:14 -0000 1.7 +++ libtc/tcmodule-info.h 7 Feb 2007 20:50:17 -0000 @@ -23,6 +23,7 @@ /* FIXME: move to a enum? */ /* actions */ #define TC_MODULE_FEATURE_NONE 0x00000000 + #define TC_MODULE_FEATURE_FILTER 0x00000001 #define TC_MODULE_FEATURE_DECODE 0x00000002 #define TC_MODULE_FEATURE_ENCODE 0x00000004 Index: libtc/tcmodule-plugin.h =================================================================== RCS file: /cvstc/transcode/libtc/tcmodule-plugin.h,v retrieving revision 1.10 diff -u -r1.10 tcmodule-plugin.h --- libtc/tcmodule-plugin.h 11 Nov 2006 20:58:09 -0000 1.10 +++ libtc/tcmodule-plugin.h 7 Feb 2007 20:50:17 -0000 @@ -29,6 +29,63 @@ } while (0) +#define TC_HAS_FEATURE(flags, feat) \ + ((flags & (TC_MODULE_FEATURE_ ## feat)) ?1 :0) + +static inline int tc_module_av_check(uint32_t flags) +{ + int i = 0; + + i += TC_HAS_FEATURE(flags, AUDIO); + i += TC_HAS_FEATURE(flags, VIDEO); + i += TC_HAS_FEATURE(flags, EXTRA); + + return i; +} + +static inline int tc_module_cap_check(uint32_t flags) +{ + int i = 0; + + i += TC_HAS_FEATURE(flags, DECODE); + i += TC_HAS_FEATURE(flags, FILTER); + i += TC_HAS_FEATURE(flags, ENCODE); + i += TC_HAS_FEATURE(flags, MULTIPLEX); + i += TC_HAS_FEATURE(flags, DEMULTIPLEX); + + return i; +} + +#undef TC_HAS_FEATURE + + +#define TC_MODULE_INIT_CHECK(self, FEATURES, feat) do { \ + int j = tc_module_cap_check((feat)); \ + \ + if ((!((FEATURES) & TC_MODULE_FEATURE_MULTIPLEX) \ + && !((FEATURES) & TC_MODULE_FEATURE_DEMULTIPLEX)) \ + && (tc_module_av_check((feat)) > 1)) { \ + tc_log_error(MOD_NAME, "unsupported stream types for" \ + " this module instance"); \ + return TC_ERROR; \ + } \ + \ + if (j != 0 && j != 1) { \ + tc_log_error(MOD_NAME, "feature request mismatch for" \ + " this module instance (req=%i)", j); \ + return TC_ERROR; \ + } \ + /* is perfectly fine to request to do nothing */ \ + if ((feat != 0) && ((FEATURES) & (feat))) { \ + (self)->features = (feat); \ + } else { \ + tc_log_error(MOD_NAME, "this module does not support" \ + " requested feature"); \ + return TC_ERROR; \ + } \ +} while (0) + + /* * plugin entry point prototype */ Index: libtc/tcmodule.c =================================================================== RCS file: /cvstc/transcode/libtc/tcmodule.c,v retrieving revision 1.24 diff -u -r1.24 tcmodule.c --- libtc/tcmodule.c 30 Jan 2007 20:15:29 -0000 1.24 +++ libtc/tcmodule.c 7 Feb 2007 20:50:17 -0000 @@ -29,8 +29,8 @@ #define TC_FACTORY_MAX_HANDLERS (16) #define MOD_TYPE_MAX_LEN (TC_BUF_MIN * 2) -#define tc_module_init(module) \ - (module)->klass->init(&((module)->instance)) +#define tc_module_init(module, features) \ + (module)->klass->init(&((module)->instance), features) #define tc_module_fini(module) \ (module)->klass->fini(&((module)->instance)) @@ -94,7 +94,7 @@ } while (0) -static int dummy_init(TCModuleInstance *self) +static int dummy_init(TCModuleInstance *self, uint32_t features) { DUMMY_HEAVY_CHECK(self, "initialization"); return -1; @@ -233,40 +233,55 @@ *************************************************************************/ /* - * is_known_modclass: + * translate_modclass: * validate a module class name, represented by a given string. * * Parameters: * modclass: a class nome to validate. * Return Value: - * TC_TRUE if given class name can be understanded -and builded- - * by actual factory code. - * TC_FALSE otherwise + * TC_MOFULE_FEATURE_* correspondent. */ -static int is_known_modclass(const char *modclass) +static uint32_t translate_modclass(const char *modclass) { - int ret = TC_FALSE; + uint32_t ret = TC_MODULE_FEATURE_NONE; if (modclass != NULL) { if (!strcmp(modclass, "filter")) { - ret = TC_TRUE; + ret = TC_MODULE_FEATURE_FILTER; } else if (!strcmp(modclass, "demultiplex") || !strcmp(modclass, "demux")) { - ret = TC_TRUE; + ret = TC_MODULE_FEATURE_DEMULTIPLEX; } else if (!strcmp(modclass, "decode")) { - ret = TC_TRUE; + ret = TC_MODULE_FEATURE_DECODE; } else if (!strcmp(modclass, "encode")) { - ret = TC_TRUE; + ret = TC_MODULE_FEATURE_ENCODE; } else if (!strcmp(modclass, "multiplex") || !strcmp(modclass, "mplex")) { - ret = TC_TRUE; - } else { - ret = TC_FALSE; + ret = TC_MODULE_FEATURE_MULTIPLEX; } } return ret; } +/* FIXME: writeme */ +static uint32_t translate_media(int media) +{ + uint32_t ret = TC_MODULE_FEATURE_NONE; + + switch (media) { + case TC_VIDEO: + ret = TC_MODULE_FEATURE_VIDEO; + break; + case TC_AUDIO: + ret = TC_MODULE_FEATURE_AUDIO; + break; + case TC_EXTRA: + ret = TC_MODULE_FEATURE_EXTRA; + break; + } + return ret; +} + /* * TCModuleDescriptorIter: * generic iterator function on factory descriptors. @@ -786,14 +801,16 @@ } TCModule tc_new_module(TCFactory factory, - const char *modclass, const char *modname) + const char *modclass, const char *modname, + int media) { char modtype[MOD_TYPE_MAX_LEN]; + uint32_t flags = translate_modclass(modclass); int id = -1, ret; TCModule module = NULL; RETURN_IF_INVALID_QUIET(factory, NULL); - if (!is_known_modclass(modclass)) { + if (flags == TC_MODULE_FEATURE_NONE) { TC_LOG_DEBUG(factory, TC_INFO, "unknown module class '%s'", modclass); return NULL; @@ -819,7 +836,7 @@ module->instance.id = factory->instance_count + 1; module->klass = &(factory->descriptors[id].klass); - ret = tc_module_init(module); + ret = tc_module_init(module, flags|translate_media(media)); if (ret != 0) { TC_LOG_DEBUG(factory, TC_DEBUG, "initialization of '%s' failed" " (code=%i)", modtype, ret); Index: src/encoder.c =================================================================== RCS file: /cvstc/transcode/src/encoder.c,v retrieving revision 1.50 diff -u -r1.50 encoder.c --- src/encoder.c 24 Oct 2006 08:54:37 -0000 1.50 +++ src/encoder.c 7 Feb 2007 20:50:17 -0000 @@ -565,19 +565,20 @@ } mod_name = (a_mod == NULL) ?TC_DEFAULT_EXPORT_AUDIO :a_mod; - encdata.aud_mod = tc_new_module(encdata.factory, "encode", mod_name); + encdata.aud_mod = tc_new_module(encdata.factory, "encode", mod_name, TC_AUDIO); if (!encdata.aud_mod) { tc_log_error(__FILE__, "can't load audio encoder"); return -1; } mod_name = (v_mod == NULL) ?TC_DEFAULT_EXPORT_VIDEO :v_mod; - encdata.vid_mod = tc_new_module(encdata.factory, "encode", mod_name); + encdata.vid_mod = tc_new_module(encdata.factory, "encode", mod_name, TC_VIDEO); if (!encdata.vid_mod) { tc_log_error(__FILE__, "can't load video encoder"); return -1; } mod_name = (m_mod == NULL) ?TC_DEFAULT_EXPORT_MPLEX :m_mod; - encdata.mplex_mod = tc_new_module(encdata.factory, "multiplex", mod_name); + encdata.mplex_mod = tc_new_module(encdata.factory, "multiplex", mod_name, + TC_VIDEO|TC_AUDIO); if (!encdata.mplex_mod) { tc_log_error(__FILE__, "can't load multiplexor"); return -1; Index: src/tc_defaults.h =================================================================== RCS file: /cvstc/transcode/src/tc_defaults.h,v retrieving revision 1.28 diff -u -r1.28 tc_defaults.h --- src/tc_defaults.h 14 Sep 2006 14:10:32 -0000 1.28 +++ src/tc_defaults.h 7 Feb 2007 20:50:17 -0000 @@ -111,10 +111,12 @@ #define TC_THREADS 512 //import/export/filter frame buffer status flag +#define TC_NONE 0 #define TC_VIDEO 1 #define TC_AUDIO 2 #define TC_SUBEX 4 #define TC_RESERVED 8 +#define TC_EXTRA 16 #define TC_FILTER_INIT 16 #define TC_PRE_S_PROCESS 32