new filter: Dynamic GPS text

[dany123]

This is a filter I've been working for the last 2 months and I managed to implement most of the features I needed or wanted, there are still some small TODOs in the code but nothing major. As I'm not familiar at all with the mlt framework I suspect there's quite a bit of changes I need to make to be accepted so feel free to comment on them.

The file got pretty large but it's easy to follow from bottom up. The base logic is identical to dynamic text filter on top of which I added the gps related keywords.

One important note: I never got around to succesfully compiling the entire mlt project, I only got as far as compiling inside the plus folder to create libmltplus.dll to test with melt.exe (and shotcut) but after the latest pull from upstream I can't compile anymore at all (undefined references to mlt_event_data_from_frame and some other similar in folder). I probably messed up some paths or need to update some other dlls too. The files still compile succesfully on my original dev folder so I assume they are ok C-syntax-wise.

---

melt usage: melt.exe <video file path> -filter dynamicgpstext gps.file=<.gpx or .tcx file path> (+offset: "major_offset=-7200" if video is 2h ahead of gps)
Sample video with filter applied: https://youtu.be/29dsPm5l3jg

[ddennedy]

after the latest pull from upstream I can't compile anymore at all

There are major changes happening with MLT with a major new version 7 whose branch I merged to master today. This probably does not affect you too much, but what will catch you soon is our removal of the old configure and Makefile-based build system and replaced by CMake. This addition will have to go into that branch and will not be accepted into the v6 branch. Shotcut development is already based on the new version, and there will be a new SDK soon. You just happened to become a new developer at a bad time for you.

I can share you that one thing I am not happy about is the length of the filter code and its home-grown XML parser. I am no longer accepting just any contribution because there is a maintenance cost to it. On the Shotcut forum I did suggest that you use the xml module's libxml or qt module's Qt parser.

[dany123]

On the Shotcut forum I did suggest that you use the xml module's libxml or qt module's Qt parser.

I understood that completely different, as in I can't use the parsers unless I move the filter into that folder and that would create licensing problems or something, so I thought I have to make my own instead.

[ddennedy]

Yes, you would need to move your filter into the module of the respective library you are using. But there is no hard requirement that you use the plus module for this filter.

[dany123]

I assumed that because dynamic text was in plus module I also needed to put this filter in plus.
I'll update the parsing using one of the 2. I assume I can use any of them just by adapting the 2 functions, there's no need to update anything else (like the text printing method or switching to c++) correct?

[ddennedy]

You do not need to worry about the text rendering because you are driving an external text filter similar to dynamictext. Qt is C++ whereas libxml2 is plain old C. At least with libxml2 you can see our usage of it and follow it along with the docs. The xml producer uses the faster callback-based parser, but it has a object tree API if you prefer that. I guess a GPX file could have a lot of data, and that will be faster and less memory using the callback API.

[ddennedy]

I want to mention that I do like the concept for this filter and appreciate the effort you have made to contribute!

[bmatherly]

Here are a few comments for your consideration. I will make a more thorough review after you have resolved the major comments.

I would also echo Dan's comments in thanking you for this great contribution. I expect many people will enjoy this fun feature. I hope you do not become discouraged by the review comments. We will help you usher this change to the end.

[bmatherly]

I find it cumbersome that there are two offsets. Could there just be one offset and let the application be responsible for adding two different values if that is needed?

[dany123]

The minor_offset property appeared as a bugfix to the shotcut ui i made (it lost the value it stored in some circumstances, like after closing and opening the project, I think even if minimizing the window). The UI element is extermely useful and I would really not want to remove it, but the filter property I don't care about.

On the other hand it's completely optional so I didn't think it would matter much.

[bmatherly]

Maybe it will be more obvious to me when I see the filter UI. But based on the YML text, I am imagining the UI has some abstract settings that might be difficult for users to understand. Why can't the UI just have one offset?

[dany123]

The major offset has textboxes for days/hours/mins/seconds, the minor one is just a slider with a -60 to 60 seconds range that can be changed (and processed in realtime) using the mouse scrollwheel. Of course I can just move the minor value into the major one but in practice I noticed I almost always end up splitting the video in multiple clips and just keep the major offset identical across the gps file while adjusting the minor one a few seconds forward/backwards to match some obvious movements in the particular clip.

[dany123]

