Merge pull request #49 from alchemy-fr/fix-47

Fix #47, remove stateful filters on MediaTypeInterface::save

CHANGELOG.md

@@ -4,6 +4,7 @@ CHANGELOG
 * 0.3.1 (xx-xx-2013)
 
   * Allow use of FFProbe on remote URIs.
+  * Fix #47 : MediaTypeInterface::save adds filters depending on the codec.
 
 * 0.3.0 (07-04-2013)
 

src/FFMpeg/Media/Audio.php

@@ -69,21 +69,24 @@ class Audio extends AbstractStreamableMedia
 
         $commands = array('-y', '-i', $this->pathfile);
 
-        $this->addFilter(new SimpleFilter($format->getExtraParams(), 10));
+        $filters = clone $this->filters;
+        $filters->add(new SimpleFilter($format->getExtraParams(), 10));
 
         if ($this->driver->getConfiguration()->has('ffmpeg.threads')) {
-            $this->addFilter(new SimpleFilter(array('-threads', $this->driver->getConfiguration()->get('ffmpeg.threads'))));
+            $filters->add(new SimpleFilter(array('-threads', $this->driver->getConfiguration()->get('ffmpeg.threads'))));
         }
         if (null !== $format->getAudioCodec()) {
-            $this->addFilter(new SimpleFilter(array('-acodec', $format->getAudioCodec())));
+            $filters->add(new SimpleFilter(array('-acodec', $format->getAudioCodec())));
         }
 
