a-tze / fuse-ts

GNU General Public License v3.0
10 stars 7 forks source link

Support for Shotcut 22 #14

Closed MaZderMind closed 1 year ago

MaZderMind commented 2 years ago

This PR improves Support for Shotcut 22 by implementing the new ML-XML Format which uses <chain> instead of <producer>. Also Write-Access to the Tempfiles is rejected for both the Shotcut 19+ as well as the Shotcut 22 Tempfile Naming-Schema in order to prevent QtSaveFile from performing an atomic write, which fuse-ts does not support.

Fixes #13

MaZderMind commented 2 years ago

These changes work for Shotcut 22, however they seem to break support for Shotcut 19. Should I try to incorporate the old Schema of handling Shotcut 19 style Tempfiles or would you rather like to drop support for Shotcut 19 @a-tze ?

a-tze commented 2 years ago

@MaZderMind I did not dig into the past of the shotcut code, but my expectation was that the "direct write fallback" would also apply to older shotcut versions. I'd rather think that the XML content itself is not compatible between the versions?

MaZderMind commented 2 years ago

I tried to combine both paths, but I found Shotcut 19 Support with the Tempfile-Hack to be quite unstable on MacOS. Saving results in the App hanging and calling into Fuse over and over again. This is also the case with current master.

a-tze commented 2 years ago

I see.. Shotcut 19 had a very different method of writing the project file. so with unstable and hack you mean the previous versions of fuse-ts before adapting to Shotcut 22?

MaZderMind commented 2 years ago

Here is my effort to combine Shotcut 19 and 22 Support: https://github.com/a-tze/fuse-ts/compare/master...MaZderMind:fuse-ts:13-shotcut-project-file-shotcut-19+22

However, this is still broken for Shotcut 19 in the same was as the current master is. From my point of view, it would be nicer to ditch the broken Shotcut 19 Support and only support the most recent version.

MaZderMind commented 2 years ago

It seems there is still some sort of Problem with Shotcut 22: After Saving, the file is received by fuse-ts and re-opening the xml also yields the correct cutmarks, however inframe and outframe are not updated until the file is re-opened again.

a-tze commented 2 years ago

@MaZderMind Agree. The old versions of fuse-ts are still available, its no force to the users and there is no other change bundled to this. Additionally the Tempfile hack might be dropped for Kdenlive as well in a later version.

You know that I will likely squash the PR?

a-tze commented 2 years ago

It seems there is still some sort of Problem with Shotcut 22: After Saving, the file is received by fuse-ts and re-opening the xml also yields the correct cutmarks, however inframe and outframe are not updated until the file is re-opened again.

Must be a remain of the tempfile hack, where the cutmarks where extracted in the rename function.

MaZderMind commented 2 years ago

find_cutmarks_in_shotcut_project_file is called in ts_release which seems resonable.

I think it might be a timing problem: when saving and closing the project, i see

filebuffer__write:  writing 1204 bytes from 0x7f3b61976060 to 0x7f3b54002760 with offset 0
filebuffer__truncate:  truncating filebuffer 0x7f3b54002760 to size 1204

and only ~40s later I get the corresponding

release called on '/project_shotcut.mlt'

which then results in the cutmarks being updated. I guess that's how it always was on MacOS but in production use nobody noticed. I don't know if it would be save to parse the file in truncate or not.

I don't care if you'll sqush or not – commits are part of the delivery and should be nice and coherent, also to allow for better cherry-picking or review.

a-tze commented 2 years ago

I think that truncate is an internal truncate method, not the filesystem truncate syscall. So it would make sense, to try to read the cutmarks in ts_write (after truncating) as well, but ignore errors because it might have been a partial write of the file. In reality it's pretty unlikely that the project file is not written with one single call, because it is so small but there still is a chance. When release is called later, they can safely be evaluated again, as the outcome would be the same.

MaZderMind commented 2 years ago

There's another Problem when the production is setup with fps != 25 – the Shotcut-File does not mention the Framerate and Shotcut defaults to 25 fps, which when the file is opened again will shift the cutmarks. The fps is known so I would just add them back again to the profile (they were removed in 0587dd364dfbb9d4e3afadf6dba72ef73d97e76e)

MaZderMind commented 2 years ago

I'm happy now. Let me know if you want anything to be changed :)

MaZderMind commented 1 year ago

Anything holding this up?

a-tze commented 1 year ago

Sorry, I remembered "something still needs to be done".

So let me summarize: the tip of the PR branch supports Shotcut 21 and 22 and reintroduced fps?

MaZderMind commented 1 year ago

Shotcut 22 & 21 are fully supported, lower Versions will struggle with the Temp-File Problem but can still be used via Save-As-Local-File-and-copy-over Workaround. The fps have been reintroduced for Shotcut to generate the correct Timestamps on != 25 fps Projects (we produce with 30 fps and tested with that material).

MaZderMind commented 1 year ago

Ping :)

MaZderMind commented 1 year ago

FTR I triggered the Build-Job in the VOC Jenkins and now up-to-date Packets are available in https://pkg.c3voc.de/pool/main/f/fuse-ts/