I forgot to mention in the first post a sample run, for melt the minimum arguments would look like this:
melt.exe <video file path> -filter dynamicgpstext gps.file=<.gpx or .tcx file path>
Everything else has sane defaults. If the output is only "--"s, the files aren't synced. A good start is the timezone in seconds as "major_offset=-7200" (if video is 2 hours ahead)

And this is a sample shotcut exported clip: https://youtu.be/29dsPm5l3jg

[bmatherly]

A good start is the timezone in seconds as "major_offset=-7200" (if video is 2 hours ahead)

Maybe tz_offset would be a good field to add?

[dany123]

Maybe tz_offset would be a good field to add?

The videofile_timezone_seconds property does exactly that. I don't know how read only properties can be used in melt, I tried major_offset=videofile_timezone_seconds and it didn't seem to work. Maybe because it's not available at init and only after filter_process?

[ddennedy]

https://www.msys2.org is your friend for windows dependencies outside of the ones included in the SDK. You also typically need to run make distclean and run configure (or cmake) with your own arguments. For example, ./configure --prefix=/c/Projects/Shotcut --enable-gpl --enable-gpl3 --without-kde --disable-sdl --disable-gdk --disable-gtk2 --disable-kino --disable-vorbis --enable-debug. Then, you can either ignore failed dependencies or search msys2 packages with pacman -Ss ___ to try to satisfy them. You do not need to get everything working. Installing is required for anything to affect Shotcut, and if you only want to install the new xml module with your addition make -C src/modules/xml.

[dany123]

I installed msys2 (and pkg-config + libxml2, not sure if these were needed) but I get this weird error that it can't find the tree.h file even though that header is at the path in the gcc command (c/Projects/libxml2-2.9.10/include/libxml). Any idea why it can't find it?

Daniel@DESKTOP MSYS /c/Projects/Shotcut/src/mlt/src/modules/xml
$ make
gcc -Ic:/Projects/Shotcut/include -DHAVE_STRUCT_TIMESPEC -I../.. -DARCH_X86_64 -Wall -DPIC    -DUSE_MMX -mmmx -DUSE_SSE -msse -DUSE_SSE2 -msse2 -g -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -I/c/Projects/libxml2-2.9.10/include/libxml -I/usr/local/include   -c -o consumer_xml.o consumer_xml.c
consumer_xml.c:28:10: fatal error: libxml/tree.h: No such file or directory
 #include <libxml/tree.h>
          ^~~~~~~~~~~~~~~
compilation terminated.
mingw32-make: *** [<builtin>: consumer_xml.o] Error 1

[bmatherly]

Did you install it using "pacman -S", or manually somehow?

I usually install all these packages since that is what we use in our build system:
https://github.com/mltframework/shotcut/blob/master/.github/workflows/build-sdk-windows.yml#L17

[dany123]

I installed it both ways at some point trying to fix the problem.
I now reinstalled a bunch of stuff, but in the end I changed the path to an absolute one in the libxml-2.0.pc file, changed $PKG_CONFIG_PATH to it's location inside msys64 folder and now seems to be ok.
But I can't compile anymore due to this other error: undefined reference to WinMain, I think since installing msys2 some environment variables have been messed up as same error appears in the plus directory.

