From a2798c8763f4579665892d4c4b3520987744ab51 Mon Sep 17 00:00:00 2001 From: Varun Patil Date: Fri, 24 Feb 2023 00:21:38 -0800 Subject: [PATCH] video: improve transcode logging (#428) --- lib/Controller/ApiBase.php | 6 ++++- lib/Controller/VideoController.php | 43 +++++++++++++++++++++++++----- src/components/viewer/PsVideo.ts | 30 ++++++++++----------- 3 files changed, 55 insertions(+), 24 deletions(-) diff --git a/lib/Controller/ApiBase.php b/lib/Controller/ApiBase.php index 7bfe6e88..9fb10268 100644 --- a/lib/Controller/ApiBase.php +++ b/lib/Controller/ApiBase.php @@ -39,6 +39,7 @@ use OCP\IConfig; use OCP\IDBConnection; use OCP\IRequest; use OCP\IUserSession; +use Psr\Log\LoggerInterface; class ApiBase extends Controller { @@ -48,6 +49,7 @@ class ApiBase extends Controller protected IAppManager $appManager; protected TimelineQuery $timelineQuery; protected IDBConnection $connection; + protected LoggerInterface $logger; public function __construct( IRequest $request, @@ -55,7 +57,8 @@ class ApiBase extends Controller IUserSession $userSession, IDBConnection $connection, IRootFolder $rootFolder, - IAppManager $appManager + IAppManager $appManager, + LoggerInterface $logger ) { parent::__construct(Application::APPNAME, $request); @@ -64,6 +67,7 @@ class ApiBase extends Controller $this->connection = $connection; $this->rootFolder = $rootFolder; $this->appManager = $appManager; + $this->logger = $logger; $this->timelineQuery = new TimelineQuery($connection); } diff --git a/lib/Controller/VideoController.php b/lib/Controller/VideoController.php index 2d43be3d..d49af184 100644 --- a/lib/Controller/VideoController.php +++ b/lib/Controller/VideoController.php @@ -80,8 +80,20 @@ class VideoController extends ApiBase } // Request and check data was received - if (200 !== $this->getUpstream($client, $path, $profile)) { - return new JSONResponse(['message' => 'Transcode failed'], Http::STATUS_INTERNAL_SERVER_ERROR); + try { + $status = $this->getUpstream($client, $path, $profile); + if ($status === 409 || $status === -1) { + // Just a conflict (transcoding process changed) + return new JSONResponse(['message' => 'Conflict'], Http::STATUS_CONFLICT); + } + if ($status !== 200) { + throw new \Exception("Transcoder returned {$status}"); + } + } catch (\Exception $e) { + $msg = 'Transcode failed: '.$e->getMessage(); + $this->logger->error($msg, ['app' => 'memories']); + + return new JSONResponse(['message' => $msg], Http::STATUS_INTERNAL_SERVER_ERROR); } // The response was already streamed, so we have nothing to do here @@ -224,12 +236,15 @@ class VideoController extends ApiBase // Get transcoder path $transcoder = $this->config->getSystemValue('memories.transcoder', false); if (!$transcoder) { - return 0; + throw new \Exception('Transcoder not configured'); } // Make transcoder executable if (!is_executable($transcoder)) { @chmod($transcoder, 0755); + if (!is_executable($transcoder)) { + throw new \Exception("Transcoder not executable (chmod 755 {$transcoder})"); + } } // Kill the transcoder in case it's running @@ -271,20 +286,34 @@ class VideoController extends ApiBase $tmpPath .= $this->config->getSystemValue('instanceid', 'default'); // (Re-)create temp dir - shell_exec("rm -rf '{$tmpPath}'"); - mkdir($tmpPath, 0755, true); + shell_exec("rm -rf '{$tmpPath}' && mkdir -p '{$tmpPath}' && chmod 755 '{$tmpPath}'"); + + // Check temp directory exists + if (!is_dir($tmpPath)) { + throw new \Exception("Temp directory could not be created ({$tmpPath})"); + } + + // Check temp directory is writable + if (!is_writable($tmpPath)) { + throw new \Exception("Temp directory is not writable ({$tmpPath})"); + } // Set temp dir $env[] = "GOVOD_TEMPDIR='{$tmpPath}'"; // Start transcoder $env = implode(' ', $env); - shell_exec("{$env} nohup {$transcoder} > '{$tmpPath}.log' 2>&1 & > /dev/null"); + $logFile = $tmpPath.'.log'; + shell_exec("{$env} nohup {$transcoder} > '{$logFile}' 2>&1 & > /dev/null"); // wait for 1s and try again sleep(1); - return $this->getUpstreamInternal($client, $path, $profile); + $returnCode = $this->getUpstreamInternal($client, $path, $profile); + if (0 === $returnCode) { + throw new \Exception("Transcoder could not be started, check {$logFile}"); + } + return $returnCode; } private function getUpstreamInternal($client, $path, $profile) diff --git a/src/components/viewer/PsVideo.ts b/src/components/viewer/PsVideo.ts index 812407e4..baaa8603 100644 --- a/src/components/viewer/PsVideo.ts +++ b/src/components/viewer/PsVideo.ts @@ -170,7 +170,7 @@ class VideoContentSetup { }); const overrideNative = !vidjs.browser.IS_SAFARI; - content.videojs = vidjs(content.videoElement, { + const vjs = (content.videojs = vidjs(content.videoElement, { fill: true, autoplay: true, controls: false, @@ -186,24 +186,22 @@ class VideoContentSetup { nativeAudioTracks: !overrideNative, nativeVideoTracks: !overrideNative, }, - }); + })); - content.videojs.on("error", () => { - if (content.videojs.error().code === 4) { - if (content.videojs.src().includes("m3u8")) { - // HLS could not be streamed - console.error("Video.js: HLS stream could not be opened."); + vjs.on("error", () => { + if (vjs.error().code === 4 && vjs.src().includes("m3u8")) { + // HLS could not be streamed + console.error("Video.js: HLS stream could not be opened."); - if (getCurrentUser()?.isAdmin) { - showError(t("memories", "Transcoding failed.")); - } - - content.videojs.src({ - src: content.data.src, - type: "video/mp4", - }); - this.updateRotation(content, 0); + if (getCurrentUser()?.isAdmin) { + showError(t("memories", "Transcoding failed, check Nextcloud logs.")); } + + vjs.src({ + src: content.data.src, + type: "video/mp4", + }); + this.updateRotation(content, 0); } });