3x: Allow homarus to use faststart

[bibliophileaxe]

Rebase with 3.x
Original Issue: Islandora/documentation#2162

[seth-shaw-asu]

I spun up the new islandora-playbook is Crayfish 3.x and then applied this PR. The video derivative didn't work and the logs show:

[2022-10-17 12:49:51] request.INFO: Matched route "convert_get". {"route":"convert_get","route_parameters":{"_route":"convert_get","_controller":"App\\Islandora\\Homarus\\Controller\\HomarusController::convert"},"request_uri":"http://127.0.0.1:8000/homarus/convert","method":"GET"} []
...skipping authentication stuff...
[2022-10-17 12:49:52] app.INFO: Ffmpeg Convert request. [] []
[2022-10-17 12:49:52] app.DEBUG: Tempfile: /var/www/html/Crayfish/Homarus/src/Controller/../../static/file_example_mov_1920_2_2mb.mov.mp4 [] []
[2022-10-17 12:49:52] app.DEBUG: X-Islandora-Args: {"args":" -loglevel error"} []
[2022-10-17 12:49:52] app.DEBUG: Ffmpeg Command: {"cmd":"ffmpeg -headers 'Authorization:  Bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJpYXQiOjE2NjYwMjg5ODksImV4cCI6MTY2NjAzNjE4OSwid2ViaWQiOiIxIiwiaXNzIjoiaHR0cHM6XC9cL2xvY2FsaG9zdDo4MDAwIiwic3ViIjoiYWRtaW4iLCJyb2xlcyI6WyJhdXRoZW50aWNhdGVkIiwiZmVkb3JhYWRtaW4iXSwiYXVkIjpbImlzbGFuZG9yYSJdfQ.racbxnihkudPadE8o0T_7coId-yOmAgVN_w0QmO3OOBrmyEghz4QUxww5NzFdiAwaNbecBzQpcgPYamsyPmazWYeN9RlAOmGqptVzUIO7Rfytc04lzrmg6BgBm8YEYYeiz2GCtVJMTs0Wp4t5mOUzJLJP7zdHBDk2qwfpb_0I-QVaVuNN_czfU00vyHD5I2KXXsgc9tKCnSRdIZXMLCGilvuUTMFQZXZF1NFhsX5GzeIxU7m_3Juq1tmxpvb140p8TANj_B0sp0MB9Lbrv_wgGIKfyZh4MsWndW1pZcHfmHpxImgIHUcB4wpn2hMoCw7C6cxK8usMkDZm8TTZ6enWQ' -i http://localhost:8000/_flysystem/fedora/2022-10/file_example_mov_1920_2_2mb.mov   -loglevel error  -vcodec libx264 -preset medium -acodec aac -strict -2 -ab 128k -ac 2 -async 1 -movflags faststart -y -f mp4 /var/www/html/Crayfish/Homarus/src/Controller/../../static/file_example_mov_1920_2_2mb.mov.mp4"} []
[2022-10-17 12:49:54] app.ERROR: Process exited with non-zero code. {"exit_code":1,"stderr":"/var/www/html/Crayfish/Homarus/src/Controller/../../static/file_example_mov_1920_2_2mb.mov.mp4: No such file or directory\n"} []
[2022-10-17 12:49:54] app.ERROR: RuntimeException: {"exception":"[object] (RuntimeException(code: 500): /var/www/html/Crayfish/Homarus/src/Controller/../../static/file_example_mov_1920_2_2mb.mov.mp4: No such file or directory\n at /var/www/html/Crayfish/Homarus/vendor/islandora/crayfish-commons/CmdExecuteService.php:120)"} []

[seth-shaw-asu]

Shouldn't we be using a temp directory instead? Or, even better, something configurable?

[codecov]

Codecov Report

Base: 76.67% // Head: 75.55% // Decreases project coverage by -1.12% ⚠️

Coverage data is based on head (2b2e18b) compared to base (aed6511).
Patch has no changes to coverable lines.

Additional details and impacted files

@@             Coverage Diff              @@
##                3.x     #170      +/-   ##
============================================
- Coverage     76.67%   75.55%   -1.13%     
- Complexity      158      159       +1     
============================================
  Files             6        6              
  Lines           583      589       +6     
============================================
- Hits            447      445       -2     
- Misses          136      144       +8     

Impacted Files	Coverage Δ
...d_dir/Homarus/src/Controller/HomarusController.php	84.31% <0.00%> (-15.69%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[seth-shaw-asu]

So, question for the @Islandora/committers, am I being paranoid with my discomfort that we are dropping these processing files in a directory that it publicly accessible to anyone who can reach the Crayfish webserver?

I mean, sure, Isle doesn't expose the Crayfish containers to the public web, but that doesn't mean others won't, and someone can keep pinging crayfish.example:8000/homarus/static/ to see what files are in process, even if they will be removed once they get shipped back to Drupal. Normally our derivatives will be publicly available anyway, but some will be subject to restrictions and that just feels like a security gap.

Let me know if I'm being overly paranoid and I'll smash the green button.

[jordandukart]

Honestly, probably worth bringing up on the tech call.

[seth-shaw-asu]

Any reason why we shouldn't use a system temp directory instead of a public one? We are sending the binary back in the response, not a download link...

[adam-vessey]

Ideally, I think, we would be using unnamed/anonymous files similar to what tmpfile() returns (that the OS would garbage collect/mark free after it detects that there are no references to it remaining in the system, thereby avoiding the possibility of files being left around if the PHP process crashes before deleting the file); however, I think the ideal implementation would require reworking the command executor in order to pass in the file resource instead of having stdout of the command go to a pipe (https://github.com/Islandora/Crayfish-Commons/blob/01e65d6182601834a94283aeacd8ef1ec10dc4e0/src/CmdExecuteService.php#L63-L66)... (really, would it make sense for it to deal with streams more explicitly? Seems like it already buffers things in one, could be just something like: Islandora/Crayfish-Commons@3.x...adam-vessey:Crayfish-Commons:feature/more-streaming).

... though that's not to say that having the target file system used as configuration could not be useful, and may be something of a moot point if we do indeed need to pass a file name to ffmpeg (which... may not be 100% necessary? For example, running things on CLI, redirecting things to/from files can be handled more or less transparently, behind the scenes managing to deal with the files directly... whether or not things via proc_open() would work the same way would have to be tested out)...

[bibliophileaxe]

I think making the directory configurable makes sense. That way, people can decide if they need it to be a public or a private directory and handle it themselves.

[seth-shaw-asu]

Is this ready to test again, @bibliophileaxe?

[bibliophileaxe]

This is ready for review now.

[whikloj]

👍 It works, but I don't have a fragmented MP4 file to test with.