$  make -C src/modules/xml
mingw32-make: Entering directory 'C:/Projects/Shotcut/src/mlt/src/modules/xml'
gcc  -o ../libmltxml.so factory.o consumer_xml.o producer_xml.o common.o -Lc:/Projects/Shotcut/lib -Lc:/Projects/Shotcut -L../../framework -lmlt -lpthread -LC:/MyPrograms/msys64/mingw64/lib -lxml2
C:/Qt/Tools/mingw810_64/bin/../lib/gcc/x86_64-w64-mingw32/8.1.0/../../../../x86_64-w64-mingw32/lib/../lib/libmingw32.a(lib64_libmingw32_a-crt0_c.o):crt0_c.c:(.text.startup+0x2e): undefined reference to `WinMain'
collect2.exe: error: ld returned 1 exit status
mingw32-make: *** [Makefile:23: ../libmltxml.so] Error 1
mingw32-make: Leaving directory 'C:/Projects/Shotcut/src/mlt/src/modules/xml'

[bmatherly]

I do not know the specific solution to your problem. There might be a hint here:
https://stackoverflow.com/questions/58324230/undefined-reference-to-winmain-c-mingw
But maybe it is too much effort to repair your environment.

I would suggest to start over:

1. Delete all the source and install directories you have
2. Install msys (done)
3. Install all these dependencies: https://github.com/mltframework/shotcut/blob/master/.github/workflows/build-sdk-windows.yml#L17
4. Execute these commands: https://github.com/mltframework/shotcut/blob/master/.github/workflows/build-sdk-windows.yml#L53
   (except exclude the "-s" switch so that it does not create the SDK zip file)

Then, you should have a working build and build environment.
When this is done, you can edit build-shotcut.conf with some switches to make compile-run cycles faster:

ACTION_GET=0
ACTION_CONFIGURE=0
ACTION_COMPILE_INSTALL=1
ACTION_CLEAN_SOURCE=0
DEBUG_BUILD=1
DEPLOY=1
SUBDIRS="shotcut"

Or hook in QT Creator if you want to use that.

[vpinon]

IIRC there are also tricks regarding winmain when using SDL in melt...

[dany123]

I think I got this figured out, it was a combination of having the entire msys lib folder added and not cleaning properly the entire project. I basically copied the needed stuff from pacman (btw needed the libxml2-devel package to get all dependencies) directly into shotcut's project folder /include and /lib, fixed the pkg-config file (I'm using a separate binary to remove the msys /bin from path, long story here, some other errors) and somehow it works now (I already did all this same stuff yesterday but somehow ended up crashing shotcut at startup).

[dany123]

Quick update as it's been a long time since my last: I've switched to libxml2 for file read/parsing and I've also been updating the rest of the code into a way more organized format and should probably have a commit ready the next few days.

I got stuck for a while into a weird crash some random seconds after reading the file, turns out I was calling xmlCleanupParser() at the end of my parsing routine (copied from the libxml's example code) which is only supposed to be called after finishing all work with xml lib. But the fun random part was triggered by shotcut's autosave feature (some time after my file read) which obviously needed the xml parser initialized and crashed the entire program when running. Not sure if this is a known/useful info, just writing it out here. I found out about it in the shotcut.RPT crash dump and not sure who's creating it (QT creator?) but it's a really good feature.

[dany123]

Hm, not exactly what I wanted to do here, I'll try to figure out how to fix it.
I probably shoudn't have merged.

Actually, can I just delete this branch and create a new one with just the correct files for the new filter?

[dany123]

Ok, so I fixed everything in the latest commit, I don't understand how squashing made extra commits instead of hiding them, github is not as intuitive as I hoped.
Anyway, the Files changed tab shows the latest version of my changes now so that's great, altough my offer to redo everything in a new branch still stands, the commit diffs are useless anyway as the files have now completely changed (well, except the .yml one).

Now onto the changes:
-fully switched to libxml for file parsing
-split gps read & parse into a separate file: gps_parser.c (with a gps_parser.h)
-reorganized the main filter file:
--file read+parse is now an independent call (and other functions check if data is available)
--filter property stuff is all grouped into a single function process_filter_properties() which has 3 clear groups: reading, processing, writing
--switched all times to miliseconds
---this forced me to add a new filter property updates_per_second otherwise the video text would update multiple times per second (if gps frequency is that high) and that looks horrible
--split the gps struct into raw and processed
-decided to clean up the available keywords a bit, instead of raw and processed it's now only the base keyword + optional RAW keyword
-also an optional keyword for the time-related keywords there's a +/-seconds offset

What I didn't do:
-forgot to add updates_per_second property to .yml
-forgot to remove the minor_offset property //edit: actually I remember why I added this one: if I declare something property int x; and set it to some value, after minimising/restoring shotcut (in windows) it will lose it's value so if I have to store anything reliably I have to use a filter.set("id"), and at this point might as well just use it directly in the backend

I'll update it and create a parallel pull request for the UI

I have one question about printing int64_t type: in a printf() I can use the format "%I64d" but in mlt_log_info this is not recognised as a proper format and literally prints "%I64d" instead of the variable, does it work for you or are you using something else?

[bmatherly]

Fantastic work on this. My only open question is about the major and minor times. But I will make that discussion in the Shotcut review.

Also, you will need to rebase properly. git pull --rebase origin master and then a force push should do it.

[dany123]

Also, you will need to rebase properly. git pull --rebase origin master and then a force push should do it.

Just did it, hope it's correct now.

[dany123]

One thing that's annoying me is getting the actual media start recording date on some files because I get the actual create date + video duration.
Does anyone know if there's an actual standard that defines this field? I tried googleing but can't find a lot. Right now I'm assuming it's at the video start moment but testing with some other random videos this assumption fails.

It seems like every device has it's own rule about this, from what I've tested only the gopro seems to set the time to start of recording, my phone is setting it to video end, my sony camera (in AVCHD format) has a different date field (which is not read by shotcut, I assume it falls back to file modification time which is set at the end of the recording). Here's a sample from this camera:
Mediainfo:
Recorded date : 2021-06-22 23:37:31+03:00
Exiftool:
Date/Time Original : 2021:06:22 23:37:31+03:00 DST
Windows:
File created: ‎22 ‎June ‎2021, ‏‎23:37:31
File modified: ‎22 ‎June ‎2021, ‏‎23:54:50
(file length is 17m14s)

Looking at mlt_producer_get_creation_time(), it seems to read the properties by passing a string (like "creation_time") to mlt_properties_get(). How can I figure out the string for the above field? I'm thinking this could be worth adding to the mlt_producer_get_creation_time() implementation.

The alternative would be to assume creation time is always start time + video duration, but of course this would break the files that don't do this.

Another alternative would be to add another button that adds video length to the offset (so many buttons...)
Or just let the user figure it out using the status text in the filter ui.

Thoughts?

[bmatherly]

Does anyone know if there's an actual standard that defines this field?

I am fairly confident there is not a universal standard to define a date for the start of all media file types. SMPTE time code exists for time (but not date). But many formats do not support SMPTE time codes.

Looking at mlt_producer_get_creation_time(), it seems to read the properties by passing a string (like "creation_time") to mlt_properties_get(). How can I figure out the string for the above field? I'm thinking this could be worth adding to the mlt_producer_get_creation_time() implementation.

The "creation_time" property is a way for an application to override the "discovered" time code. You can see in the function that if "creation_time" is not set by the application, then the code searches for properties set by ffmpeg: "meta.attr.com.apple.quicktime.creationdate" and "meta.attr.creation_time". If those are not set, then it falls back to using the file properties. I think you are right that some devices might uses the clip start time while some might use the end time. To top it off, some use UTC and some us local time. You are welcome to research ways to improve this code.

[dany123]

Annoying that there's no standard for this. From a logical standpoint, would "creation_time" imply start of recording? Technically it can also mean when the file was fully created so I don't even know if this is a path I should try to work out.

The "creation_time" property is a way for an application to override the "discovered" time code. You can see in the function that if "creation_time" is not set by the application, then the code searches for properties set by ffmpeg: "meta.attr.com.apple.quicktime.creationdate" and "meta.attr.creation_time".

Indeed, but how were those discovered? A ffprobe on the file shows no metadata at all, MediaInfo and exiftool show the datetime but under different names which I assume means it isn't a standard field.

I think the only sane solution for this is to add a new button in the UI. Can I access the total length (seconds) of the current video straight from qml? I'd rather not add another filter property in the backend to expose this.

[bmatherly]

Indeed, but how were those discovered?

I don't completely understand the question. The "creation_time" producer property can be set in Shotcut in the "hamburger" menu in the properties panel.

The frame properties come from ffmpeg. You can see the millions of places that the "creation_time" metadata is set in ffmpeg:
https://github.com/FFmpeg/FFmpeg/search?q=%22creation_time%22
"creation_time" is a name that ffmpeg has chosen to store the data that is parses from various formats. I suppose each format has some different name for it in the syntax definition for that format.

I think the only sane solution for this is to add a new button in the UI.

It is not clear to me what the button would do, but we can probably find a better way.

If we convince ourselves that MOST files set the creation time to the end of the clip, then we can apply the offset in the producer function. It would be good for that change to be in a different pull request.

[dany123]

I don't completely understand the question. The "creation_time" producer property can be set in Shotcut in the "hamburger" menu in the properties panel.

What I mean is: how were the "meta.attr.com.apple.quicktime.creationdate" and "meta.attr.creation_time" alternative strings found? Basically how can I search if there's some sort of "meta.attr[.mts].datetimeoriginal" that corresponds to the data found by exiftool in this .mts file from a sony camera.

If we convince ourselves that MOST files set the creation time to the end of the clip, then we can apply the offset in the producer function. It would be good for that change to be in a different pull request.

Indeed, but there's one more variable here that would skew the priority: I think most footage that would make sense to be used with GPS stats would come from an action cam, so even if there are millions of android phones that set it at the end of recording, I'd still go with the begining as Gopro s (and one other action cam I have and tested that saves .MOV (quicktime)) set it at the start. Also when I sync 2 things it is very natural to go for the start times. so bonus points for easy of use here.

It is not clear to me what the button would do, but we can probably find a better way.

If you try to sync a gps track with a file that has a creation time at the end of the video, it will be out of sync by it's length.
As an example: say a 1minute video is recorded starting at 18:00:00. Creation time would be read as 18:01:00, but backend assumes creation time is at begining so all GPS related text will be late by 1 min (this is because GPS time is always set by the satellites, not by the device itself).
In order to correct this the user will have to manually add this length (1min) to time_offset, which is not a big deal if he knows this is an issue. I'm thinking a button would both raise awareness about this and also make it one-click instead of checking file properties and adding it manually. There's already a button that does exactly this but for the timezone. I think adding a similar one at the bottom, near the video file start status text could work.

[bmatherly]

What I mean is: how were the "meta.attr.com.apple.quicktime.creationdate" and "meta.attr.creation_time" alternative strings found? Basically how can I search if there's some sort of "meta.attr[.mts].datetimeoriginal" that corresponds to the data found by exiftool in this .mts file from a sony camera.

The avformat producer maps metadata from the ffmpeg context into the meta.attr properties:
https://github.com/mltframework/mlt/blob/master/src/modules/avformat/producer_avformat.c#L513

So any metadata provided by ffmpeg should be available there.

More info here:
https://stackoverflow.com/questions/9464617/retrieving-and-saving-media-metadata-using-ffmpeg
https://ffmpeg.org/ffmpeg-all.html#Metadata-1

Shotcut shows the container and stream metadata in the Metadata tab on the properties panel:

[dany123]

Went through the links and I think I got the answer: the AVCHD (and mts container) is bad at storing metadata and the creation_time I see in mediainfo is not seen/supported by ffmpeg but extracted from deeper. This is also confirmed by this comment on the exiftool website:

AVCHD (.M2TS) (added 2009-10-29)

This format is used in .M2TS video files (and Blu-Ray HD DVD's). It is painfully obvious that this was never designed as a storage format. It uses MPEG-2 transport stream (M2TS) container, which is a communications protocol and never should have been used for storage. The M2TS format is based on a 188-byte packet size which makes no sense for modern filesystems. And to make things worse, metadata in AVCHD files is stored in the H.264 video stream (!!) which uses insane and convoluted data structures such as the variable-bit-length exponential-Golomb which are painful and inefficient to parse in software, and as far as I can tell the format for the metadata stored in these streams is proprietary and undocumented.

(one cool thing about this stream-store mode is that it has complex camera info for every second so f-number, exposure time, focus distance, date+time etc for every second)

This is also confirmed by the fact that the Metadata tab in properties in Shotcut is completely empty for this file.
Also if I use the camera's software to export the video to mp4 it adds the correct creation_time (even at the begining of recording).

So I guess this settles it, .MTS metadata is not supported, it will fall back to file modification time.

[dany123]

All the tests I could came up with are ok with the latest commit so I think this is the final version.

There are 3, let's call them "known issues" that I don't plan to fix (don't think they need) unless there's a good argument against:

• I arbitrarily chose 10s of no gps data as a "cut off" limit to show the last gps position and other stuff, anything more than 10s will show as "--". 10 seconds should be enough time if there's a short gps lost event due to going under a bridge or something common, and also not too much time to show wrong info if entering a tunnel/underground for long periods of time. I guess an extreme case like gps track frequency of once per minute would look pretty bad, but in practice there's no reason for anyone to use such a file for a video overlay. Pretty much all gps trackers use 1s interval (very rarely 3s to save a few MB space for a full day track). [An alternative would be to allow the user to define the max cut-off time or just to remove this limit. Both changes would be easy to implement but also, not really needed. ]
• add some rounding to altitude math: my watch records altitude with 1 digit precision so no issue here, but my phone has 14 digits of precision (of course, just random values after a point), which basically means that the track is never flat to increase flat_distance, only uphill/downhill (and total distance, which is the important one and is unaffected). Not sure who cares about this particular feature. Also not sure what the better algorithm would be for this.
• there's no support for tracks going through the 180* meridian. I mean.. at all!! The smoothing, processing, interpolation, everything would look wrong at that passing -179 to +179 point. I honestly don't know how to fix this... I could probably try some iterative/very inefficient stuff but it would just be bad and very likely still wrong. I think the best I can do is add a check in every step to completely ignore those points (I mean no math/smoothing/processing at all on them) and just get over it.

[bmatherly]

Looks good to me. I have no further comments.