Skip to content

Commit 14b91cd

Browse files
invarioinvario
committed
enhancement(previews): allow ffmpeg to connect direct for AWS S3 buckets
Signed-off-by: invario <67800603+invario@users.noreply.github.com>
1 parent ec73f4a commit 14b91cd

File tree

2 files changed

+82
-35
lines changed

2 files changed

+82
-35
lines changed

‎apps/files_external/lib/Lib/Storage/AmazonS3.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/**
34
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
45
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -756,4 +757,29 @@ public function writeStream(string $path, $stream, ?int $size = null): int {
756757

757758
return $size;
758759
}
760+
761+
/**
762+
* Generates and returns a presigned URL that expires after 1 minute.
763+
*
764+
*/
765+
public function getDirectDownload(string $path): array|false {
766+
$command = $this->getConnection()->getCommand('GetObject', [
767+
'Bucket' => $this->bucket,
768+
'Key' => $path,
769+
]);
770+
// generate a presigned URL that expires after 1 minute
771+
$request = $this->getConnection()->createPresignedRequest($command, '+1 minute', []);
772+
try {
773+
$presignedUrl = (string)$request->getUri();
774+
} catch (S3Exception $exception) {
775+
$this->logger->error($exception->getMessage(), [
776+
'app' => 'files_external',
777+
'exception' => $exception,
778+
]);
779+
}
780+
$result = [
781+
'url' => $presignedUrl
782+
];
783+
return $result;
784+
}
759785
}

‎lib/private/Preview/Movie.php‎

Lines changed: 56 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,21 @@ public function isAvailable(FileInfo $file): bool {
4242
return is_string($this->binary);
4343
}
4444

45+
private function connectDirect(File $file) {
46+
// Checks for availability to access the video file directly via HTTP/HTTPS.
47+
// Returns a string containing URL if available. Only implemented and tested
48+
// with Amazon S3 currently. In all other cases, return false. ffmpeg
49+
// supports other protocols so this function may expand in the future.
50+
$gddValues = $file->getStorage()->getDirectDownload($file->getName());
51+
if (is_array($gddValues)) {
52+
if (array_key_exists('url', $gddValues)) {
53+
$directUrl = str_starts_with($gddValues['url'], 'http') ? $gddValues['url'] : false;
54+
return $directUrl;
55+
}
56+
}
57+
return false;
58+
}
59+
4560
/**
4661
* {@inheritDoc}
4762
*/
@@ -51,54 +66,60 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
5166
if (!$this->isAvailable($file)) {
5267
return null;
5368
}
54-
5569
$result = null;
56-
if ($this->useTempFile($file)) {
57-
// Try downloading 5 MB first, as it's likely that the first frames are present there.
58-
// In some cases this doesn't work, for example when the moov atom is at the
59-
// end of the file, so if it fails we fall back to getting the full file.
60-
// Unless the file is not local (e.g. S3) as we do not want to download the whole (e.g. 37Gb) file
61-
if ($file->getStorage()->isLocal()) {
62-
$sizeAttempts = [5242880, null];
70+
$connectDirect = $this->connectDirect($file);
71+
if (($connectDirect === false) || $file->isEncrypted()) {
72+
// If HTTP/HTTPS direct connect is not available or if the file is encrypted,
73+
// process normally with temp files
74+
if ($this->useTempFile($file)) {
75+
// Try downloading 5 MB first, as it's likely that the first frames are
76+
// present there. In some cases this doesn't work (e.g. when the
77+
// moov atom is at the end) so if it fails, fall back to
78+
// getting the full file, unless the file is not local as we do not want
79+
// to download the whole (e.g. 37GB) file from remote.
80+
if ($file->getStorage()->isLocal()) {
81+
$sizeAttempts = [5242880, null];
82+
} else {
83+
$sizeAttempts = [5242880];
84+
}
6385
} else {
64-
$sizeAttempts = [5242880];
86+
// size is irrelevant, only attempt once
87+
$sizeAttempts = [null];
6588
}
66-
} else {
67-
// size is irrelevant, only attempt once
68-
$sizeAttempts = [null];
69-
}
70-
71-
foreach ($sizeAttempts as $size) {
72-
$absPath = $this->getLocalFile($file, $size);
73-
if ($absPath === false) {
74-
Server::get(LoggerInterface::class)->error(
75-
'Failed to get local file to generate thumbnail for: ' . $file->getPath(),
76-
['app' => 'core']
77-
);
78-
return null;
79-
}
80-
81-
$result = $this->generateThumbNail($maxX, $maxY, $absPath, 5);
82-
if ($result === null) {
83-
$result = $this->generateThumbNail($maxX, $maxY, $absPath, 1);
84-
if ($result === null) {
85-
$result = $this->generateThumbNail($maxX, $maxY, $absPath, 0);
89+
90+
foreach ($sizeAttempts as $size) {
91+
$absPath = $this->getLocalFile($file, $size);
92+
if ($absPath === false) {
93+
Server::get(LoggerInterface::class)->error(
94+
'Failed to get local file to generate thumbnail for: ' .
95+
$file->getPath(), ['app' => 'core']
96+
);
97+
return null;
8698
}
87-
}
8899

89-
$this->cleanTmpFiles();
100+
$result = ($this->generateThumbNail($maxX, $maxY, $absPath, 5)) ??
101+
($this->generateThumbNail($maxX, $maxY, $absPath, 1)) ??
102+
($this->generateThumbNail($maxX, $maxY, $absPath, 0));
103+
104+
$this->cleanTmpFiles();
90105

91-
if ($result !== null) {
92-
break;
106+
if ($result !== null) {
107+
break;
108+
}
93109
}
110+
} else {
111+
// HTTP/HTTPS direct connect is available so pass the URL directly to ffmpeg
112+
$result = ($this->generateThumbNail($maxX, $maxY, $connectDirect, 5)) ??
113+
($this->generateThumbNail($maxX, $maxY, $connectDirect, 1)) ??
114+
($this->generateThumbNail($maxX, $maxY, $connectDirect, 0));
94115
}
95-
96116
return $result;
97117
}
98118

99119
private function useHdr(string $absPath): bool {
100120
// load ffprobe path from configuration, otherwise generate binary path using ffmpeg binary path
101-
$ffprobe_binary = $this->config->getSystemValue('preview_ffprobe_path', null) ?? (pathinfo($this->binary, PATHINFO_DIRNAME) . '/ffprobe');
121+
$ffprobe_binary = ($this->config->getSystemValue('preview_ffprobe_path', null)) ??
122+
(pathinfo($this->binary, PATHINFO_DIRNAME) . '/ffprobe');
102123
// run ffprobe on the video file to get value of "color_transfer"
103124
$test_hdr_cmd = [$ffprobe_binary,'-select_streams', 'v:0',
104125
'-show_entries', 'stream=color_transfer',

0 commit comments

Comments
 (0)