Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts dynamically generated metadata (and its YAML serialization) for multiple modules to better conform to MLT’s metadata schema expectations.
Changes:
- OpenFX: emit
normalized_coordinatesas YAML-style booleans (yes/no) instead of integers. - frei0r: add missing top-level metadata field
language: en. - Framework: treat
*as a reserved YAML character to ensure correct quoting during YAML serialization.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/modules/openfx/mlt_openfx.c |
Aligns normalized_coordinates output with schema bool expectations by using yes/no. |
src/modules/frei0r/factory.c |
Adds required language metadata when generating frei0r YAML dynamically. |
src/framework/mlt_properties.c |
Improves YAML output correctness by quoting strings containing *. |
Update: ResolvedThere is an issue remaining I have locally with an OpenFX filter: The problem is that our YAML serializer in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/framework/mlt_properties.c:2172
is_numeric_identifiertakeschar *seven though it only reads from the string. In this file, it is called withconst char *value, so this dropsconstand can trigger compiler warnings (or fail builds if-Werror). Make the parameterconst char *s(and keep the function read-only).
static inline int is_numeric_identifier(const char *name, char *s)
{
if (strcmp(name, "identifier"))
return 0;
while (*s) {
if (!isdigit((unsigned char) *s))
return 0;
s++;
}
return 1;
But the service that is generating the metadata knows. Maybe the service could convert the value before generating metadata with it.
I think we should not put scientific notation in the metadata so that applications do not have to deal with that complication. We could write a simple function to convert scientific notation to decimal and pass decimal through. Then, the functions that generate metadata could call that. I see you added a function to our properties yaml function to detect the "identifier" - does that mean our yaml parser is really a "MLT metadata YAML" parser? |
The metadata should be usable via API without parsing YAML. For example, your Shotcut branch is not, right? My comment is about problems with YAML serialization and kwalify.
But it's not. It is a double value. Or, do you mean the YAML? Well, maybe but many humans and YAML parsers DO handle it. My hack to convert "1e-08" to "0.00000001" is just a workaround to get tests passing. The problem is how to generalize. At that spot, the double property is retrieved as string, which gets printf-ed using "%g". I am reluctant to change that because I do not want every real number shown with a large number of zeroes in the decimal portion. How to generalize the below? } else if (!strcmp(value, "1e-08")) {
// Special case for numeric values not accepted by kwalify
strbuf_printf(output, "%.8f\n", 1e-8);Interestingly, I see other metadata numeric values in scientific notation that it does not complain about. 🤔 It looks like kwalify only has a problem when the coefficient is an integer! But I noticed it does not have a problem with a
It is a serializer that is primarily concerned with our metadata, yes. But also Shotcut uses for saving presets. |
|
I decided after some conversation with you and Claude to keep the intent of the schema and generalize the scientific to fixed point conversion. Maybe that can improve Shotcut's UI generator too, but hmm, it seems to treat a "minimum: 0.000030" as 0 currently. |
No description provided.