-
Notifications
You must be signed in to change notification settings - Fork 14
Thumbnails API for SourceImages #1306
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
base: main
Are you sure you want to change the base?
Changes from all commits
fff2f04
a2c26f6
8077c2a
d11cfa3
33f42df
04cb3b3
e9abc52
1a150a2
f94b5c8
c7484ab
182c035
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,16 @@ | |
| import logging | ||
| from statistics import mode | ||
|
|
||
| from django.conf import settings | ||
| from django.contrib.postgres.search import TrigramSimilarity | ||
| from django.core import exceptions | ||
| from django.core.files.storage import default_storage | ||
| from django.db import models | ||
| from django.db.models import Prefetch, Q | ||
| from django.db.models.functions import Coalesce | ||
| from django.db.models.query import QuerySet | ||
| from django.forms import BooleanField, CharField, IntegerField | ||
| from django.shortcuts import get_object_or_404 | ||
| from django.shortcuts import get_object_or_404, redirect | ||
| from django.utils import timezone | ||
| from django_filters.rest_framework import DjangoFilterBackend | ||
| from drf_spectacular.types import OpenApiTypes | ||
|
|
@@ -709,6 +711,35 @@ def unstar(self, _request, pk=None) -> Response: | |
| raise api_exceptions.ValidationError(detail="Source image must be associated with a project") | ||
|
|
||
|
|
||
| class SourceImageThumbnailViewSet(DefaultReadOnlyViewSet, ProjectMixin): | ||
| """ | ||
| Endpoint for capture thumbnails | ||
| """ | ||
|
|
||
| queryset = SourceImage.objects.all() | ||
|
|
||
| permission_classes = [ObjectPermission] | ||
|
|
||
| def list(self, request): | ||
| raise api_exceptions.NotFound(detail=f"No collection of thumbnails") | ||
|
|
||
| def retrieve(self, request, pk=None): | ||
| _sizes = settings.THUMBNAILS["SIZES"] | ||
|
|
||
| label = self.request.query_params.get("label", next(iter(_sizes))) | ||
| size = _sizes.get(label, None) | ||
|
loppear marked this conversation as resolved.
|
||
| if size is None: | ||
| raise api_exceptions.ValidationError( | ||
| detail=f"Invalid thumbnail size label provided: {label} not in {', '.join(_sizes.keys())}" | ||
| ) | ||
| obj: SourceImage = self.get_object() | ||
| try: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm tempted to say we don't need the SourceImageThumbnail model and can rely on the exists() check in the storage. But then I suppose we have to parse the height/width from the file name, and make sure we have a way to create those consistently. So I could go either way! This will be a big table eventually.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assuming files are on S3, I don't think we'll want to do filesystem name checks. Table size will be commensurate to SourceImage, could be pruned for age if that becomes an issue, though would lose the "we still have thumbnails if source files are removed" benefit. We don't join to the table just per-image lookups - should probably add a (source_image, label) index. |
||
| thumb = obj.find_or_generate_thumbnail_for_label(label) | ||
| except exceptions.ObjectDoesNotExist as e: | ||
| raise api_exceptions.NotFound(detail=f"{e}") | ||
| return redirect(default_storage.url(thumb.path)) | ||
|
|
||
|
|
||
| class SourceImageCollectionViewSet(DefaultViewSet, ProjectMixin): | ||
| """ | ||
| Endpoint for viewing capture sets or samples of captures. | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,49 @@ | ||||||||||||||||||||||||||||||||||||
| # Generated by Django 4.2.10 on 2026-05-13 10:17 | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| from django.db import migrations, models | ||||||||||||||||||||||||||||||||||||
| import django.db.models.deletion | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| class Migration(migrations.Migration): | ||||||||||||||||||||||||||||||||||||
| dependencies = [ | ||||||||||||||||||||||||||||||||||||
| ("main", "0084_revoke_delete_job_from_roles"), | ||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| operations = [ | ||||||||||||||||||||||||||||||||||||
| migrations.CreateModel( | ||||||||||||||||||||||||||||||||||||
| name="SourceImageThumbnail", | ||||||||||||||||||||||||||||||||||||
| fields=[ | ||||||||||||||||||||||||||||||||||||
| ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), | ||||||||||||||||||||||||||||||||||||
| ("created_at", models.DateTimeField(auto_now_add=True)), | ||||||||||||||||||||||||||||||||||||
| ("updated_at", models.DateTimeField(auto_now=True)), | ||||||||||||||||||||||||||||||||||||
| ("path", models.CharField(blank=True, max_length=255)), | ||||||||||||||||||||||||||||||||||||
| ("label", models.CharField(max_length=255)), | ||||||||||||||||||||||||||||||||||||
| ("width", models.IntegerField(blank=True, null=True)), | ||||||||||||||||||||||||||||||||||||
| ("height", models.IntegerField(blank=True, null=True)), | ||||||||||||||||||||||||||||||||||||
| ("size", models.BigIntegerField(blank=True, null=True)), | ||||||||||||||||||||||||||||||||||||
| ("last_modified", models.DateTimeField(blank=True, null=True, auto_now_add=True)), | ||||||||||||||||||||||||||||||||||||
| ( | ||||||||||||||||||||||||||||||||||||
| "source_image", | ||||||||||||||||||||||||||||||||||||
| models.ForeignKey( | ||||||||||||||||||||||||||||||||||||
| null=True, | ||||||||||||||||||||||||||||||||||||
| on_delete=django.db.models.deletion.SET_NULL, | ||||||||||||||||||||||||||||||||||||
| related_name="thumbnails", | ||||||||||||||||||||||||||||||||||||
| to="main.sourceimage", | ||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+25
to
+33
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
A thumbnail is meaningless without its 🐛 Proposed fix (
"source_image",
models.ForeignKey(
- null=True,
- on_delete=django.db.models.deletion.SET_NULL,
+ on_delete=django.db.models.deletion.CASCADE,
related_name="thumbnails",
to="main.sourceimage",
),
),Note: this requires removing 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||
| options={ | ||||||||||||||||||||||||||||||||||||
| "abstract": False, | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||
| migrations.AddIndex( | ||||||||||||||||||||||||||||||||||||
| model_name="sourceimagethumbnail", | ||||||||||||||||||||||||||||||||||||
| index=models.Index(fields=["source_image", "label"], name="main_source_source__b0d4cd_idx"), | ||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||
| migrations.AddConstraint( | ||||||||||||||||||||||||||||||||||||
| model_name="sourceimagethumbnail", | ||||||||||||||||||||||||||||||||||||
| constraint=models.UniqueConstraint( | ||||||||||||||||||||||||||||||||||||
| fields=("source_image", "label"), name="unique_source_image_thumbnail_label" | ||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ | |
| from django.conf import settings | ||
| from django.contrib.auth.models import AbstractUser, AnonymousUser | ||
| from django.contrib.postgres.fields import ArrayField | ||
| from django.core.exceptions import ValidationError | ||
| from django.core.exceptions import ObjectDoesNotExist, ValidationError | ||
| from django.core.files.storage import default_storage | ||
| from django.db import IntegrityError, models, transaction | ||
| from django.db.models import Exists, OuterRef, Q | ||
|
|
@@ -44,6 +44,7 @@ | |
| from ami.users.models import User | ||
| from ami.utils.media import calculate_file_checksum, extract_timestamp | ||
| from ami.utils.requests import get_apply_default_filters_flag, get_default_classification_threshold | ||
| from ami.utils.s3 import botocore, read_image | ||
| from ami.utils.schemas import OrderedEnum | ||
|
|
||
| if typing.TYPE_CHECKING: | ||
|
|
@@ -2206,6 +2207,63 @@ def get_custom_user_permissions(self, user) -> list[str]: | |
| custom_perms.add(Project.Permissions.RUN_SINGLE_IMAGE_JOB) | ||
| return list(custom_perms) | ||
|
|
||
| def find_or_generate_thumbnail_for_label(self, label): | ||
| try: | ||
| thumb = self.thumbnails.get(label=label) | ||
| except SourceImageThumbnail.DoesNotExist: | ||
| thumb = None | ||
| size = settings.THUMBNAILS["SIZES"].get(label) | ||
| prefix = settings.THUMBNAILS["STORAGE_PREFIX"] | ||
|
|
||
| if ( | ||
| not thumb | ||
| or thumb.width != size["width"] | ||
| or thumb.last_modified < self.last_modified | ||
| or not default_storage.exists(thumb.path) | ||
| ): | ||
|
loppear marked this conversation as resolved.
|
||
| if self.path and self.deployment and self.deployment.data_source: | ||
| config = self.deployment.data_source.config | ||
| # Get the file | ||
| try: | ||
| img = read_image(config=config, key=self.path) | ||
| except botocore.exceptions.ClientError as e: | ||
| logger.error(f"Could not read image for {self.path}: {e}") | ||
| raise ObjectDoesNotExist(f"SourceImage with id {self.pk} media not found") from e | ||
| elif self.path: | ||
| img = PIL.Image.open(default_storage.open(self.path)) | ||
| else: | ||
| raise ObjectDoesNotExist(f"SourceImage with id {self.pk} media config not found") | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| # Make the thumbnail | ||
| orig_width, orig_height = img.size | ||
| width = size["width"] | ||
| height = size.get("height", None) | ||
| if not height: | ||
| height = int(orig_height * (width / float(orig_width))) | ||
| new_size = (width, height) | ||
| img.thumbnail(new_size) | ||
|
|
||
| buffer = BytesIO() | ||
| img.save(buffer, format="JPEG") | ||
| contents = buffer.getvalue() | ||
| file_size = len(contents) | ||
|
Comment on lines
+2236
to
+2248
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: The error "OSError: cannot write mode RGBA as JPEG" occurs because the JPEG file format does not support an alpha channel (transparency) [1][2][3]. Pillow explicitly prevents saving RGBA images as JPEG to avoid ambiguity regarding how to handle the transparent pixels [4]. To resolve this, you must convert the image to a mode supported by JPEG before saving [2][3]. The standard JPEG format supports the following modes: - L (8-bit grayscale) [3][5] - RGB (24-bit true color) [3][5] - CMYK (32-bit color separation) [3][5] Solution You can convert your image to RGB using the convert method [2][3]. Note that this will discard the alpha channel, and transparent areas will typically turn black or white depending on how the conversion is handled [1][2]. Example code: from PIL import Image # Open the RGBA image img = Image.open("input.png") # Convert to RGB (discards alpha channel) rgb_img = img.convert("RGB") # Save as JPEG rgb_img.save("output.jpg") If you need to preserve transparency, you should save the image in a format that supports it, such as PNG [1][2]. If you specifically need to keep the alpha channel but must use a JPEG-like compression, consider formats like JPEG 2000, which supports RGBA [3][5]. Citations:
JPEG encoding fails for non-RGB source images without conversion.
Proposed fix new_size = (width, height)
img.thumbnail(new_size)
+ if img.mode not in ("RGB", "L"):
+ img = img.convert("RGB")
+
buffer = BytesIO()
img.save(buffer, format="JPEG")🤖 Prompt for AI Agents
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mihow My guess is this an assumption throughout (that source images do not include alpha layer PNGs) and I'm not going to address/guard that here if so.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| # Remove prior thumbnails for this size | ||
| for t in self.thumbnails.filter(label=label): | ||
| default_storage.delete(t.path) | ||
| t.delete() | ||
|
|
||
| # Write to storage | ||
| buffer.seek(0) | ||
| thumbnail_key = f"{prefix}capture_{self.pk}/{label}.jpg" | ||
| thumbnail_path = default_storage.save(thumbnail_key, buffer) | ||
|
|
||
| # Save to DB | ||
| width, height = img.size | ||
| thumb = self.thumbnails.create( | ||
| path=thumbnail_path, label=label, width=width, height=height, size=file_size | ||
| ) | ||
| return thumb | ||
|
|
||
| class Meta: | ||
| ordering = ("deployment", "event", "timestamp") | ||
|
|
||
|
|
@@ -2392,6 +2450,31 @@ def sample_captures_by_nth( | |
| yield from qs[::nth] | ||
|
|
||
|
|
||
| @final | ||
| class SourceImageThumbnail(BaseModel): | ||
| """A thumbnail cache of a SourceImage""" | ||
|
|
||
| path = models.CharField(max_length=255, blank=True) | ||
| label = models.CharField(max_length=255) | ||
| width = models.IntegerField(null=True, blank=True) | ||
| height = models.IntegerField(null=True, blank=True) | ||
| size = models.BigIntegerField(null=True, blank=True) | ||
| last_modified = models.DateTimeField(null=True, blank=True, auto_now_add=True) | ||
|
|
||
| source_image = models.ForeignKey(SourceImage, on_delete=models.SET_NULL, null=True, related_name="thumbnails") | ||
|
|
||
|
loppear marked this conversation as resolved.
|
||
| class Meta: | ||
| constraints = [ | ||
| models.UniqueConstraint( | ||
| fields=["source_image", "label"], | ||
| name="unique_source_image_thumbnail_label", | ||
| ), | ||
| ] | ||
| indexes = [ | ||
| models.Index(fields=["source_image", "label"]), | ||
| ] | ||
|
|
||
|
|
||
| # @final | ||
| # class IdentificationHistory(BaseModel): | ||
| # """A history of identifications for an occurrence.""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funny that this
thumbnailfield has been hanging out empty all along! Glad to retire it