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

Improve serialization performance by creating a simple binary format for transform files. #97

Merged
merged 4 commits into from
Nov 6, 2020

Conversation

gabilan
Copy link
Contributor

@gabilan gabilan commented Nov 5, 2020

Improve serialization performance by creating a simple binary format for transform files. Binary serialization results in faster load/store times and files that are 40%+ smaller in size. Use of this binary serialization is controlled by using the argument "fileformat".

Also change Field and Vec to represent coordinate and size fields as 16-bit integers. This reduces the runtime memory usage of these data structures by ~50%.

@georgmartius
Copy link
Owner

Wonderful!
This is really a welcome feature. However, I would prefer if both formats are supported at runtime, in particular, to be able to load old files. Do you think that would be too complicated? Maybe just the way you have it now, but the load module would be able to check if it is the ascii version?
Also, we should make this the default and make a minor version bump.

@mviereck
Copy link

mviereck commented Nov 5, 2020

Does this also affect global_motions.trf that is created with option debug?
I ask because I am parsing global_motions.trf to apply the changes myself with imagemagick.
(I would prefer no change in global_motions.trf).

@georgmartius
Copy link
Owner

Does this also affect global_motions.trf that is created with option debug?
Would be good to keep this untouched, you are right.

@gabilan
Copy link
Contributor Author

gabilan commented Nov 5, 2020

Does this also affect global_motions.trf that is created with option debug?
I ask because I am parsing global_motions.trf to apply the changes myself with imagemagick.
(I would prefer no change in global_motions.trf).

As far as I can tell, global_motions.trf is not impacted by my change. It's populated in vsLocalmotions2Transforms/vsMotionsToTransform and doesn't appear to use any of the serialization methods.

@gabilan
Copy link
Contributor Author

gabilan commented Nov 5, 2020

Wonderful!
This is really a welcome feature. However, I would prefer if both formats are supported at runtime, in particular, to be able to load old files. Do you think that would be too complicated? Maybe just the way you have it now, but the load module would be able to check if it is the ascii version?
Also, we should make this the default and make a minor version bump.

I've implemented your feedback.

@georgmartius
Copy link
Owner

Dear Ernest,
looks good. Quite a bit of boilerplate code though. Is there no standard lib for all this big-endian to little endian conversion and binary serialization?

Repository owner deleted a comment from martius-lab Nov 6, 2020
@gabilan
Copy link
Contributor Author

gabilan commented Nov 6, 2020

There are a few that come to mind -- FlatBuffers, ProtoBuf, and Cereal. Cereal is probably the easiest to integrate because it's just a header. The other immediate alternatives require defining a schema and then generating code. I don't mind implementing Cereal if you are OK with mixing C and C++11. I avoided adding dependencies in order to keep this PR simple. I'd look to you for a strong suggestion in this area as this is your project.

@georgmartius
Copy link
Owner

You are right, let's keep it simple and do not create more dependencies than necessary.

@georgmartius georgmartius merged commit d847ad0 into georgmartius:master Nov 6, 2020
@gabilan gabilan deleted the binary_file_format branch November 7, 2020 02:01
@georgmartius
Copy link
Owner

Apparently, it causes problems on windows, see issue 104
#104
Do you have access to a windows machine to give it a try?

@sufehmi
Copy link

sufehmi commented Oct 30, 2023

Apparently, it causes problems on windows, see issue 104 #104 Do you have access to a windows machine to give it a try?

Please change the default to the ASCII transform files - ffmpeg v6.0.1 on Mac M2 can't stabilize video as well because of this.

Second pass always output error "Cannot parse localmotion"

ffmpeg version 6.0 Copyright (c) 2000-2023 the FFmpeg developers built with Apple clang version 14.0.3 (clang-1403.0.22.14.1) configuration: --prefix=/opt/homebrew/Cellar/ffmpeg/6.0_1 --enable-shared --enable-pthreads --enable-version3 --cc=clang --host-cflags= --host-ldflags= --enable-ffplay --enable-gnutls --enable-gpl --enable-libaom --enable-libaribb24 --enable-libbluray --enable-libdav1d --enable-libjxl --enable-libmp3lame --enable-libopus --enable-librav1e --enable-librist --enable-librubberband --enable-libsnappy --enable-libsrt --enable-libsvtav1 --enable-libtesseract --enable-libtheora --enable-libvidstab --enable-libvmaf --enable-libvorbis --enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxml2 --enable-libxvid --enable-lzma --enable-libfontconfig --enable-libfreetype --enable-frei0r --enable-libass --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libopenjpeg --enable-libspeex --enable-libsoxr --enable-libzmq --enable-libzimg --disable-libjack --disable-indev=jack --enable-videotoolbox --enable-audiotoolbox --enable-neon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants