Skip to content

Release v4.0.0: Video Converter Presets & UI Improvements#13

Merged
skerbis merged 1 commit intomasterfrom
feature/4.0.0-presets-ui-improvements
Nov 19, 2025
Merged

Release v4.0.0: Video Converter Presets & UI Improvements#13
skerbis merged 1 commit intomasterfrom
feature/4.0.0-presets-ui-improvements

Conversation

@skerbis
Copy link
Member

@skerbis skerbis commented Nov 19, 2025

  • Add video converter presets (Web, Mobile, Archive, Standard)
  • Implement command preview functionality
  • Remove separate custom_command textarea for streamlined UI
  • Add comprehensive type hints and static analysis improvements
  • Fix preset override behavior and video mapping issues
  • Improve code quality with REDAXO core methods
  • Update README and version to 4.0.0

- Add video converter presets (Web, Mobile, Archive, Standard)
- Implement command preview functionality
- Remove separate custom_command textarea for streamlined UI
- Add comprehensive type hints and static analysis improvements
- Fix preset override behavior and video mapping issues
- Improve code quality with REDAXO core methods
- Update README and version to 4.0.0
Copilot AI review requested due to automatic review settings November 19, 2025 23:35
@skerbis skerbis merged commit 9a77b2d into master Nov 19, 2025
2 checks passed
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces version 4.0.0 with significant enhancements to the FFmpeg video converter addon, adding preset-based video conversion, command preview functionality, and improved code quality through comprehensive type hints and REDAXO core method adoption.

Key Changes:

  • Added video converter presets (Web, Mobile, Archive, Standard) with real-time command preview
  • Replaced deprecated mediapool functions with rex_media_service API
  • Enhanced video mapping to support cross-extension detection (e.g., MOV → MP4)

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
pages/mediapool.ffmpeg.config.php Implements preset selection UI with JavaScript preview functionality and form handling
lib/Api/Converter.php Adds type hints, replaces deprecated functions with rex_media_service, improves regex for extension extraction, adds debug endpoint
lib/VideoInfo.php Adds comprehensive type hints and return types, includes logging for FFmpeg operations
pages/mediapool.ffmpeg.main.php Improves video mapping logic to match files by basename for cross-extension support
package.yml Updates version to 4.0.0 and corrects default command formatting
lang/en_gb.lang Adds translation strings for preset configuration UI
lang/de_de.lang Adds German translation strings for preset configuration UI
assets/css/style.css Improves accessibility by replacing opacity with subtle background color
README.md Updates documentation for v4.0.0 features and improvements

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

$outputFile = $output . '.' . pathinfo($file, PATHINFO_EXTENSION);
// Extract output file extension from command, e.g. "OUTPUT.mp4" or "OUTPUT.mkv"
// Use a robust regex to match OUTPUT.<extension>
preg_match('/OUTPUT\.([^\s]+)/', $command, $extMatch);
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The regex pattern /OUTPUT\.([^\s]+)/ will capture any non-whitespace characters after OUTPUT., which could incorrectly match patterns like OUTPUT.mp4' (with trailing quote) or OUTPUT.mp4) (with closing parenthesis). Consider using a more specific pattern like /OUTPUT\.([a-zA-Z0-9]+)/ to match only alphanumeric extensions, or handle potential trailing characters in the captured extension.

Suggested change
preg_match('/OUTPUT\.([^\s]+)/', $command, $extMatch);
preg_match('/OUTPUT\.([a-zA-Z0-9]+)/', $command, $extMatch);

