Prep 0.8.4

soluble-io · Dec 17, 2018 · dcde3ed · dcde3ed
1 parent f1c7bfd
commit dcde3ed
Show file tree

Hide file tree

Showing 12 changed files with 105 additions and 49 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,22 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/) 
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+
+## 0.8.4 (2018-12-17)
+
+### Bugfix 
+
+- Recently introduced for `symfony/process 4.2` broke older versions.
+
+### Improvement
+
+- Now relies on arguments escaping offered by `symfony/process`.  
+
+### Improved
+
+- Q&A: travis now tests with lowest supported deps !  
+
+
 ## 0.8.3 (2018-12-03)
 
 ### Updated 

diff --git a/composer.json b/composer.json
@@ -31,13 +31,13 @@
         "fig/http-message-util": "^1.1.2",
         "friendsofphp/php-cs-fixer": "^2.13",
         "guzzlehttp/guzzle": "^6.2",
-        "infection/infection": "^0.11",
+        "infection/infection": "^0.11.2",
         "jangregor/phpstan-prophecy": "^0.2",
         "monolog/monolog": "^1.23",
         "phpspec/prophecy": "^1.8",
-        "phpstan/phpstan": "^0.10",
+        "phpstan/phpstan": "^0.10.5",
         "phpstan/phpstan-phpunit": "^0.10",
-        "phpstan/phpstan-strict-rules": "^0.10",
+        "phpstan/phpstan-strict-rules": "^0.10.1",
         "phpunit/phpunit": "^7.4",
         "psr/http-message": "^1.0.1",
         "roave/security-advisories": "dev-master",
