From 4dca22a28fc210f9462f7d7b55967d5d91a05553 Mon Sep 17 00:00:00 2001 From: Akanksha SIngh Date: Wed, 30 Nov 2022 22:32:13 +0400 Subject: [PATCH] 3x: Allow homarus to use faststart (#170) * Update HomarusController and surrounding tests * Cherry-pick fastart code to 3.x * Fix fixture file path * Make temdirectory configurable --- Homarus/config/services.yaml | 2 + Homarus/src/Controller/HomarusController.php | 67 +++++++++++++++----- Homarus/tests/HomarusControllerTest.php | 23 ++++++- Homarus/tests/fixtures/foo.mp4 | 0 4 files changed, 73 insertions(+), 19 deletions(-) create mode 100644 Homarus/tests/fixtures/foo.mp4 diff --git a/Homarus/config/services.yaml b/Homarus/config/services.yaml index b8d564ba..a9bb0ffe 100644 --- a/Homarus/config/services.yaml +++ b/Homarus/config/services.yaml @@ -25,6 +25,7 @@ parameters: app.formats.defaults: mimetype: video/mp4 format: mp4 + app.tempDirectory: /tmp/ services: # default configuration for services in *this* file @@ -50,4 +51,5 @@ services: $formats: '%app.formats.valid%' $defaults: '%app.formats.defaults%' $executable: '%app.executable%' + $tempDirectory: '%app.tempDirectory%' tags: ['controller.service_arguments'] diff --git a/Homarus/src/Controller/HomarusController.php b/Homarus/src/Controller/HomarusController.php index 267e04d1..3e307edb 100644 --- a/Homarus/src/Controller/HomarusController.php +++ b/Homarus/src/Controller/HomarusController.php @@ -7,11 +7,12 @@ use Symfony\Component\HttpFoundation\BinaryFileResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\HttpFoundation\StreamedResponse; /** * Class HomarusController + * * @param $log + * * @package Islandora\Homarus\Controller */ class HomarusController @@ -48,6 +49,13 @@ class HomarusController */ private $executable; + /** + * The temp directory. + * + * @var string + */ + private $tempDirectory; + /** * Controller constructor. * @@ -59,6 +67,8 @@ class HomarusController * The default mimetype and format. * @param string $executable * The path to the programs executable. + * @param string $tempDirectory + * The temporary directory path. * @param \Psr\Log\LoggerInterface $log * The logger. */ @@ -67,18 +77,21 @@ public function __construct( array $formats, array $defaults, string $executable, + string $tempDirectory, LoggerInterface $log ) { $this->cmd = $cmd; $this->formats = $formats; $this->defaults = $defaults; $this->executable = $executable; + $this->tempDirectory = $tempDirectory; $this->log = $log; } /** * @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) { @@ -97,15 +110,18 @@ public function convert(Request $request) // Find the format $content_types = $request->getAcceptableContentTypes(); - list($content_type, $format) = $this->getFfmpegFormat($content_types); + [$content_type, $format] = $this->getFfmpegFormat($content_types); $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 "; + "faststart -y"; } + $temp_file_path = $this->tempDirectory . 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'); @@ -125,24 +141,16 @@ public function convert(Request $request) $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. - try { - return new StreamedResponse( - $this->cmd->execute($cmd_string, $source), - 200, - ['Content-Type' => $content_type] - ); - } catch (\RuntimeException $e) { - $this->log->error("RuntimeException:", ['exception' => $e]); - return new Response($e->getMessage(), 500); - } + return $this->generateDerivativeResponse($cmd_string, $source, $temp_file_path, $content_type); } /** - * Filters through an array of acceptable content-types and returns a FFmpeg format. + * Filters through an array of acceptable content-types and returns a + * FFmpeg format. * * @param array $content_types * The Accept content-types. @@ -167,6 +175,33 @@ private function getFfmpegFormat(array $content_types): array return [$this->defaults["mimetype"], $this->defaults["format"]]; } + /** + * @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 { + $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); + } + } + /** * @return \Symfony\Component\HttpFoundation\BinaryFileResponse */ diff --git a/Homarus/tests/HomarusControllerTest.php b/Homarus/tests/HomarusControllerTest.php index 374283ee..8d8fbd3e 100644 --- a/Homarus/tests/HomarusControllerTest.php +++ b/Homarus/tests/HomarusControllerTest.php @@ -10,6 +10,7 @@ use Prophecy\PhpUnit\ProphecyTrait; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\StreamInterface; +use Symfony\Component\HttpFoundation\BinaryFileResponse; use Symfony\Component\HttpFoundation\Request; /** @@ -39,6 +40,8 @@ public function setUp(): void [ 'mimetype' => 'video/x-msvideo', 'format' => 'avi'], [ 'mimetype' => 'video/ogg', 'format' => 'ogg'], ]; + + $this->tempDirectory = '/../public/static/'; } /** @@ -72,6 +75,7 @@ public function testErrorReturns500() $this->formats, $this->defaults, 'convert', + $this->tempDirectory, $this->prophesize(Logger::class)->reveal() ); @@ -210,13 +214,26 @@ private function getDefaultController() $mock_service = $prophecy->reveal(); // Create a controller. - $controller = new HomarusController( + $controller = $this->getMockBuilder(HomarusController::class) + ->onlyMethods(['generateDerivativeResponse']) + ->setConstructorArgs([ $mock_service, $this->formats, $this->defaults, 'convert', - $this->prophesize(Logger::class)->reveal() - ); + $this->tempDirectory, + $this->prophesize(Logger::class)->reveal(), + ]) + ->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