constants/base.py

@@ -103,9 +103,10 @@ ABUSE_TYPE = Choices(
 
 # **N.B.**: thumbnail sizes are not intended to be changed on the fly:
 # thumbnails of existing images must exist in MEDIA_ROOT before
-# the code expecting the new dimensions can be deployed!
-THUMBNAIL_SIZE_DEFAULT = (1920, 1080)
+# the code expecting thumbnails of new dimensions can be deployed!
+THUMBNAIL_SIZE_L = (1920, 1080)
 THUMBNAIL_SIZE_S = (640, 360)
-THUMBNAIL_SIZES = [THUMBNAIL_SIZE_DEFAULT, THUMBNAIL_SIZE_S]
+THUMBNAIL_SIZES = [THUMBNAIL_SIZE_L, THUMBNAIL_SIZE_S]
+THUMBNAIL_META = {'l': THUMBNAIL_SIZE_L, 's': THUMBNAIL_SIZE_S}
 THUMBNAIL_FORMAT = 'PNG'
 THUMBNAIL_QUALITY = 83

extensions/templates/extensions/components/card.html

@@ -1,13 +1,13 @@
 {% load common filters %}
-{% with latest=extension.latest_version thumbnail_url=extension.previews.listed.first.thumbnail_url %}
+{% with latest=extension.latest_version thumbnail_s_url=extension.previews.listed.first.thumbnail_s_url %}
 
 <div class="ext-card {% if blur %}is-background-blur{% endif %}">
   {% if blur %}
-  <div class="ext-card-thumbnail-blur" style="background-image: url({{ thumbnail_url }});"></div>
+  <div class="ext-card-thumbnail-blur" style="background-image: url({{ thumbnail_s_url }});"></div>
   {% endif %}
 
   <a class="ext-card-thumbnail" href="{{ extension.get_absolute_url }}">
-    <div class="ext-card-thumbnail-img" style="background-image: url({{ thumbnail_url }});" title="{{ extension.name }}"></div>
+    <div class="ext-card-thumbnail-img" style="background-image: url({{ thumbnail_s_url }});" title="{{ extension.name }}"></div>
   </a>
 
   <div class="ext-card-body">

extensions/templates/extensions/components/galleria.html

@@ -3,15 +3,15 @@
   {% if previews %}
   <div class="galleria-items{% if previews.count > 5 %} is-many{% endif %}{% if previews.count == 1 %} is-single{% endif %}" id="galleria-items">
     {% for preview in previews %}
-      {% with thumbnail_url=preview.thumbnail_url %}
+      {% with thumbnail_l_url=preview.thumbnail_l_url %}
       <a
         class="galleria-item js-galleria-item-preview galleria-item-type-{{ preview.content_type|slugify|slice:5 }}{% if forloop.first %} is-active{% endif %}"
-        href="{{ thumbnail_url }}"
+        href="{{ thumbnail_l_url }}"
         {% if 'video' in preview.content_type %}data-galleria-video-url="{{ preview.source.url }}"{% endif %}
         data-galleria-content-type="{{ preview.content_type }}"
         data-galleria-index="{{ forloop.counter }}">
 
-        <img src="{{ thumbnail_url }}" alt="{{ preview.extension_preview.first.caption }}">
+        <img src="{{ thumbnail_l_url }}" alt="{{ preview.extension_preview.first.caption }}">
         </a>
       {% endwith %}
     {% endfor %}

files/admin.py

@@ -7,7 +7,7 @@ import background_task.admin
 import background_task.models
 
 from .models import File, FileValidation
-from constants.base import THUMBNAIL_SIZE_DEFAULT
+from constants.base import THUMBNAIL_SIZE_L
 import files.signals
 
 logger = logging.getLogger(__name__)
@@ -21,7 +21,7 @@ def scan_selected_files(self, request, queryset):
 
 def make_thumbnails(self, request, queryset):
     """Make thumbnails for selected files."""
-    for instance in queryset.filter(type__in=(File.TYPES.IMAGE, File.TYPE.VIDEO)):
+    for instance in queryset.filter(type__in=(File.TYPES.IMAGE, File.TYPES.VIDEO)):
         files.tasks.make_thumbnails.task_function(file_id=instance.pk)
 
 
@@ -48,10 +48,9 @@ class FileAdmin(admin.ModelAdmin):
             return ''
         try:
             context = {
+                'file': obj,
                 'MEDIA_URL': settings.MEDIA_URL,
-                'THUMBNAIL_SIZE_DEFAULT': THUMBNAIL_SIZE_DEFAULT,
-                'thumbnail': obj.thumbnail,
-                'thumbnails': obj.thumbnails,
+                'THUMBNAIL_SIZE_L': THUMBNAIL_SIZE_L,
             }
             return render_to_string('files/admin/thumbnails.html', context)
         except Exception:

files/migrations/0008_alter_file_thumbnail.py

@@ -0,0 +1,19 @@
+# Generated by Django 4.2.11 on 2024-04-23 10:31
+
+from django.db import migrations, models
+import files.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('files', '0007_alter_file_status'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='file',
+            name='thumbnail',
+            field=models.ImageField(blank=True, editable=False, help_text='Thumbnail generated from uploaded image or video source file', max_length=256, null=True, upload_to=files.models.thumbnail_upload_to),
+        ),
+    ]

