Skip to content
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.

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

Added dimension properties to video media #4357

Merged
merged 3 commits into from Jan 21, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+17 −2
Diff settings

Always

Just for now

Copy path View file
@@ -1,6 +1,9 @@
CHANGELOG for Sulu
==================

* dev-master
* FEATURE #4357 [MediaBundle] Added dimension properties to video media

* 1.6.24 (2019-01-09)
* BUGFIX #4349 [ContentBundle] Fix compatibility to symfony 3.4.21, 4.1.10 and 4.2.2
* ENHANCEMENT #4319 [MediaBundle] Added possibility to have a image format configuration file without formats
@@ -307,12 +307,24 @@ private function getProperties(UploadedFile $uploadedFile)
$properties = [];
try {
// if the file is a video we add the duration
// if the file is a video we add the properties related to it
if (fnmatch('video/*', $mimeType)) {
// Duration
$properties['duration'] = $this->ffprobe->format($uploadedFile->getPathname())->get('duration');
// Dimensions
try {
$dimensions = $this->ffprobe->streams($uploadedFile->getPathname())->videos()->first()->getDimensions();
$properties['width'] = $dimensions->getWidth();
$properties['height'] = $dimensions->getHeight();
} catch (InvalidArgumentException $e) {
// Exception is thrown if the video stream could not be obtained
} catch (RuntimeException $e) {
This conversation was marked as resolved by danrot

This comment has been minimized.

Copy link
@danrot

danrot Jan 21, 2019

Member

Isn't there a more specific exception that could be caught? This might hide all different kind of exceptions...

This comment has been minimized.

Copy link
@Leobaillard

Leobaillard Jan 21, 2019

Author Contributor

Unfortunately, I don't think so. Looking at the ffmpeg lib, it seems that RuntimeException is what the different methods used to get the video's dimensions throw:

We could also catch the LogicException from FFProbe::getDimensions but it should not be triggered as we only query video streams. In any case, this would not suffice to be more precise on exception catching.

If you think this is too broad, I can try formatting the code another way. I'm open to suggestions. Thanks for your review!

This comment has been minimized.

Copy link
@danrot

danrot Jan 21, 2019

Member

Well, the only thing we could do is check the message of the exception, and that's also not nice... I guess we let it that way in this case.

// Exception is thrown if the dimension could not be extracted
}
}
} catch (ExecutableNotFoundException $e) {
// Exception is thrown if ffmpeg is not installed -> duration is not set
// Exception is thrown if ffmpeg is not installed -> video properties are not set
}
return $properties;
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.