From 6664e1d1eabcf15c499baa428c91bb0f2f81549e Mon Sep 17 00:00:00 2001 From: Phan An Date: Tue, 3 Sep 2024 12:52:07 +0200 Subject: [PATCH] feat: use transcode-and-cache instead of direct on-the-fly transcode&streaming --- .env.example | 2 +- app/Services/DownloadService.php | 8 +-- .../Adapters/PodcastStreamerAdapter.php | 8 +-- .../Adapters/TranscodingStreamerAdapter.php | 34 ++--------- app/Values/Podcast/EpisodePlayable.php | 9 ++- app/Values/TranscodeResult.php | 56 +++++++++++++++++++ .../Services/PodcastServiceTest.php | 2 +- .../Values/EpisodePlayableTest.php | 9 ++- .../Values/TranscodeResultTest.php | 55 ++++++++++++++++++ 9 files changed, 134 insertions(+), 49 deletions(-) create mode 100644 app/Values/TranscodeResult.php create mode 100644 tests/Integration/Values/TranscodeResultTest.php diff --git a/.env.example b/.env.example index d6036dec..8363d4d6 100644 --- a/.env.example +++ b/.env.example @@ -105,7 +105,7 @@ MEMORY_LIMIT= # See https://docs.koel.dev/usage/streaming for more information. # Note: This setting doesn't have effect if the media needs transcoding (e.g. FLAC). # ################################################## -# IMPORTANT: It's HIGHLY recommended to use 'x-sendfile' or 'x-accel-redirect' if +# It's HIGHLY recommended to use 'x-sendfile' or 'x-accel-redirect' if # you plan to use the Koel mobile apps. # ################################################## STREAMING_METHOD=php diff --git a/app/Services/DownloadService.php b/app/Services/DownloadService.php index 4e9124a5..3601d962 100644 --- a/app/Services/DownloadService.php +++ b/app/Services/DownloadService.php @@ -33,13 +33,7 @@ class DownloadService } if ($song->isEpisode()) { - $playable = EpisodePlayable::retrieveForEpisode($song); - - if (!$playable?->valid()) { - $playable = EpisodePlayable::createForEpisode($song); - } - - return $playable->path; + return EpisodePlayable::getForEpisode($song)->path; } if ($song->storage === SongStorageType::LOCAL) { diff --git a/app/Services/Streamer/Adapters/PodcastStreamerAdapter.php b/app/Services/Streamer/Adapters/PodcastStreamerAdapter.php index 33ec16a5..c59c4160 100644 --- a/app/Services/Streamer/Adapters/PodcastStreamerAdapter.php +++ b/app/Services/Streamer/Adapters/PodcastStreamerAdapter.php @@ -27,12 +27,6 @@ class PodcastStreamerAdapter implements StreamerAdapter return response()->redirectTo($streamableUrl); } - $playable = EpisodePlayable::retrieveForEpisode($song); - - if (!$playable?->valid()) { - $playable = EpisodePlayable::createForEpisode($song); - } - - $this->streamLocalPath($playable->path); + $this->streamLocalPath(EpisodePlayable::getForEpisode($song)->path); } } diff --git a/app/Services/Streamer/Adapters/TranscodingStreamerAdapter.php b/app/Services/Streamer/Adapters/TranscodingStreamerAdapter.php index 4be8ed4e..155a2854 100644 --- a/app/Services/Streamer/Adapters/TranscodingStreamerAdapter.php +++ b/app/Services/Streamer/Adapters/TranscodingStreamerAdapter.php @@ -3,43 +3,21 @@ namespace App\Services\Streamer\Adapters; use App\Models\Song; +use App\Services\Streamer\Adapters\Concerns\StreamsLocalPath; +use App\Values\TranscodeResult; use Illuminate\Support\Arr; class TranscodingStreamerAdapter implements StreamerAdapter { - /** - * On-the-fly stream the current song while transcoding. - */ + use StreamsLocalPath; + public function stream(Song $song, array $config = []): void { - $ffmpeg = config('koel.streaming.ffmpeg_path'); - abort_unless(is_executable($ffmpeg), 500, 'Transcoding requires valid ffmpeg settings.'); - - $path = $song->storage_metadata->getPath(); + abort_unless(is_executable(config('koel.streaming.ffmpeg_path')), 500, 'ffmpeg not found or not executable.'); $bitRate = filter_var(Arr::get($config, 'bit_rate'), FILTER_SANITIZE_NUMBER_INT) ?: config('koel.streaming.bitrate'); - $startTime = filter_var(Arr::get($config, 'start_time', 0), FILTER_SANITIZE_NUMBER_FLOAT); - - setlocale(LC_CTYPE, 'en_US.UTF-8'); // #1481 special chars might be stripped otherwise - - header('Content-Type: audio/mpeg'); - header('Content-Disposition: attachment; filename="' . basename($path) . '"'); - - $args = [ - '-i ' . escapeshellarg($path), - '-map 0:0', - '-v 0', - "-ab {$bitRate}k", - '-f mp3', - '-', - ]; - - if ($startTime) { - array_unshift($args, "-ss $startTime"); - } - - passthru("$ffmpeg " . implode(' ', $args)); + $this->streamLocalPath(TranscodeResult::getForSong($song, $bitRate)->path); } } diff --git a/app/Values/Podcast/EpisodePlayable.php b/app/Values/Podcast/EpisodePlayable.php index 2abc389f..905908f9 100644 --- a/app/Values/Podcast/EpisodePlayable.php +++ b/app/Values/Podcast/EpisodePlayable.php @@ -25,12 +25,15 @@ final class EpisodePlayable implements Arrayable, Jsonable return File::isReadable($this->path) && $this->checksum === md5_file($this->path); } - public static function retrieveForEpisode(Episode $episode): ?self + public static function getForEpisode(Episode $episode): ?self { - return Cache::get("episode-playable.$episode->id"); + /** @var self|null $cached */ + $cached = Cache::get("episode-playable.$episode->id"); + + return $cached?->valid() ? $cached : self::createForEpisode($episode); } - public static function createForEpisode(Episode $episode): self + private static function createForEpisode(Episode $episode): self { $dir = sys_get_temp_dir() . '/koel-episodes'; $file = sprintf('%s/%s.mp3', $dir, $episode->id); diff --git a/app/Values/TranscodeResult.php b/app/Values/TranscodeResult.php new file mode 100644 index 00000000..040b00d3 --- /dev/null +++ b/app/Values/TranscodeResult.php @@ -0,0 +1,56 @@ +id}.$bitRate"); + + return $cached?->valid() ? $cached : self::createForSong($song, $bitRate, $transcodedPath); + } + + private static function createForSong(Song $song, int $bitRate, ?string $transcodedPath = null): self + { + setlocale(LC_CTYPE, 'en_US.UTF-8'); // #1481 special chars might be stripped otherwise + + $dir = sys_get_temp_dir() . '/koel-transcodes'; + $transcodedPath ??= sprintf('%s/%s.%s.mp3', $dir, $song, $bitRate); + + File::ensureDirectoryExists($dir); + + Process::timeout(60)->run([ + config('koel.streaming.ffmpeg_path'), + '-i', + $song->storage_metadata->getPath(), + '-vn', + '-b:a', + "{$bitRate}k", + '-preset', + 'ultrafast', + '-y', // Overwrite output file if it exists + $transcodedPath, + ]); + + $cached = new self($transcodedPath, md5_file($transcodedPath)); + Cache::forever("transcoded.{$song->id}.$bitRate", $cached); + + return $cached; + } + + private function valid(): bool + { + return File::isReadable($this->path) && $this->checksum === md5_file($this->path); + } +} diff --git a/tests/Integration/Services/PodcastServiceTest.php b/tests/Integration/Services/PodcastServiceTest.php index c1317e8e..6e302a6f 100644 --- a/tests/Integration/Services/PodcastServiceTest.php +++ b/tests/Integration/Services/PodcastServiceTest.php @@ -8,11 +8,11 @@ use App\Models\PodcastUserPivot; use App\Models\Song; use App\Services\PodcastService; use GuzzleHttp\Client; -use GuzzleHttp\ClientInterface; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; use GuzzleHttp\Psr7\Response; use Illuminate\Support\Facades\Http; +use Psr\Http\Client\ClientInterface; use Tests\TestCase; use function Tests\create_user; diff --git a/tests/Integration/Values/EpisodePlayableTest.php b/tests/Integration/Values/EpisodePlayableTest.php index af1b6298..26a7397b 100644 --- a/tests/Integration/Values/EpisodePlayableTest.php +++ b/tests/Integration/Values/EpisodePlayableTest.php @@ -21,12 +21,17 @@ class EpisodePlayableTest extends TestCase 'path' => 'https://example.com/episode.mp3', ]); - $playable = EpisodePlayable::createForEpisode($episode); + $playable = EpisodePlayable::getForEpisode($episode); + + Http::assertSentCount(1); self::assertSame('acbd18db4cc2f85cedef654fccc4a4d8', $playable->checksum); self::assertTrue(Cache::has("episode-playable.$episode->id")); - $retrieved = EpisodePlayable::retrieveForEpisode($episode); + $retrieved = EpisodePlayable::getForEpisode($episode); + + // No extra HTTP request should be made. + Http::assertSentCount(1); self::assertSame($playable, $retrieved); self::assertTrue($retrieved->valid()); diff --git a/tests/Integration/Values/TranscodeResultTest.php b/tests/Integration/Values/TranscodeResultTest.php new file mode 100644 index 00000000..db92d83c --- /dev/null +++ b/tests/Integration/Values/TranscodeResultTest.php @@ -0,0 +1,55 @@ + '/usr/bin/ffmpeg']); + Process::fake(); + + /** @var Song $song */ + $song = Song::factory()->create(); + + $result = TranscodeResult::getForSong($song, 128, test_path('songs/full.mp3')); + + $closure = static function (PendingProcess $process) use ($song): bool { + return $process->command === [ + '/usr/bin/ffmpeg', + '-i', + $song->storage_metadata->getPath(), + '-vn', + '-b:a', + '128k', + '-preset', + 'ultrafast', + '-y', + test_path('songs/full.mp3'), + ]; + }; + + Process::assertRanTimes($closure, 1); + + self::assertSame('3c7b4e187277e40f8ae793650336e03b', $result->checksum); + self::assertSame(test_path('songs/full.mp3'), $result->path); + + self::assertTrue(Cache::has("transcoded.{$song->id}.128")); + + $cached = TranscodeResult::getForSong($song, 128, test_path('songs/full.mp3')); + + // No extra ffmpeg process should be run. + Process::assertRanTimes($closure, 1); + self::assertSame('3c7b4e187277e40f8ae793650336e03b', $cached->checksum); + self::assertSame(test_path('songs/full.mp3'), $cached->path); + } +}