files/models.py

@@ -6,13 +6,8 @@ from django.contrib.auth import get_user_model
 from django.db import models
 
 from common.model_mixins import CreatedModifiedMixin, TrackChangesMixin
-from files.utils import get_sha256, guess_mimetype_from_ext, get_base_path
-from constants.base import (
-    FILE_STATUS_CHOICES,
-    FILE_TYPE_CHOICES,
-    THUMBNAIL_FORMAT,
-    THUMBNAIL_SIZES,
-)
+from files.utils import get_sha256, guess_mimetype_from_ext, get_thumbnail_upload_to
+from constants.base import FILE_STATUS_CHOICES, FILE_TYPE_CHOICES
 import utils
 
 User = get_user_model()
@@ -43,11 +38,7 @@ def file_upload_to(instance, filename):
 
 
 def thumbnail_upload_to(instance, filename):
-    prefix = 'thumbnails/'
-    _hash = instance.hash.split(':')[-1]
-    extension = f'.{THUMBNAIL_FORMAT.lower()}'
-    path = Path(prefix, _hash[:2], _hash).with_suffix(extension)
-    return path
+    return get_thumbnail_upload_to(instance.hash)
 
 
 class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
@@ -65,7 +56,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
         null=True,
         blank=True,
         max_length=256,
-        help_text='Image thumbnail',
+        help_text='Thumbnail generated from uploaded image or video source file',
         editable=False,
     )
     content_type = models.CharField(max_length=256, null=True, blank=True)
@@ -100,7 +91,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
         default=dict,
         blank=True,
         # TODO add link to the manifest file user manual page.
-        help_text=('Meta information that was parsed from the manifest file.'),
+        help_text=('Meta information that was parsed from the `manifest file.'),
     )
 
     objects = FileManager()
@@ -207,20 +198,22 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
         return self.extension.get_draft_url()
 
     @property
-    def thumbnails(self) -> dict:
-        """Return a dictionary with relative URLs of a set of predefined thumbnails."""
-        base_path = get_base_path(thumbnail_upload_to(self, ''))
-        extension = f'.{THUMBNAIL_FORMAT.lower()}'
-        return {size: f'{base_path}_{size[0]}x{size[1]}{extension}' for size in THUMBNAIL_SIZES}
-
-    @property
-    def thumbnail_url(self) -> str:
+    def thumbnail_l_url(self) -> str:
         """Log absence of the thumbnail file instead of exploding somewhere in the templates."""
         try:
             return self.thumbnail.url
         except ValueError:
-            log.exception(f'File pk={self.pk} is missing thumbnail(s)')
-        return ''
+            log.exception(f'File pk={self.pk} is missing a large thumbnail')
+        return self.source.url
+
+    @property
+    def thumbnail_s_url(self) -> str:
+        """Log absence of the thumbnail file instead of exploding somewhere in the templates."""
+        try:
+            return self.metadata['thumbnails']['s']['path']
+        except KeyError:
+            log.exception(f'File pk={self.pk} is missing a small thumbnail')
+        return self.source.url
 
 
 class FileValidation(CreatedModifiedMixin, TrackChangesMixin, models.Model):

files/signals.py

@@ -66,5 +66,6 @@ def _log_deletion(sender: object, instance: files.models.File, **kwargs: object)
 @receiver(post_delete, sender=files.models.File)
 def delete_orphaned_files(sender: object, instance: files.models.File, **kwargs: object) -> None:
     """Delete source and thumbnail files from storage when File record is deleted."""
-    files.utils.delete_source_file(instance.source.name)
-    files.utils.delete_thumbnail_file(instance.thumbnail.name)
+    files.utils.delete_file_in_storage(instance.source.name)
+    files.utils.delete_file_in_storage(instance.thumbnail.name)
+    files.utils.delete_thumbnails(instance.metadata)

files/tasks.py

@@ -5,7 +5,7 @@ from background_task import background
 from background_task.tasks import TaskSchedule
 from django.conf import settings
 
-from constants.base import THUMBNAIL_FORMAT, THUMBNAIL_SIZE_DEFAULT, THUMBNAIL_QUALITY
+from constants.base import THUMBNAIL_FORMAT, THUMBNAIL_SIZE_L, THUMBNAIL_QUALITY, THUMBNAIL_META
 import files.models
 import files.utils
 
@@ -34,36 +34,42 @@ def clamdscan(file_id: int):
 def make_thumbnails(file_id: int) -> None:
     """Generate thumbnails for a given file."""
     file = files.models.File.objects.get(pk=file_id)
-    abs_path = os.path.join(settings.MEDIA_ROOT, file.source.path)
+    args = {'pk': file_id, 'type': file.get_type_display()}
+    if not file.is_image and not file.is_video:
+        logger.warning('File pk=%(pk)s is neither an image nor a video ("%(type)s")', args)
+        return
 
-    # TODO: For an image, source of the thumbnail is file.source itself
-    if file.is_image:
+    assert file.validation.is_ok, f'File pk={file_id} is flagged'
+
+    source_path = os.path.join(settings.MEDIA_ROOT, file.source.path)
     thumbnail_field = file.thumbnail
-        if not thumbnail_field:
-            # Use the source image if max-size thumbnail is not yet available
-            thumbnail_field.name = file.source.name
     thumbnails_paths = file.thumbnails
+    thumbnail_default_path = thumbnails_paths[THUMBNAIL_SIZE_L]
+
+    # For a video, source of the thumbnail is some frame fetched with ffpeg
+    if file.is_video:
+        output_path = os.path.join(settings.MEDIA_ROOT, thumbnail_default_path)
+        files.utils.extract_frame(source_path, output_path)
+        source_path = output_path
+
     files.utils.make_thumbnails(
-            abs_path,
+        source_path,
         thumbnails_paths,
         output_format=THUMBNAIL_FORMAT,
         quality=THUMBNAIL_QUALITY,
         optimize=True,
         progressive=True,
     )
-        thumbnail_default_path = thumbnails_paths[THUMBNAIL_SIZE_DEFAULT]
-        # Reverse to make JSON-serialisable
-        thumbnail_metadata = {path: size for size, path in thumbnails_paths.items()}
-        update_fields = {}
+
+    update_fields = set()
     if thumbnail_default_path != thumbnail_field.name:
         thumbnail_field.name = thumbnail_default_path
         update_fields.add('thumbnail')
+    thumbnail_metadata = {
+        key: {'size': size, 'path': thumbnails_paths[size]} for key, size in THUMBNAIL_META.items()
+    }
     if file.metadata.get('thumbnails') != thumbnail_metadata:
         file.metadata.update({'thumbnails': thumbnail_metadata})
         update_fields.add('metadata')
     if update_fields:
         file.save(update_fields=update_fields)
-    # TODO: For a video, source of the thumbnail is some frame fetched with ffpeg
-    elif file.is_video:
-        raise NotImplementedError
-    # TODO: file.thumbnail field is updated

files/templates/files/admin/thumbnails.html

@@ -1,14 +1,14 @@
 <div class="file-thumbnails">
-{% for size, thumb_url in thumbnails.items %}
+{% for key, thumb in file.metadata.thumbnails.items %}
   <div class="file-thumbnail">
-    <span class="file-thumbnail-size">{{ size.0 }}x{{ size.1 }}px</span>
-    <img height="{% widthratio size.1 10 1 %}" src="{{ MEDIA_URL }}{{ thumb_url }}" title={{ thumb_url }}>
+    <span class="file-thumbnail-size">{{ thumb.size.0 }}x{{ thumb.size.1 }}px</span>
+    <img height="{% widthratio thumb.size.1 10 1 %}" src="{{ MEDIA_URL }}{{ thumb.path }}" title={{ thumb.path }}>
   </div>
 {% endfor %}
-{% if thumbnail.width != THUMBNAIL_SIZE_DEFAULT.0 or thumbnail.height != THUMBNAIL_SIZE_DEFAULT.1 %}
+{% if file.thumbnail.width != THUMBNAIL_SIZE_L.0 or file.thumbnail.height != THUMBNAIL_SIZE_L.1 %}
   <p>
-    <b class="icon-alert">⚠</b> Expected {{ THUMBNAIL_SIZE_DEFAULT.0 }}x{{ THUMBNAIL_SIZE_DEFAULT.1 }}px,
-    got {{ thumbnail.width }}x{{ thumbnail.height }}px: resolution of the source file might be too low?
+    <b class="icon-alert">⚠</b> Expected {{ THUMBNAIL_SIZE_L.0 }}x{{ THUMBNAIL_SIZE_L.1 }}px,
+    got {{ file.thumbnail.width }}x{{ file.thumbnail.height }}px: resolution of the source file might be too low?
   </p>
 {% endif %}
 </div>

files/utils.py

@@ -12,6 +12,7 @@ import zipfile
 
 from PIL import Image
 from django.core.files.storage import default_storage
+from ffmpeg import FFmpeg, FFmpegFileNotFound, FFmpegInvalidCommand, FFmpegError
 from lxml import etree
 import clamd
 import magic
@@ -202,7 +203,7 @@ def make_thumbnails(image_field, output_paths: dict, output_format: str = 'PNG',
         import files.utils
         files_wo_thumbs = File.objects.filter(type__in=(File.TYPES.IMAGE, File.TYPES.VIDEO))
         for b in badges:
-            # TODO: thumbnail should be set to THUMBNAIL_SIZE_DEFAULT of the source for IMAGE
+            # TODO: thumbnail should be set to THUMBNAIL_SIZE_L of the source for IMAGE
             base_path = files.utils.get_base_path(b.thumbnail.path)
             files.utils.make_thumbnails(b.thumbnail, base_path, THUMBNAIL_SIZES)
 
@@ -227,21 +228,44 @@ def make_thumbnails(image_field, output_paths: dict, output_format: str = 'PNG',
     return thumbnails
 
 
-def get_thumbnails_paths(file_name: str) -> dict:
-    # Should we check actual existence of the file?
-    # if not storage.exists(file_name):
-    #    return {}
-    base_name, _ = os.path.splitext(file_name)
-    thumbnail_ext = THUMBNAIL_FORMAT.lower()
-    if thumbnail_ext == 'jpeg':
-        thumbnail_ext = 'jpg'
-    return {
-        size_in_px: f'{base_name}_{size_in_px}x{size_in_px}.{thumbnail_ext}'
-        for size_in_px in THUMBNAIL_SIZES
-    }
+def get_thumbnail_upload_to(file_hash: str):
+    prefix = 'thumbnails/'
+    _hash = file_hash.split(':')[-1]
+    extension = f'.{THUMBNAIL_FORMAT.lower()}'
+    path = Path(prefix, _hash[:2], _hash).with_suffix(extension)
+    return path
 
 
-def _delete_file_in_storage(file_name: str) -> None:
+def get_thumbnails_paths(file_hash: str) -> dict:
+    """Return a dictionary of paths, derived from hash and predefined thumbnails dimensions."""
+    base_path = get_base_path(get_thumbnail_upload_to(file_hash))
+    extension = f'.{THUMBNAIL_FORMAT.lower()}'
+    return {size: f'{base_path}_{size[0]}x{size[1]}{extension}' for size in THUMBNAIL_SIZES}
+
+
+def extract_frame(source_path: str, output_path: str):
+    """Extract a single frame of a video at a given path, write it to the given output path."""
+    try:
+        ffmpeg = (
+            FFmpeg()
+            .option('y')
+            .input(source_path)
+            .output(
+                output_path,
+                {'ss': '00:00:00.01', 'frames:v': 1, 'update': 'true'},
+            )
+        )
+        output_dir = os.path.dirname(output_path)
+        if not os.path.isdir(output_dir):
+            os.mkdir(output_dir)
+        ffmpeg.execute()
+    except (FFmpegError, FFmpegFileNotFound, FFmpegInvalidCommand) as e:
+        logger.exception(f'Failed to extract a frame: {e.message}, {" ".join(ffmpeg.arguments)}')
+        raise
+
+
+def delete_file_in_storage(file_name: str) -> None:
+    """Delete file from disk or whatever other default storage."""
     if not file_name:
         return
 
@@ -252,21 +276,11 @@ def _delete_file_in_storage(file_name: str) -> None:
         default_storage.delete(file_name)
 
 
-def delete_thumbnail_file(file_name: str) -> None:
-    _delete_file_in_storage(file_name)
-
-    # Also delete associated thumbnails
-    thumbnail_names = get_thumbnails_paths(file_name).values()
-    for thumbnail_name in thumbnail_names:
-        if not default_storage.exists(thumbnail_name):
+def delete_thumbnails(file_metadata: dict) -> None:
+    """Read thumbnail paths from given metadata and delete them from storage."""
+    thumbnails = file_metadata.get('thumbnails', {})
+    for _, thumb in thumbnails.items:
+        path = thumb.get('path', '')
+        if not path:
             continue
-        logger.info(
-            'Deleting thumbnail %s from storage because %s was also deleted',
-            thumbnail_name,
-            file_name,
-        )
-        default_storage.delete(thumbnail_name)
-
-
-def delete_source_file(file_name: str) -> None:
-    _delete_file_in_storage(file_name)
+        delete_file_in_storage(path)

requirements.txt

@@ -40,6 +40,7 @@ mistune==2.0.4
 multidict==6.0.2
 oauthlib==3.2.0
 Pillow==9.2.0
+python-ffmpeg==2.0.12
 python-magic==0.4.27
 requests==2.28.1
 requests-oauthlib==1.3.1

reviewers/templates/reviewers/extensions_review_detail.html

@@ -76,12 +76,14 @@
   <h3>Previews Pending Approval</h3>
   <div class="row">
     {% for preview in pending_previews %}
+      {% with thumbnail_l_url=preview.file.thumbnail_l_url %}
         <div class="col-md-3">
           <a href="{{ preview.file.source.url }}" class="d-block mb-2" title="{{ preview.caption }}" target="_blank">
-          <img class="img-fluid rounded" src="{{ preview.file.source.url }}" alt="{{ preview.caption }}">
+            <img class="img-fluid rounded" src="{{ thumbnail_l_url }}" alt="{{ preview.caption }}">
           </a>
           {% include "common/components/status.html" with object=preview.file class="d-block" %}
         </div>
+      {% endwith %}
     {% endfor %}
   </div>
 </section>