Copilot uses AI. Check for mistakes.
if (count($matches) > 0) {
$file = (trim($matches[0][0]));
$fileExt = pathinfo($file, PATHINFO_EXTENSION);
preg_match('/OUTPUT\.([^\s]+)/', $command, $extMatch);
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

Same issue as in handleStart(): The regex pattern /OUTPUT\.([^\s]+)/ could incorrectly capture non-alphanumeric characters as part of the file extension. Use a more specific pattern like /OUTPUT\.([a-zA-Z0-9]+)/ to match only valid file extensions.

Suggested change
preg_match('/OUTPUT\.([^\s]+)/', $command, $extMatch);
preg_match('/OUTPUT\.([a-zA-Z0-9]+)/', $command, $extMatch);

Copilot uses AI. Check for mistakes.
if ($syncResult) {
rex_file::put($log, sprintf("Destination file %s was successfully added to rex_mediapool", $outputFile) . PHP_EOL, FILE_APPEND);
// If addMedia didn't throw, we proceed with metadata and consider it successful.
$this->appendLog($log, sprintf("Destination file %s was successfully added to rex_mediapool", $outputFile) . PHP_EOL);
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

Indentation is inconsistent. This line is indented too far compared to the surrounding code (lines 774, 777-845). It should be at the same indentation level as the metadata handling code below.

Suggested change
$this->appendLog($log, sprintf("Destination file %s was successfully added to rex_mediapool", $outputFile) . PHP_EOL);
$this->appendLog($log, sprintf("Destination file %s was successfully added to rex_mediapool", $outputFile) . PHP_EOL);

Copilot uses AI. Check for mistakes.
$content .= " var sel = document.getElementById('ffmpeg-config-preset');" . PHP_EOL;
$content .= " var cmd = document.getElementById('ffmpeg-config-command');" . PHP_EOL;
$content .= " if (!sel || !cmd) { return; }" . PHP_EOL;
$content .= " var custom = null;" . PHP_EOL;
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The variable custom is declared but never used in the JavaScript. Consider removing this unused variable declaration to keep the code clean.

Suggested change
$content .= " var custom = null;" . PHP_EOL;

Copilot uses AI. Check for mistakes.
$n['field'] = $select->get();
$formElements[] = $n;

n['field'] = '<textarea class="form-control" id="ffmpeg-config-custom" name="config[custom_command]" rows="3">' . htmlspecialchars((string) $this->getConfig('custom_command')) . '</textarea>';
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

Missing $ before variable name in assignment. This line should be $n['field'] = ... but the $ is missing, which will cause a fatal PHP error.

Suggested change
n['field'] = '<textarea class="form-control" id="ffmpeg-config-custom" name="config[custom_command]" rows="3">' . htmlspecialchars((string) $this->getConfig('custom_command')) . '</textarea>';
$n['field'] = '<textarea class="form-control" id="ffmpeg-config-custom" name="config[custom_command]" rows="3">' . htmlspecialchars((string) $this->getConfig('custom_command')) . '</textarea>';

Copilot uses AI. Check for mistakes.
Comment on lines +754 to +764
// Replace deprecated rex_mediapool_syncFile with rex_media_service::addMedia
$syncData = [
'file' => [
'name' => pathinfo($outputFile, PATHINFO_BASENAME),
'path' => $outputFile,
],
'category_id' => 0,
'title' => '',
];
try {
$syncResultResult = \rex_media_service::addMedia($syncData, false);
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The class \rex_media_service does not appear to exist in the REDAXO core. The correct method for adding media is rex_mediapool_syncFile() (as used in the previous version) or rex_media_service::addMedia() if using a newer API. Please verify that \rex_media_service::addMedia() exists in the REDAXO version specified in requirements (5.18.1+), as this will cause a fatal error if the class doesn't exist.

Copilot uses AI. Check for mistakes.
Comment on lines +763 to +775
try {
$syncResultResult = \rex_media_service::addMedia($syncData, false);
$syncResult = !empty($syncResultResult['ok']);
} catch (\rex_api_exception $e) {
$this->appendLog($log, 'rex_media_service::addMedia failed: ' . $e->getMessage() . PHP_EOL);
rex_logger::factory()->log('warning', 'rex_media_service::addMedia failed: {exception}', ['exception' => $e->getMessage()]);
$syncResult = false;
}
rex_unset_session('ffmpeg_output_video_file');

if ($syncResult) {
rex_file::put($log, sprintf("Destination file %s was successfully added to rex_mediapool", $outputFile) . PHP_EOL, FILE_APPEND);
// If addMedia didn't throw, we proceed with metadata and consider it successful.
$this->appendLog($log, sprintf("Destination file %s was successfully added to rex_mediapool", $outputFile) . PHP_EOL);
rex_logger::factory()->log('info', 'Destination file {file} was successfully added to media pool', ['file' => $outputFile, 'conversion_id' => $conversionId]);
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The variable $syncResult is assigned but never used after line 769. The code proceeds unconditionally with "If addMedia didn't throw, we proceed with metadata" (line 773), regardless of whether $syncResult is true or false. This means conversion will be marked as successful even if rex_media_service::addMedia() returns a failure status without throwing an exception.

Copilot uses AI. Check for mistakes.
if (!empty($inputFile) && rex_addon::get('ffmpeg')->getConfig('delete') == 1) {
rex_mediapool_deleteMedia(pathinfo($inputFile, PATHINFO_BASENAME));
try {
\rex_media_service::deleteMedia(pathinfo($inputFile, PATHINFO_BASENAME));
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

Similar to the addMedia issue, \rex_media_service::deleteMedia() may not exist in REDAXO core. The previous version used rex_mediapool_deleteMedia(). Please verify this API exists in the target REDAXO version, as this will cause a fatal error if the class/method doesn't exist.

Suggested change
\rex_media_service::deleteMedia(pathinfo($inputFile, PATHINFO_BASENAME));
rex_mediapool_deleteMedia(pathinfo($inputFile, PATHINFO_BASENAME));

Copilot uses AI. Check for mistakes.
Comment on lines +473 to +474
$this->appendLog($log, 'Konvertierung für "' . $video . '" gestartet um ' . date('d.m.Y H:i:s') . "\n");
$this->appendLog($log, 'Kommando: ' . $command . "\n\n");
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

Indentation is incorrect. These lines (473-474) have extra leading whitespace compared to the surrounding code. They should be aligned with line 472 and 476.

Suggested change
$this->appendLog($log, 'Konvertierung für "' . $video . '" gestartet um ' . date('d.m.Y H:i:s') . "\n");
$this->appendLog($log, 'Kommando: ' . $command . "\n\n");
$this->appendLog($log, 'Konvertierung für "' . $video . '" gestartet um ' . date('d.m.Y H:i:s') . "\n");
$this->appendLog($log, 'Kommando: ' . $command . "\n\n");

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments