From 48cd6acfe4b6892d4ffa20e4fdfe16057a19a142 Mon Sep 17 00:00:00 2001
From: Akanksha SIngh <akankshasingh.viet@gmail.com>
Date: Wed, 12 Oct 2022 23:33:51 +0530
Subject: [PATCH] Allow homarus to use faststart for video conversion (#162)

* Allow homarus to use faststart for video conversion

* Allow homarus to use faststart for video conversion

* Allow homarus to use faststart for video conversion

* Create mp4 fixture

* Refactor controller and surrounding tests

* Codesniffer fix

* Add -y flag to allow file overwrite

Co-authored-by: Akanksha Singh <akanksha@Akankshas-MacBook-Pro-2.local>
Co-authored-by: Chris MacDonald <31731869+chrismacdonaldw@users.noreply.github.com>
Co-authored-by: Akanksha Singh <akanksha@ip-192-168-29-132.ec2.internal>
---
 Homarus/src/Controller/HomarusController.php  | 35 +++++++++++++------
 .../Homarus/Tests/HomarusControllerTest.php   | 31 +++++++++++-----
 Homarus/tests/fixtures/foo.mp4                |  0
 3 files changed, 46 insertions(+), 20 deletions(-)
 create mode 100644 Homarus/tests/fixtures/foo.mp4

diff --git a/Homarus/src/Controller/HomarusController.php b/Homarus/src/Controller/HomarusController.php
index 2d6e4a1b..0d432fd1 100644
--- a/Homarus/src/Controller/HomarusController.php
+++ b/Homarus/src/Controller/HomarusController.php
@@ -1,14 +1,11 @@
 <?php
 namespace Islandora\Homarus\Controller;
 
-use GuzzleHttp\Psr7\StreamWrapper;
 use Islandora\Crayfish\Commons\CmdExecuteService;
 use Islandora\Crayfish\Commons\ApixFedoraResourceRetriever;
-use Psr\Http\Message\ResponseInterface;
 use Symfony\Component\HttpFoundation\BinaryFileResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
-use Symfony\Component\HttpFoundation\StreamedResponse;
 
 /**
  * Class HomarusController
@@ -92,7 +89,7 @@ public function __construct(
 
   /**
    * @param \Symfony\Component\HttpFoundation\Request $request
-   * @return \Symfony\Component\HttpFoundation\Response|\Symfony\Component\HttpFoundation\StreamedResponse
+   * @return \Symfony\Component\HttpFoundation\Response|\Symfony\Component\HttpFoundation\BinaryFileResponse
    */
     public function convert(Request $request)
     {
@@ -116,10 +113,13 @@ public function convert(Request $request)
         $cmd_params = "";
         if ($format == "mp4") {
             $cmd_params = " -vcodec libx264 -preset medium -acodec aac " .
-                "-strict -2 -ab 128k -ac 2 -async 1 -movflags " .
-                "frag_keyframe+empty_moov ";
+                "-strict -2 -ab 128k -ac 2 -async 1 -y -movflags " .
+                "faststart ";
         }
 
+        $temp_file_path = __DIR__ . "/../../static/" . basename($source) . "." . $format;
+        $this->log->debug('Tempfile: ' . $temp_file_path);
+
         // Arguments to ffmpeg command are sent as a custom header.
         $args = $request->headers->get('X-Islandora-Args');
 
@@ -133,22 +133,35 @@ public function convert(Request $request)
                 400
             );
         }
-      
+
         // Add -loglevel error so large files can be processed.
         $args .= ' -loglevel error';
         $this->log->debug("X-Islandora-Args:", ['args' => $args]);
         $token = $request->headers->get('Authorization');
         $headers = "'Authorization:  $token'";
-        $cmd_string = "$this->executable -headers $headers -i $source  $args $cmd_params -f $format -";
+        $cmd_string = "$this->executable -headers $headers -i $source  $args $cmd_params -f $format $temp_file_path";
         $this->log->debug('Ffmpeg Command:', ['cmd' => $cmd_string]);
 
         // Return response.
+        return $this->generateDerivativeResponse($cmd_string, $source, $temp_file_path, $content_type);
+    }
+
+    /**
+     * @param string $cmd_string
+     * @param string $source
+     * @param string $path
+     * @param string $content_type
+     * @return \Symfony\Component\HttpFoundation\Response|\Symfony\Component\HttpFoundation\BinaryFileResponse
+     */
+    public function generateDerivativeResponse(string $cmd_string, string $source, string $path, string $content_type)
+    {
         try {
-            return new StreamedResponse(
-                $this->cmd->execute($cmd_string, $source),
+            $this->cmd->execute($cmd_string, $source);
+            return (new BinaryFileResponse(
+                $path,
                 200,
                 ['Content-Type' => $content_type]
-            );
+            ))->deleteFileAfterSend(true);
         } catch (\RuntimeException $e) {
             $this->log->error("RuntimeException:", ['exception' => $e]);
             return new Response($e->getMessage(), 500);
diff --git a/Homarus/tests/Islandora/Homarus/Tests/HomarusControllerTest.php b/Homarus/tests/Islandora/Homarus/Tests/HomarusControllerTest.php
index 5b5a26fe..55798db9 100644
--- a/Homarus/tests/Islandora/Homarus/Tests/HomarusControllerTest.php
+++ b/Homarus/tests/Islandora/Homarus/Tests/HomarusControllerTest.php
@@ -10,6 +10,7 @@
 use Prophecy\Argument;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\StreamInterface;
+use Symfony\Component\HttpFoundation\BinaryFileResponse;
 use Symfony\Component\HttpFoundation\Request;
 
 /**
@@ -219,15 +220,27 @@ private function getDefaultController()
         $mock_service = $prophecy->reveal();
 
         // Create a controller.
-        $controller = new HomarusController(
-            $mock_service,
-            $this->content_types,
-            $this->default_content_type,
-            'convert',
-            $this->prophesize(Logger::class)->reveal(),
-            $this->mime_to_format,
-            $this->default_format
-        );
+        $controller = $this->getMockBuilder(HomarusController::class)
+            ->onlyMethods(['generateDerivativeResponse'])
+            ->setConstructorArgs([
+                $mock_service,
+                $this->content_types,
+                $this->default_content_type,
+                'convert',
+                $this->prophesize(Logger::class)->reveal(),
+                $this->mime_to_format,
+                $this->default_format,
+            ])
+            ->getMock();
+
+        $controller->method('generateDerivativeResponse')
+            ->will($this->returnCallback(function ($cmd_string, $source, $path, $content_type) {
+                return new BinaryFileResponse(
+                    __DIR__ . "/../../../fixtures/foo.mp4",
+                    200,
+                    ['Content-Type' => $content_type]
+                );
+            }));
         return $controller;
     }
 
diff --git a/Homarus/tests/fixtures/foo.mp4 b/Homarus/tests/fixtures/foo.mp4
new file mode 100644
index 00000000..e69de29b