From 5edcde3a8d922745fd98690c630d46846d987425 Mon Sep 17 00:00:00 2001 From: Peter Deltchev Date: Mon, 15 Feb 2016 05:06:06 -0800 Subject: [PATCH] The rebuild:track command properly deals with unfinished uploads now. This commit involved a huge refactoring of UploadTrackCommand. --- app/Commands/GenerateTrackFilesCommand.php | 187 +++++++++++++++++++++ app/Commands/UploadTrackCommand.php | 109 ++---------- app/Console/Commands/RebuildTrack.php | 14 +- app/Jobs/EncodeTrackFile.php | 2 +- app/Models/Track.php | 2 +- 5 files changed, 212 insertions(+), 102 deletions(-) create mode 100644 app/Commands/GenerateTrackFilesCommand.php diff --git a/app/Commands/GenerateTrackFilesCommand.php b/app/Commands/GenerateTrackFilesCommand.php new file mode 100644 index 00000000..74156fdd --- /dev/null +++ b/app/Commands/GenerateTrackFilesCommand.php @@ -0,0 +1,187 @@ +. + */ + +namespace Poniverse\Ponyfm\Commands; + +use FFmpegMovie; +use Illuminate\Foundation\Bus\DispatchesJobs; +use Poniverse\Ponyfm\Exceptions\InvalidEncodeOptionsException; +use Poniverse\Ponyfm\Jobs\EncodeTrackFile; +use Poniverse\Ponyfm\Models\Track; +use Poniverse\Ponyfm\Models\TrackFile; +use AudioCache; +use File; +use Illuminate\Support\Str; +use SplFileInfo; +use Validator; + +/** + * This command is the "second phase" of the upload process - once metadata has + * been parsed and the track object is created, this generates the track's + * corresponding TrackFile objects and ensures that all of them have been encoded. + * + * @package Poniverse\Ponyfm\Commands + */ +class GenerateTrackFilesCommand extends CommandBase +{ + use DispatchesJobs; + + private $track; + private $autoPublish; + private $sourceFile; + + static $_losslessFormats = [ + 'flac', + 'pcm', + 'adpcm', + ]; + + public function __construct(Track $track, SplFileInfo $sourceFile, bool $autoPublish = false, int $reprocessTrackId = null) + { + $this->track = $track; + $this->autoPublish = $autoPublish; + $this->sourceFile = $sourceFile; + } + + /** + * @throws \Exception + * @return CommandResponse + */ + public function execute() + { + try { + $source = $this->sourceFile->getPathname(); + + // Lossy uploads need to be identified and set as the master file + // without being re-encoded. + $audioObject = AudioCache::get($source); + $isLossyUpload = !$this->isLosslessFile($audioObject); + + if ($isLossyUpload) { + if ($audioObject->getAudioCodec() === 'mp3') { + $masterFormat = 'MP3'; + + } else if (Str::startsWith($audioObject->getAudioCodec(), 'aac')) { + $masterFormat = 'AAC'; + + } else if ($audioObject->getAudioCodec() === 'vorbis') { + $masterFormat = 'OGG Vorbis'; + + } else { + $validator = new Validator(); + $validator->messages()->add('track', 'The track does not contain audio in a known lossy format.'); + $this->track->delete(); + return CommandResponse::fail($validator); + } + + // Sanity check: skip creating this TrackFile if it already exists. + $trackFile = $this->trackFileExists($masterFormat); + + if (!$trackFile) { + $trackFile = new TrackFile(); + $trackFile->is_master = true; + $trackFile->format = $masterFormat; + $trackFile->track_id = $this->track->id; + $trackFile->save(); + } + + // Lossy masters are copied into the datastore - no re-encoding involved. + File::copy($source, $trackFile->getFile()); + } + + + $trackFiles = []; + + foreach (Track::$Formats as $name => $format) { + // Don't bother with lossless transcodes of lossy uploads, and + // don't re-encode the lossy master. + if ($isLossyUpload && ($format['is_lossless'] || $name === $masterFormat)) { + continue; + } + + // Sanity check: skip creating this TrackFile if it already exists. + // But, we'll still encode it! + if ($trackFile = $this->trackFileExists($name)) { + $trackFiles[] = $trackFile; + continue; + } + + $trackFile = new TrackFile(); + $trackFile->is_master = $name === 'FLAC' ? true : false; + $trackFile->format = $name; + $trackFile->status = TrackFile::STATUS_PROCESSING_PENDING; + + if (in_array($name, Track::$CacheableFormats) && $trackFile->is_master == false) { + $trackFile->is_cacheable = true; + } else { + $trackFile->is_cacheable = false; + } + $this->track->trackFiles()->save($trackFile); + + // All TrackFile records we need are synchronously created + // before kicking off the encode jobs in order to avoid a race + // condition with the "temporary" source file getting deleted. + $trackFiles[] = $trackFile; + } + + try { + foreach ($trackFiles as $trackFile) { + $this->dispatch(new EncodeTrackFile($trackFile, false, true, $this->autoPublish)); + } + + } catch (InvalidEncodeOptionsException $e) { + $this->track->delete(); + return CommandResponse::fail(['track' => [$e->getMessage()]]); + } + + } catch (\Exception $e) { + $this->track->delete(); + throw $e; + } + + return CommandResponse::succeed([ + 'id' => $this->track->id, + 'name' => $this->track->name, + 'title' => $this->track->title, + 'slug' => $this->track->slug, + 'autoPublish' => $this->autoPublish, + ]); + } + + /** + * @param FFmpegMovie|string $file object or full path of the file we're checking + * @return bool whether the given file is lossless + */ + private function isLosslessFile($file) { + if (is_string($file)) { + $file = AudioCache::get($file); + } + + return Str::startsWith($file->getAudioCodec(), static::$_losslessFormats); + } + + /** + * @param string $format + * @return TrackFile|null + */ + private function trackFileExists(string $format) { + return $this->track->trackFiles()->where('format', $format)->first(); + } +} diff --git a/app/Commands/UploadTrackCommand.php b/app/Commands/UploadTrackCommand.php index f1ba085d..1dec7158 100644 --- a/app/Commands/UploadTrackCommand.php +++ b/app/Commands/UploadTrackCommand.php @@ -26,12 +26,9 @@ use getID3; use Illuminate\Foundation\Bus\DispatchesJobs; use Input; use Poniverse\Ponyfm\Models\Album; -use Poniverse\Ponyfm\Exceptions\InvalidEncodeOptionsException; use Poniverse\Ponyfm\Models\Genre; use Poniverse\Ponyfm\Models\Image; -use Poniverse\Ponyfm\Jobs\EncodeTrackFile; use Poniverse\Ponyfm\Models\Track; -use Poniverse\Ponyfm\Models\TrackFile; use AudioCache; use File; use Illuminate\Support\Str; @@ -39,24 +36,26 @@ use Poniverse\Ponyfm\Models\TrackType; use Poniverse\Ponyfm\Models\User; use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; +use Validator; class UploadTrackCommand extends CommandBase { use DispatchesJobs; - private $_allowLossy; private $_allowShortTrack; private $_customTrackSource; private $_autoPublishByDefault; - private $_losslessFormats = [ - 'flac', - 'pcm', - 'adpcm', - ]; - - public function __construct($allowLossy = false, $allowShortTrack = false, $customTrackSource = null, $autoPublishByDefault = false) + /** + * UploadTrackCommand constructor. + * + * @param bool $allowLossy + * @param bool $allowShortTrack allow tracks shorter than 30 seconds + * @param string|null $customTrackSource value to set in the track's "source" field; if left blank, "direct_upload" is used + * @param bool $autoPublishByDefault + */ + public function __construct(bool $allowLossy = false, bool $allowShortTrack = false, string $customTrackSource = null, bool $autoPublishByDefault = false) { $this->_allowLossy = $allowLossy; $this->_allowShortTrack = $allowShortTrack; @@ -137,7 +136,6 @@ class UploadTrackCommand extends CommandBase return CommandResponse::fail($validator); } - // Process optional track fields $autoPublish = (bool) ($input['auto_publish'] ?? $this->_autoPublishByDefault); @@ -184,91 +182,8 @@ class UploadTrackCommand extends CommandBase $track->save(); - - try { - $source = $trackFile->getPathname(); - - // Lossy uploads need to be identified and set as the master file - // without being re-encoded. - $audioObject = AudioCache::get($source); - $isLossyUpload = !Str::startsWith($audioObject->getAudioCodec(), $this->_losslessFormats); - - if ($isLossyUpload) { - if ($audioObject->getAudioCodec() === 'mp3') { - $masterFormat = 'MP3'; - - } else if (Str::startsWith($audioObject->getAudioCodec(), 'aac')) { - $masterFormat = 'AAC'; - - } else if ($audioObject->getAudioCodec() === 'vorbis') { - $masterFormat = 'OGG Vorbis'; - - } else { - $validator->messages()->add('track', 'The track does not contain audio in a known lossy format.'); - $track->delete(); - return CommandResponse::fail($validator); - } - - $trackFile = new TrackFile(); - $trackFile->is_master = true; - $trackFile->format = $masterFormat; - $trackFile->track_id = $track->id; - $trackFile->save(); - - // Lossy masters are copied into the datastore - no re-encoding involved. - File::copy($source, $trackFile->getFile()); - } - - - $trackFiles = []; - - foreach (Track::$Formats as $name => $format) { - // Don't bother with lossless transcodes of lossy uploads, and - // don't re-encode the lossy master. - if ($isLossyUpload && ($format['is_lossless'] || $name === $masterFormat)) { - continue; - } - - $trackFile = new TrackFile(); - $trackFile->is_master = $name === 'FLAC' ? true : false; - $trackFile->format = $name; - $trackFile->status = TrackFile::STATUS_PROCESSING_PENDING; - - if (in_array($name, Track::$CacheableFormats) && $trackFile->is_master == false) { - $trackFile->is_cacheable = true; - } else { - $trackFile->is_cacheable = false; - } - $track->trackFiles()->save($trackFile); - - // All TrackFile records we need are synchronously created - // before kicking off the encode jobs in order to avoid a race - // condition with the "temporary" source file getting deleted. - $trackFiles[] = $trackFile; - } - - try { - foreach($trackFiles as $trackFile) { - $this->dispatch(new EncodeTrackFile($trackFile, false, true, $autoPublish)); - } - - } catch (InvalidEncodeOptionsException $e) { - $track->delete(); - return CommandResponse::fail(['track' => [$e->getMessage()]]); - } - - } catch (\Exception $e) { - $track->delete(); - throw $e; - } - - return CommandResponse::succeed([ - 'id' => $track->id, - 'name' => $track->name, - 'title' => $track->title, - 'slug' => $track->slug, - 'autoPublish' => $autoPublish, - ]); + $generateTrackFiles = new GenerateTrackFilesCommand($track, $trackFile, $autoPublish); + return $generateTrackFiles->execute(); } /** diff --git a/app/Console/Commands/RebuildTrack.php b/app/Console/Commands/RebuildTrack.php index 136faf8d..334cdaef 100644 --- a/app/Console/Commands/RebuildTrack.php +++ b/app/Console/Commands/RebuildTrack.php @@ -22,6 +22,8 @@ namespace Poniverse\Ponyfm\Console\Commands; use Illuminate\Console\Command; use Illuminate\Foundation\Bus\DispatchesJobs; +use Poniverse\Ponyfm\Commands\GenerateTrackFilesCommand; +use Poniverse\Ponyfm\Commands\UploadTrackCommand; use Poniverse\Ponyfm\Jobs\EncodeTrackFile; use Poniverse\Ponyfm\Models\Track; @@ -66,9 +68,15 @@ class RebuildTrack extends Command $track = Track::with('trackFiles')->withTrashed()->find((int) $this->argument('trackId')); if($this->option('upload')) { - foreach($track->trackFiles as $trackFile) { - $this->info("Re-encoding this track's {$trackFile->format} file..."); - $this->dispatch(new EncodeTrackFile($trackFile, false, true, false)); + $this->info("Attempting to finish this track's upload..."); + $sourceFile = new \SplFileInfo($track->getTemporarySourceFile()); + $generateTrackFiles = new GenerateTrackFilesCommand($track, $sourceFile, false); + $result = $generateTrackFiles->execute(); + // The GenerateTrackFiles command will re-encode all TrackFiles. + + if ($result->didFail()) { + $this->error("Something went wrong!"); + $this->error(json_encode($result->getMessages(), JSON_PRETTY_PRINT)); } } else { diff --git a/app/Jobs/EncodeTrackFile.php b/app/Jobs/EncodeTrackFile.php index d28f606c..75a1972f 100644 --- a/app/Jobs/EncodeTrackFile.php +++ b/app/Jobs/EncodeTrackFile.php @@ -87,7 +87,7 @@ class EncodeTrackFile extends Job implements SelfHandling, ShouldQueue { DB::reconnect(); - // Sanity-check: was this file just generated, or is it already being processed? + // Sanity check: was this file just generated, or is it already being processed? if ($this->trackFile->status === TrackFile::STATUS_PROCESSING) { Log::warning('Track file #'.$this->trackFile->id.' (track #'.$this->trackFile->track_id.') is already being processed!'); return; diff --git a/app/Models/Track.php b/app/Models/Track.php index d0fdfdf0..0dcfdd7e 100644 --- a/app/Models/Track.php +++ b/app/Models/Track.php @@ -682,7 +682,7 @@ class Track extends Model implements Searchable * * @return string */ - public function getTemporarySourceFile() { + public function getTemporarySourceFile():string { return Config::get('ponyfm.files_directory').'/queued-tracks/'.$this->id; }