I am working on a new filter (the gps graphic one) and I have a few questions related to coding that don’t deserve individual threads.
What does this _filter_private do? (as in: mlt_properties_set_int( properties, "_filter_private", 1 );) I found the description (“Set this on a service to ensure that attached filters are handled privately”) but I still don’t know what it does or if I need it. Does this have an interaction with 3.?
One of the filters I looked at/inspired from had this check in filter process: mlt_frame_is_test_card( frame ), from the references list it seems to be used only by audio filters but I don’t really understand what a test card is (I assume it’s testing for image presence?). I’m using QT to draw my stuff whether there is or not an image below, I’m thinking this might be needed to still be able to draw?
What exactly does parallel processing do in the export panel from the code perspective?
Here’s the context I’m asking this: I noticed that if I have more than 2-3 instances of my filter active, the export fails (with the popular -1073741819 code), but then it retries without parallel processing and it succeeds.
I did find a “workaround” as this started happening when I moved some code from the filter_process() call into the filter_get_image() call. This [=moving the code to filter_process()] seems to completely fix the parallel export issue BUT it somehow doesn’t work for my filter logic: I have some processing there that calculates the in-graph position depending on a UI property and current video time and if I read&calculate this in filter_process() the output does some jerky movement instead of smoothly panning around.
So this is what I’m trying to understand, how are filter_process() and filter_get_image() called when doing parallel processing and how/at what point is filter private_data struct or the filter properties I read get changed. I assume that one of these is read once for every batch of threads so it’s using a mix of previous and current data which makes the movement not appear smooth.
Is the private data duplicated for each “thread” if doing parallel or are they using the same memory and my crash is related to concurrent memory read/writes? How does this interact with reading filter properties? Is it done once per batch of threads?
Edit: ok, for 3. the parallel error might have been a coding mistake, it doesn’t crash anymore after a few code changes.
This I am not entirely clear about. I expect you to set this property non-zero if your filter will be calling mlt_service_apply_filters() per the way it is used in mlt_service.c and filter_watermark.c. However, I see some filters created by Brian that are setting this even though they do not call it.
Yes, a test card is a dummy image. I am not sure what you mean by “whether there is or not an image below.” A filter should generally assume that if it’s mlt_frame_get_image() to get an image from upstream does not error then it has an image to work with. Otherwise, what is it filtering? But a dummy image will not be included in the multitrack output. So, you only need to check and workaround this if you want the filter to always be applied for some special cases like Brian did in his audio visualization filters. Don’t do it unless you find a specific reason to.
Multiple frames are processed in parallel. To deal with this you need to call mlt_service_lock() and mlt_service_unlock() inside your get_image callback in areas of code that access shared state, typically somewhere in the filter object and its child. The process frame callback function already locks the service when the base mlt_service calls it. Thus there can only be one thread inside process at any one time, but there can be multiple threads inside get_image - and you cannot assume the position of one get_image to the next is sequential. There is no automatic data duplication, but sometimes, you need to give each thread some private snapshot-like properties to prevent them from being changed by another thread - whether in process or get_image. Search the code for mlt_frame_unique_properties() for examples of that.
Thanks, I removed the 1) and 2), and for 3) I used mlt_frame_unique_properties() but then figured out I can just reorganise the code to separate the thread independent variables into a local struct that I just process there and pass it to the functions that need it.
I have another batch of questions (I’ll continue the numbering for readability):
Is the Preview scaling support mandatory? I started with the graph.cpp implementation as an example for my filter but unlike there, I compute a lot of stuff before drawing so right now I basically do everything just as without scaling but on a smaller rectangle and with a thick line size. I can’t easily separate the more intense processing part and the output makes little sense if I skip some of it. Is the smaller rect useful in anyway for lower specs pc/laptops? I guess I could just skip point drawing so instead of iterating i++ i’d do i+=10 for the main draw loop but that’s a few milliesconds saved per frame and the drawing would have gaps. I vote I just remove all the scaling use (maybe just leave the rect resizing?) but I don’t know the standard use case for it or if the thickness/rect actually matters.
The call to: mlt_properties_frames_to_time( properties, frame_position, mlt_time_clock ) sometimes fails to return a value when using parallel mode (in export settings). I managed to work around it with a retry and my problem went away. I ended up doing up to 3 retries if it returns null (I did some tests and noticed it usually only fails once but on one occasion where I did some unreasonably stress test it failed 3 times in a row with 4th being succesfull). Is there a better way for this or is the 3xretry ok?
I noticed that on my cmake build (on windows) the #ifdef guard added for the freeBSD compile issue is not defined so it goes on #else. Does the official build have it?
It’s more of a curiosity at this point as I plan to use the suggested internal timegm which should remove the need for a timezone correction if it works out.
Edit: the internal_timegm() seems to fix the main issue I had that needed the timezone workaround so if this gets exposed/moved to a header I can just delete all the #ifdefs and the weird logic there.
And a few questions regarding the .qml interface:
Is it generally a good idea for most inputs (textbox, spinner, checkbox, etc) that change to do a filter.set() followed by a setControls() that goes through the entire UI or should I directly code in the “onChanged() { … }” only the directly affected fields? I imagine the former would do a bunch of unneeded work but it will be very readable, while the latter is more efficient but it will be harder to follow. I now have a combination of the two (usually if only one other thing is affected I do it in the onChanged() {}).
I ask this because for some reason sometimes when I click on the filter in the filter list shotcut just stops responding for 5-10s. It happens randomly and otherwise it’s just instant so I’m not sure if it’s my .qml complexity or the shotcut sdk I downloaded a while back is bugged. I’ll do more tests but curious if this is a known issue.
Is it ok to change some text labels in response to a combo-box value to make it more specific to that choice? (for example I have generic “Crop horizontal” and “Crop vertical” labels that can mean “crop latitude” or “crop time” depending on the previous choices)
Same question but for disabling elements when they don’t apply to the currently selected combobox option so they’re just useless there. I don’t think I’ve seen filters disabling elements so far (also a mention here: setting enabled:false to a combo box changes it too little to be easily noticeable, it’s just a slightly darker shade of grey - but this on Windows with Fusion dark theme).
In the GPS text filters I used a few icons as button images with this pattern: icon.source: 'qrc:///icons/dark/32x32/document-open-recent' but I (just) noticed that they all only look ok/(=readable) in the dark theme. How do I make it so it chooses automatically the dark/light version depending on theme?
How do i filter.set a value to the empty string? I am trying to set up a few helpful presets but I have 2 string properties that I can’t “clear” at all, the property is not written to the preset file (in %appdata%) and if I force it by giving it a value of " ", it gets written but not read. Moreover the next properties after this [empty space] bug-out (I assume because the reader is going out of sync due to not matching on the space string) Is there a workaround for this?
It so happens that when I built my first filter in MLT I used filter_watermark as an example. So that line of code was a carry-over from copy/paste. It should not be needed in a filter unless it calls apply_filters() as Dan suggested.
Yes. Usually it is a small matter of scaling the user provided rectangle values before performing your computations. I had not thought of scaling your line size. You could try to skip that and see how bad it is. Consider looking at the audio spectrum/waveform filters in the QT module as examples.
I have never experienced this before, and upon inspection, I do not see how it can happen. I’m sure you are eager to get your new filter working, but would you be willing to debug this a little further into the MLT function to see why it is returning NULL? If you can get some more hints, I am willing to help solve it. We do not want you to have code that retries until a good value is returned.
I prefer the mode that changes all the controls. In fact, I prefer that you try to use the same function names and patters that all the other filters use. There have been times that I needed to edit all filters in order to fix a bug or change behaviors, and it is much simpler if all filters are implemented with the same paradigm and assumptions.
Changing label names is not preferred. Maybe you can use a slash like “horizontal/latitude” or maybe you can help the user with tooltips. We can review the UI and make suggestions about tradeoffs after you have it posted.
Our general pattern has been to leave the controls enabled even when they do not apply. We try to use labels and tooltips to make it clearer when a setting applies and when it does not. But this has not been a hard rule. I suggest to leave all controls enabled for the first draft, and we can debate it during a UI review.
Did you also set the icon.name?
Example:
This is the purpose of the “clear” property. Why does this not work for you? It is OK if the property is not written to the settings file if you intend for it to be cleared anyway.
The audio spectrum scales the width via the setup_graph_pen() call, this is why I thought of also scaling it. I removed the thickness change and it looks ok (rect change still there).
Ok, I’ll try to see what’s happening.
Ok, I’ll try to use standard names there. The rect/vui/preset interaction is killing me but that’s one for the inspection phase.
After having a try I fully agree, they don’t really get refreshed when I expect them to. I think tooltips is the way to go.
I did not. This solved it.
But unlike the example I had to remove the icon.source line or it would not switch automatically (the icon I use has separate folders for dark/light).
This is in particular to the savePreset() call. Unless I’m using it wrong the idea is this: I have a string property tied to a textfield (which is set via a setControls() call in the onPresetSelected() ). When loading a particular preset I want to clear this field, but for other presets I don’t want to clear it - just let the previous value in there (well I guess in this particular instance I could always clear it in the onBeforePresetSelected() as a workaround but let’s say* I don’t want to always clear it).
*For a bit more context: I intentionally made all presets to only save a subset of the total properties because if a user loads a gps file and sets the offset or trims it just right for the video, I don’t want loading a preset to reset the previous work if it doesn’t need to.
Thanks. I understand as a developer, sometimes you see someone else’s work and you think “that isn’t how I would do it”. In the case of these filters, it is good to resist the urge to design the filter differently than the others because it makes it more difficult to maintain or extend the framework in the future.
This might not be possible with the current framework. Also, it might not meet the user’s expectations.
I tried to debug the mlt_properties_frames_to_time() to see where it’s returning NULL and it seems… nowhere . Every function return up to mlt_property_get_time() seems valid/not null.
My only (a bit absurd) guess at this point is that self->prop_string gets cleared up just between the mlt_property_get_time() return and my filter checking for the return value. I don’t know when the cleanup happens exactly but to reproduce this easily I have at least 2-3 filter instances at once (I’m actually using 12 now just to guarantee fails) and export 10s of a video with parallel processing enabled. Every filter instance calls mlt_property_get_time() at least 5 times I’d say.
I did a test and modified inside the mlt_property_get_time()'s mutex area to malloc a char *tmp and copyed the self->prop_string into it and returning that instead of self->prop_string and I had no failures at all, but this is not a valid fix, of course.
I’m trying to wrap everything up (as I realised I’m tweaking very small details for weeks now) and I don’t know how to properly use the internal_timegm() function. It’s currently defined static in framework\mlt_producer.c and for the time being I just copy pasted it into my header to make sure it works.
How should I proceed with this? move it into some .h? just let it duplicated into my header and deal with it later?
. Every function return up to mlt_property_get_time() seems valid/not null.