-        foreach ($this->filters as $filter) {
+        foreach ($filters as $filter) {
             $commands = array_merge($commands, $filter->apply($this, $format));
         }
 
-        $commands[] = '-b:a';
+        if (null !== $format->getAudioKiloBitrate()) {
-        $commands[] = $format->getAudioKiloBitrate() . 'k';
+            $commands[] = '-b:a';
+            $commands[] = $format->getAudioKiloBitrate() . 'k';
+        }
         $commands[] = $outputPathfile;
 
         try {

src/FFMpeg/Media/Video.php

@@ -59,19 +59,20 @@ class Video extends Audio
     {
         $commands = array('-y', '-i', $this->pathfile);
 
-        $this->addFilter(new SimpleFilter($format->getExtraParams(), 10));
+        $filters = clone $this->filters;
+        $filters->add(new SimpleFilter($format->getExtraParams(), 10));
 
         if ($this->driver->getConfiguration()->has('ffmpeg.threads')) {
-            $this->addFilter(new SimpleFilter(array('-threads', $this->driver->getConfiguration()->get('ffmpeg.threads'))));
+            $filters->add(new SimpleFilter(array('-threads', $this->driver->getConfiguration()->get('ffmpeg.threads'))));
         }
         if (null !== $format->getVideoCodec()) {
-            $this->addFilter(new SimpleFilter(array('-vcodec', $format->getVideoCodec())));
+            $filters->add(new SimpleFilter(array('-vcodec', $format->getVideoCodec())));
         }
         if (null !== $format->getAudioCodec()) {
-            $this->addFilter(new SimpleFilter(array('-acodec', $format->getAudioCodec())));
+            $filters->add(new SimpleFilter(array('-acodec', $format->getAudioCodec())));
         }
 
-        foreach ($this->filters as $filter) {
+        foreach ($filters as $filter) {
             $commands = array_merge($commands, $filter->apply($this, $format));
         }
 
@@ -97,8 +98,11 @@ class Video extends Audio
         $commands[] = '4';
         $commands[] = '-trellis';
         $commands[] = '1';
-        $commands[] = '-b:a';
+
-        $commands[] = $format->getAudioKiloBitrate() . 'k';
+        if (null !== $format->getAudioKiloBitrate()) {
+            $commands[] = '-b:a';
+            $commands[] = $format->getAudioKiloBitrate() . 'k';
+        }
 
         $passPrefix = uniqid('pass-');
 

tests/FFMpeg/Tests/Media/AudioTest.php

@@ -278,6 +278,56 @@ class AudioTest extends AbstractStreamableTestCase
         );
     }
 
+    public function testSaveShouldNotStoreCodecFiltersInTheMedia()
+    {
+        $driver = $this->getFFMpegDriverMock();
+        $ffprobe = $this->getFFProbeMock();
+
+        $configuration = $this->getMock('Alchemy\BinaryDriver\ConfigurationInterface');
+
+        $driver->expects($this->any())
+            ->method('getConfiguration')
+            ->will($this->returnValue($configuration));
+
+        $configuration->expects($this->any())
+            ->method('has')
+            ->with($this->equalTo('ffmpeg.threads'))
+            ->will($this->returnValue(true));
+
+        $configuration->expects($this->any())
+            ->method('get')
+            ->with($this->equalTo('ffmpeg.threads'))
+            ->will($this->returnValue(24));
+
+        $capturedCommands = array();
+
+        $driver->expects($this->exactly(2))
+            ->method('command')
+            ->with($this->isType('array'), false, $this->anything())
+            ->will($this->returnCallback(function ($commands, $errors, $listeners) use (&$capturedCommands, &$capturedListeners) {
+                $capturedCommands[] = $commands;
+            }));
+
+        $outputPathfile = '/target/file';
+
+        $format = $this->getMock('FFMpeg\Format\AudioInterface');
+        $format->expects($this->any())
+            ->method('getExtraParams')
+            ->will($this->returnValue(array('param')));
+
+        $audio = new Audio(__FILE__, $driver, $ffprobe);
+        $audio->save($format, $outputPathfile);
+        $audio->save($format, $outputPathfile);
+
+        $expected = array(
+            '-y', '-i', __FILE__, 'param', '-threads', 24, '/target/file',
+        );
+
+        foreach ($capturedCommands as $capturedCommand) {
+            $this->assertEquals($expected, $capturedCommand);
+        }
+    }
+
     public function getClassName()
     {
         return 'FFMpeg\Media\Audio';

tests/FFMpeg/Tests/Media/VideoTest.php

@@ -404,6 +404,98 @@ class VideoTest extends AbstractStreamableTestCase
         );
     }
 
+    public function testSaveShouldNotStoreCodecFiltersInTheMedia()
+    {
+        $driver = $this->getFFMpegDriverMock();
+        $ffprobe = $this->getFFProbeMock();
+
+        $configuration = $this->getMock('Alchemy\BinaryDriver\ConfigurationInterface');
+
+        $driver->expects($this->any())
+            ->method('getConfiguration')
+            ->will($this->returnValue($configuration));
+
+        $configuration->expects($this->any())
+            ->method('has')
+            ->with($this->equalTo('ffmpeg.threads'))
+            ->will($this->returnValue(true));
+
+        $configuration->expects($this->any())
+            ->method('get')
+            ->with($this->equalTo('ffmpeg.threads'))
+            ->will($this->returnValue(24));
+
+        $capturedCommands = array();
+
+        $driver->expects($this->exactly(4))
+            ->method('command')
+            ->with($this->isType('array'), false, $this->anything())
+            ->will($this->returnCallback(function ($commands, $errors, $listeners) use (&$capturedCommands, &$capturedListeners) {
+                $capturedCommands[] = $commands;
+            }));
+
+        $outputPathfile = '/target/file';
+
+        $format = $this->getMock('FFMpeg\Format\VideoInterface');
+        $format->expects($this->any())
+            ->method('getExtraParams')
+            ->will($this->returnValue(array('param')));
+
+        $video = new Video(__FILE__, $driver, $ffprobe);
+        $video->save($format, $outputPathfile);
+        $video->save($format, $outputPathfile);
+
+        $expectedPass1 = array(
+            '-y', '-i', __FILE__, 'param', '-threads', 24, '-b:v', 'k', '-refs',
+            '6', '-coder', '1', '-sc_threshold', '40', '-flags', '+loop',
+            '-me_range', '16', '-subq', '7', '-i_qfactor', '0.71',
+            '-qcomp', '0.6', '-qdiff', '4', '-trellis', '1',
+            '-pass', '1', '-passlogfile', '/target/file',
+        );
+        $expectedPass2 = array(
+            '-y', '-i', __FILE__, 'param', '-threads', 24, '-b:v', 'k', '-refs',
+            '6', '-coder', '1', '-sc_threshold', '40', '-flags', '+loop',
+            '-me_range', '16', '-subq', '7', '-i_qfactor', '0.71',
+            '-qcomp', '0.6', '-qdiff', '4', '-trellis', '1',
+            '-pass', '2', '-passlogfile', '/target/file',
+        );
+
+        $n = 1;
+        foreach ($capturedCommands as $capturedCommand) {
+            foreach ($capturedCommand as $command) {
+                if (0 === strpos($command, 'pass-')) {
+                    $prefix = $command;
+                    break;
+                }
+            }
+
+            if (null === $prefix) {
+                $this->fail('Unable to find pass prefix command.');
+            }
+
+            $found = false;
+            foreach ($capturedCommand as $key => $command) {
+                if ($command === $prefix) {
+                    $found = true;
+                    unset($capturedCommand[$key]);
+                    $capturedCommand = array_values($capturedCommand);
+                    break;
+                }
+            }
+
+            if (!$found) {
+                $this->fail('Unable to find pass prefix command back.');
+            }
+
+            if (0 === $n % 2) {
+                $this->assertEquals($expectedPass2, $capturedCommand);
+            } else {
+                $this->assertEquals($expectedPass1, $capturedCommand);
+            }
+            $n++;
+        }
+    }
+
     public function getClassName()
     {
         return 'FFMpeg\Media\Video';