FFMPeg handling is very inefficient

[Kirby]

Affected version

2.2.4

\XFMG\Attachment\Media::onNewAttachment

    public function onNewAttachment(Attachment $attachment, \XF\FileWrapper $file)
    {
        /** @var \XFMG\Service\Media\TempCreator $tempCreator */
        $tempCreator = \XF::app()->service('XFMG:Media\TempCreator');
        $tempCreator->setAttachment($attachment);
        if ($file->getExif())
        {
            $tempCreator->setExif($file->getExif());
        }
        $tempCreator->save();
    }

When this is called, the attachment upload has been completed, the file has been copied to data and the original uploaded file does still exist.

\XFMG\Service\Media\TempCreator::_save

protected function _save()
{
    $mediaTemp = $this->mediaTemp;
    $mediaTemp->save();

    $abstractedThumbnailPath = $mediaTemp->getAbstractedTempThumbnailPath();
    $abstractedPosterPath = $mediaTemp->getAbstractedTempPosterPath();

    $thumbnailDate = 0;
    $posterDate = 0;

    /** @var \XFMG\Service\Media\ThumbnailGenerator $thumbnailGenerator */
    $thumbnailGenerator = $this->service('XFMG:Media\ThumbnailGenerator');

    $updates = [];

    if ($this->attachment)
    {
        $attachment = $this->attachment;

        $updates += [
            'title' => $attachment->Data->filename,
            'attachment_id' => $attachment->attachment_id
        ];

        if ($thumbnailGenerator->createTempThumbnailFromAttachment($attachment, $abstractedThumbnailPath, $mediaTemp->media_type))
        {
            $thumbnailDate = time();
        }

        if ($thumbnailGenerator->createTempPosterFromAttachment($attachment, $abstractedPosterPath, $mediaTemp->media_type))
        {
            $posterDate = time();
        }

This code generates poster and tumbnail by calling methods XFMG\Service\Media\ThumbnailGenerator::createTempThumbnailFromAttachment and
XFMG\Service\Media\ThumbnailGenerator::createTempPosterFromAttachment respectively.

Both methods then call
XFMG\Service\Media\ThumbnailGenerator::getTempFrameFromFfMpeg

    public function getTempFrameFromFfMpeg($abstractedSourcePath, $mediaType)
    {
        if (isset($this->cachedFrames[$abstractedSourcePath]))
        {
            return $this->cachedFrames[$abstractedSourcePath];
        }

        $sourceFile = \XF\Util\File::copyAbstractedPathToTempFile($abstractedSourcePath);

This does copy (copy #1) the uploaded video teo a temporary file.

Further down in \XFMG\Service\Media\TempCreator::_save another copy (copy #2) gets created

if ($mediaTemp->media_type == 'video')
{
    $abstractedPath = $attachment->Data->getAbstractedDataPath();
    $tempPath = \XF\Util\File::copyAbstractedPathToTempFile($abstractedPath);

So in total 2 additional copies of the uploaded video are taken, even though a temporary file with the same content does already exist at [icode[$_FILES['upload']['tmp_name'][/icode].
This does not only take up 3x the filesize of the video for temporary files during upload processing, it also does waste traffic if data is on external storage (S3, etc.)

This should be changed so the original upload tmp is used for all those operations (generating thumb, poster & getting metadata).

 

[Kirby]

This should be changed so the original upload tmp is used for all those operations (generating thumb, poster & getting metadata).

... at least if it happens for the upload.

There could be situations where ThumbnailGenerator is used well after the upload.

In this case it still seems like an insane waste of resources to copy the whole file from abstracted storage to a temporary file just to extract a frame from the first 10 seconds of the video (and build a thumbnail / poster from that).

A better approach could be to pass the stream (from FlySystem) to FFMPeg via stdin or maybe just copy the first few MB to the temporary file (should be well enough to extract the first frame).