Skip to content

Fix Video Processor when TmpStore is missing #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
solverat merged 1 commit into from
Feb 25, 2026
Merged

Fix Video Processor when TmpStore is missing #95

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 37 additions & 19 deletions models/Asset/Video/Thumbnail/Processor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public static function process(Model\Asset\Video $asset, Config $config, array $

$instance = new self();
$formats = $onlyFormats === [] ? ['mp4'] : $onlyFormats;
$instance->setProcessId(uniqid());
$instance->setProcessId(uniqid('', false));
$instance->setAssetId($asset->getId());
$instance->setConfig($config);

Expand All @@ -82,11 +82,11 @@ public static function process(Model\Asset\Video $asset, Config $config, array $
$customSetting = $asset->getCustomSetting('thumbnails');
$existingFormats = [];
if (is_array($customSetting) && array_key_exists($config->getName(), $customSetting)) {
if ($customSetting[$config->getName()]['status'] == 'inprogress') {
if ($customSetting[$config->getName()]['status'] === 'inprogress') {
if (TmpStore::get($instance->getJobStoreId($customSetting[$config->getName()]['processId']))) {
return null;
}
} elseif ($customSetting[$config->getName()]['status'] == 'finished') {
} elseif ($customSetting[$config->getName()]['status'] === 'finished') {
// check if the files are there
$formatsToConvert = [];
foreach ($formats as $f) {
Expand All @@ -103,7 +103,7 @@ public static function process(Model\Asset\Video $asset, Config $config, array $
} else {
return null;
}
} elseif ($customSetting[$config->getName()]['status'] == 'error') {
} elseif ($customSetting[$config->getName()]['status'] === 'error') {
throw new Exception('Unable to convert video, see logs for details.');
}
}
Expand All @@ -122,7 +122,7 @@ public static function process(Model\Asset\Video $asset, Config $config, array $
$converter->setStorageFile($storagePath);

//add media queries for mpd file generation
if ($format == 'mpd') {
if ($format === 'mpd') {
$medias = $config->getMedias();
foreach ($medias as $media => $transformations) {
//used just to generate arguments for medias
Expand All @@ -148,11 +148,10 @@ public static function process(Model\Asset\Video $asset, Config $config, array $
'formats' => $existingFormats,
'processId' => $instance->getProcessId(),
];

$asset->setCustomSetting('thumbnails', $customSetting);

Model\Version::disable();
$asset->save();
Model\Version::enable();
self::saveWithoutVersion($asset);

$instance->save();

Expand Down Expand Up @@ -197,19 +196,25 @@ public static function execute(string $processId, ?int $assetId = null): void

$instanceItem = TmpStore::get($instance->getJobStoreId($processId));

if (!$instanceItem) {
if (!$instanceItem instanceof TmpStore) {
Logger::error('Video conversion job with processId "' . $processId . '" not found in TmpStore.');
if ($assetId) {
$asset = Model\Asset::getById($assetId);
if (!$asset instanceof Model\Asset\Video) {
return;
}
$customSetting = $asset->getCustomSetting('thumbnails');
$customSetting = is_array($customSetting) ? $customSetting : [];
if (array_key_exists($instance->getConfig()->getName(), $customSetting)) {
$customSetting[$instance->getConfig()->getName()]['status'] = 'error';
$changed = false;
foreach ($customSetting as $configName => $setting) {
if (($setting['processId'] ?? null) === $processId && $setting['status'] === 'inprogress') {
$customSetting[$configName]['status'] = 'error';
$changed = true;
}
}
if ($changed) {
$asset->setCustomSetting('thumbnails', $customSetting);

Model\Version::disable();
$asset->save();
Model\Version::enable();
self::saveWithoutVersion($asset);
}
}

Expand Down Expand Up @@ -248,6 +253,7 @@ public static function execute(string $processId, ?int $assetId = null): void

continue;
}

Storage::get('thumbnail')->writeStream($converter->getStorageFile(), $source);

fclose($source);
Expand Down Expand Up @@ -302,11 +308,9 @@ public static function execute(string $processId, ?int $assetId = null): void
'status' => $conversionStatus,
'formats' => $formats,
];
$asset->setCustomSetting('thumbnails', $customSetting);

Model\Version::disable();
$asset->save();
Model\Version::enable();
$asset->setCustomSetting('thumbnails', $customSetting);
self::saveWithoutVersion($asset);
}

@unlink($workerSourceFile);
Expand All @@ -321,6 +325,20 @@ public function save(): bool
return true;
}

private static function saveWithoutVersion(Model\Asset $asset): void
{
$versioningEnabled = Model\Version::isEnabled();
if ($versioningEnabled) {
Model\Version::disable();
}

$asset->save();

if ($versioningEnabled) {
Model\Version::enable();
}
Comment on lines +330 to +339
Copy link
Contributor

@blankse blankse Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$versioningEnabled = Model\Version::isEnabled();
if ($versioningEnabled) {
Model\Version::disable();
}
$asset->save();
if ($versioningEnabled) {
Model\Version::enable();
}
$versioningEnabled = Model\Version::isEnabled();
try {
if ($versioningEnabled) {
Model\Version::disable();
}
$asset->save();
} finally {
if ($versioningEnabled) {
Model\Version::enable();
}
}

Copy link
Contributor Author

@solverat solverat Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blankse Oh no, that was too close! Your proposal makes perfect sense. Could you please submit another PR for that?

Copy link
Contributor Author

@solverat solverat Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blankse wait, i just have seen this:

https://github.com/search?q=repo%3Aopen-dxp%2Fopendxp%20version%3A%3Aenable()&type=code

maybe we should change this globally...

}

protected function getJobStoreId(?string $processId = null): string
{
if (!$processId) {
Expand Down