@@ -62,7 +62,6 @@
         }
     },
     "scripts": {
-        "post-install-cmd": "\\CaptainHook\\CaptainHook\\Composer\\Cmd::install",
         "check": [
             "@cs-check",
             "@phpstan"

diff --git a/src/Common/Process/ProcessFactory.php b/src/Common/Process/ProcessFactory.php
@@ -15,10 +15,10 @@ class ProcessFactory
     protected $processParams;
 
     /**
-     * @param array|string                $command
+     * @param array                       $command
      * @param null|ProcessParamsInterface $processParams
      */
-    public function __construct($command, ?ProcessParamsInterface $processParams = null)
+    public function __construct(array $command, ?ProcessParamsInterface $processParams = null)
     {
         $this->command       = $command;
         $this->processParams = $processParams;
@@ -27,7 +27,9 @@ public function __construct($command, ?ProcessParamsInterface $processParams = n
     public function __invoke(): Process
     {
         if (!is_array($this->command)) {
-            $process = Process::fromShellCommandline($this->command);
+            var_dump($this->command);
+            die('cool');
+        //            $process = Process::fromShellCommandline($this->command);
         } else {
             $process = new Process($this->command);
         }

diff --git a/src/Video/Adapter/ConverterAdapterInterface.php b/src/Video/Adapter/ConverterAdapterInterface.php
@@ -25,9 +25,9 @@ public function getMappedConversionParams(VideoConvertParamsInterface $conversio
      * @param null|string|UnescapedFileInterface $outputFile
      * @param array<string,string>               $prependArguments args that must be added at the beginning of the command
      *
-     * @return string
+     * @return array<int, string>
      */
-    public function getCliCommand(array $arguments, ?string $inputFile, $outputFile = null, array $prependArguments = []): string;
+    public function getCliCommand(array $arguments, ?string $inputFile, $outputFile = null, array $prependArguments = []): array;
 
     public function getDefaultThreads(): ?int;
 }
diff --git a/src/Video/Adapter/FFMpegAdapter.php b/src/Video/Adapter/FFMpegAdapter.php
@@ -192,7 +192,7 @@ public function getMappedConversionParams(VideoConvertParamsInterface $conversio
      *
      * @throws InvalidArgumentException
      */
-    public function getCliCommand(array $arguments, ?string $inputFile, $outputFile = null, array $prependArguments = []): string
+    public function getCliCommand(array $arguments, ?string $inputFile, $outputFile = null, array $prependArguments = []): array
     {
         $inputArg = ($inputFile !== null && $inputFile !== '')
                         ? sprintf('-i %s', escapeshellarg($inputFile))
@@ -202,28 +202,26 @@ public function getCliCommand(array $arguments, ?string $inputFile, $outputFile
         if ($outputFile instanceof UnescapedFileInterface) {
             $outputArg = $outputFile->getFile();
         } elseif (is_string($outputFile)) {
-            $outputArg = sprintf('%s', escapeshellarg($outputFile));
+            //$outputArg = sprintf('%s', escapeshellarg($outputFile));
+            $outputArg = $outputFile;
         } elseif ($outputFile !== null) {
             throw new InvalidArgumentException(sprintf(
                 'Output file must be either a non empty string, null or PlatformNullFile (type %s)',
                 gettype($outputFile)
             ));
         }
 
-        $ffmpegCmd = preg_replace(
-            '/(\ ){2,}/',
-            ' ',
-            trim(sprintf(
-                '%s %s %s %s %s',
+        $ffmpegCmd = array_merge(
+            [
                 $this->ffmpegConfig->getBinary(),
-                implode(' ', $prependArguments),
-                $inputArg,
-                implode(' ', $arguments),
-                $outputArg
-            ))
+            ],
+            $this->getArgsWithExplodedValues($prependArguments),
+            ['-i', $inputFile],
+            $this->getArgsWithExplodedValues($arguments),
+            [$outputArg]
         );
 
-        if ($ffmpegCmd === null) {
+        if (count($ffmpegCmd) < 2) {
             throw new UnexpectedValueException(
                 'Cannot generate ffmpeg cli command'
             );
@@ -232,6 +230,30 @@ public function getCliCommand(array $arguments, ?string $inputFile, $outputFile
         return $ffmpegCmd;
     }
 
+    /**
+     * As we rely on symfony process unescaping, we
+     * need to explode options name and values... i.e
+     * ['-tune animation'] will become ['-tune', 'animation'].
+     *
+     * @param array<string, string> $args
+     *
+     * @return string[]
+     */
+    private function getArgsWithExplodedValues(array $args): array
+    {
+        $exploded = [];
+        foreach ($args as $key => $value) {
+            $elems      = explode(' ', $value);
+            $exploded[] = (string) array_shift($elems);
+            if (count($elems) <= 0) {
+                continue;
+            }
+            $exploded[] = implode(' ', $elems);
+        }
+
+        return $exploded;
+    }
+
     public function getDefaultThreads(): ?int
     {
         return $this->ffmpegConfig->getThreads();

diff --git a/src/Video/Filter/SelectFilter.php b/src/Video/Filter/SelectFilter.php
@@ -8,9 +8,7 @@
 
 class SelectFilter implements FFMpegVideoFilterInterface
 {
-    /**
-     * @var null|string
-     */
+    /** @var null|string */
     private $expression;
 
     /**
@@ -21,15 +19,15 @@ class SelectFilter implements FFMpegVideoFilterInterface
      * @param string|null $expression ffmpeg selected expression
      */
     public function __construct(
-        string $expression = null
+        ?string $expression = null
     ) {
         $this->expression = $expression;
     }
 
     public function getFFmpegCLIValue(): string
     {
         return sprintf(
-            '"select=%s"',
+            'select=%s',
             str_replace('"', '\"', $this->expression ?? '')
         );
     }

diff --git a/src/Video/VideoInfoReader.php b/src/Video/VideoInfoReader.php
@@ -46,6 +46,7 @@ public function __construct(FFProbeConfigInterface $ffProbeConfig, ?LoggerInterf
      */
     public function getSymfonyProcess(string $inputFile, ?ProcessParamsInterface $processParams = null): Process
     {
+        /*
         $ffprobeCmd = trim(sprintf(
             '%s %s %s',
             $this->ffprobeConfig->getBinary(),
@@ -57,6 +58,19 @@ public function getSymfonyProcess(string $inputFile, ?ProcessParamsInterface $pr
             ]),
             sprintf('-i %s', escapeshellarg($inputFile))
         ));
+        */
+
+        $ffprobeCmd = [
+            $this->ffprobeConfig->getBinary(),
+            '-v',
+            'quiet',
+            '-print_format',
+            'json',
+            '-show_format',
+            '-show_streams',
+            '-i',
+            $inputFile,
+        ];
 
         $pp = $processParams ?? $this->ffprobeConfig->getProcessParams();
 

diff --git a/tests/functional/UseCases/VideoThumbGeneratorTest.php b/tests/functional/UseCases/VideoThumbGeneratorTest.php
@@ -26,9 +26,7 @@ class VideoThumbGeneratorTest extends TestCase
     /** @var VideoThumbGeneratorInterface */
     protected $thumbService;
 
-    /**
-     * @var VideoInfoReaderInterface
-     */
+    /** @var VideoInfoReaderInterface */
     protected $infoService;
 
     /** @var string */

diff --git a/tests/unit/Video/Adapter/FFMpegAdapterTest.php b/tests/unit/Video/Adapter/FFMpegAdapterTest.php
@@ -176,7 +176,7 @@ public function testGetCliCommand(): void
             '/test/output.mp4'
         );
 
-        self::assertContains('ffmpeg -i \'/test/video.mp4\' -crf 32 -c:v h264 -y \'/test/output.mp4\'', $cmd);
+        self::assertContains('ffmpeg -i /test/video.mp4 -crf 32 -c:v h264 -y /test/output.mp4', implode(' ', $cmd));
     }
 
     public function testGetCliCommandWrongOutputFileThrowsInvalidArgumentException(): void

diff --git a/tests/unit/Video/Filter/SelectFilterTest.php b/tests/unit/Video/Filter/SelectFilterTest.php
@@ -16,7 +16,7 @@ public function setUp(): void
     public function testGetFFMpegCLIValue(): void
     {
         self::assertEquals(
-            '"select=eq(n\,10)"',
+            'select=eq(n\,10)',
             (new SelectFilter('eq(n\,10)'))->getFFmpegCLIValue()
         );
     }

diff --git a/tests/unit/Video/VideoConverterTest.php b/tests/unit/Video/VideoConverterTest.php
@@ -64,7 +64,8 @@ public function testGetSymfonyProcessMustReturnCorrectParams(): void
             $convertParams
         );
 
-        $cmdLine = $process->getCommandLine();
+        // We test on unescaped command argument (because it's more convenient)
+        $cmdLine = str_replace("'", '', $process->getCommandLine());
 
         self::assertContains(' -c:v libvpx-vp9 ', $cmdLine);
         self::assertContains(' -b:v 200k ', $cmdLine);
@@ -82,7 +83,7 @@ public function testGetSymfonyProcessMustReturnCorrectParams(): void
         self::assertContains(' -f webm ', $cmdLine);
         self::assertContains(' -ss 0:00:01.0 ', $cmdLine);
         self::assertContains(' -frames:v 200 ', $cmdLine);
-        self::assertContains(escapeshellarg('/path/output'), $cmdLine);
+        self::assertContains('/path/output', $cmdLine);
     }
 
     public function testGetSymfonyProcessMustThrowExceptionOnWrongOutput(): void
@@ -116,8 +117,10 @@ public function getFile(): string
             $convertParams
         );
 
+        // We test on unescaped command argument (because it's more convenient)
         $cmdLine = $process->getCommandLine();
-        self::assertContains(' /a n/un \'escaped/file', $cmdLine);
+
+        self::assertContains('/a n/un \'\\\'\'escaped/file\'', $cmdLine);
     }
 
     public function testGetSymfonyProcessMustDefaultToConfigThreads(): void
@@ -132,7 +135,8 @@ public function testGetSymfonyProcessMustDefaultToConfigThreads(): void
             $convertParams
         );
 
-        $cmdLine = $process->getCommandLine();
+        // We test on unescaped command argument (because it's more convenient)
+        $cmdLine = str_replace("'", '', $process->getCommandLine());
 
         self::assertContains(' -threads 3 ', $cmdLine);
 

diff --git a/tests/unit/Video/VideoThumbGeneratorTest.php b/tests/unit/Video/VideoThumbGeneratorTest.php
@@ -35,11 +35,12 @@ public function testGetSymfonyProcessWithFrame(): void
             $thumbParams
         );
 
-        $cmdLine = $process->getCommandLine();
+        // We test on unescaped command argument (because it's more convenient)
+        $cmdLine = str_replace("'", '', $process->getCommandLine());
 
         // Note that we did 2 -1
-        self::assertContains(' -filter:v "select=eq(n\,1)" ', $cmdLine);
-        self::assertContains(escapeshellarg('/path/output.jpg'), $cmdLine);
+        self::assertContains(' -filter:v select=eq(n\,1) ', $cmdLine);
+        self::assertContains('/path/output.jpg', $cmdLine);
     }
 
     public function testGetSymfonyProcessWithFrame0(): void
@@ -53,10 +54,11 @@ public function testGetSymfonyProcessWithFrame0(): void
             $thumbParams
         );
 
-        $cmdLine = $process->getCommandLine();
+        // We test on unescaped command argument (because it's more convenient)
+        $cmdLine = str_replace("'", '', $process->getCommandLine());
 
-        self::assertContains(' -filter:v "select=eq(n\,0)" ', $cmdLine);
-        self::assertContains(escapeshellarg('/path/output.jpg'), $cmdLine);
+        self::assertContains(' -filter:v select=eq(n\,0) ', $cmdLine);
+        self::assertContains('/path/output.jpg', $cmdLine);
     }
 
     public function testGetSymfonyProcessWithTime(): void
@@ -70,10 +72,11 @@ public function testGetSymfonyProcessWithTime(): void
             $thumbParams
         );
 
-        $cmdLine = $process->getCommandLine();
+        // We test on unescaped command argument (because it's more convenient)
+        $cmdLine = str_replace("'", '', $process->getCommandLine());
 
         self::assertContains(' -ss 0:00:01.234 ', $cmdLine);
-        self::assertContains(escapeshellarg('/path/output.jpg'), $cmdLine);
+        self::assertContains('/path/output.jpg', $cmdLine);
     }
 
     public function testGetSymfonyProcessWithFrameAndFilters(): void
@@ -92,10 +95,10 @@ public function testGetSymfonyProcessWithFrameAndFilters(): void
             $thumbParams
         );
 
-        $cmdLine = $process->getCommandLine();
+        // We test on unescaped command argument (because it's more convenient)
+        $cmdLine = str_replace("'", '', $process->getCommandLine());
 
-        self::assertContains(' -filter:v "select=eq(n\,1)",hqdn3d,nlmeans ', $cmdLine);
-        self::assertContains(escapeshellarg('/path/output.jpg'), $cmdLine);
+        self::assertContains(' -filter:v select=eq(n\,1),hqdn3d,nlmeans ', $cmdLine);
     }
 
     public function testGetSymfonyProcessWithEmptyFilterChain(): void
@@ -112,9 +115,9 @@ public function testGetSymfonyProcessWithEmptyFilterChain(): void
             $thumbParams
         );
 
-        $cmdLine = $process->getCommandLine();
+        // We test on unescaped command argument (because it's more convenient)
+        $cmdLine = str_replace("'", '', $process->getCommandLine());
 
         self::assertNotContains(' -filter:v ', $cmdLine);
-        self::assertContains(escapeshellarg('/path/output.jpg'), $cmdLine);
     }
 }