Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,6 @@ PhotoSort is a powerful desktop application focused on speed designed to streaml

## Getting Started

### Prerequisites

* **Python 3.x**: Download from [python.org](https://www.python.org/).
* **jpegtran (Optional, for Lossless JPEG Rotation)**:
* **Windows**: Download from [jpegclub.org](http://jpegclub.org/jpegtran/) or install via Chocolatey: `choco install libjpeg-turbo`
* **macOS**: `brew install jpeg-turbo`
* **Linux**: `sudo apt-get install libjpeg-turbo-progs` (Ubuntu/Debian) or `sudo yum install libjpeg-turbo-utils` (RedHat/CentOS)

### Hardware Acceleration (Optional, Recommended)

For significantly faster AI-powered features like **Rotation Detection** and **Similarity Analysis**, use the appropriate requirements file for your hardware.
Expand Down
2 changes: 0 additions & 2 deletions src/core/app_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@
DEFAULT_RATING_CACHE_SIZE_LIMIT_MB = 256 # Default 256MB limit for rating cache

# --- File Operation Constants ---
# Rotation timeout
JPEGTRAN_TIMEOUT_SECONDS = 5 # Timeout for jpegtran version check

# --- Image Processing Constants ---
# Image size settings
Expand Down
141 changes: 24 additions & 117 deletions src/core/image_processing/image_rotator.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import os
import logging
import subprocess
import tempfile
from typing import Literal, Tuple
from PIL import Image, ImageOps

# Use the new pyexiv2 abstraction layer
from core.pyexiv2_wrapper import PyExiv2Operations, safe_pyexiv2_image

from pathlib import Path
from core.image_file_ops import ImageFileOperations
from core.app_settings import JPEGTRAN_TIMEOUT_SECONDS
import piexif # For EXIF manipulation with Pillow
from pillow_heif import HeifImageFile # For HEIF/HEIC support

Expand All @@ -19,6 +15,14 @@
# Rotation directions
RotationDirection = Literal["clockwise", "counterclockwise", "180"]

"""
Handles image rotation with support for:
1. Standard rotation for JPEG, PNG and other formats
2. Metadata-only rotation for RAW formats (ARW, CR2, NEF, etc.)
3. Metadata-only rotation for HEIF/HEIC formats
4. XMP orientation metadata updates for all supported formats
"""
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

This module-level docstring is positioned after imports and before the class definition. It should be moved to the top of the file, right after the imports, to follow Python docstring conventions for module documentation.

Copilot uses AI. Check for mistakes.


class ImageRotator:
# Define supported image formats for different rotation types
Expand Down Expand Up @@ -52,33 +56,6 @@ class ImageRotator:
".heif",
".heic",
}
"""
Handles image rotation with support for:
1. Lossless rotation for JPEG files (using jpegtran if available)
2. Standard rotation for PNG and other formats
3. XMP orientation metadata updates for all supported formats
"""

def __init__(self):
self.jpegtran_available = self._check_jpegtran_availability()
logger.info(f"jpegtran availability: {self.jpegtran_available}")

def _check_jpegtran_availability(self) -> bool:
"""Check if jpegtran is available in the system PATH."""
try:
result = subprocess.run(
["jpegtran", "-version"],
capture_output=True,
text=True,
timeout=JPEGTRAN_TIMEOUT_SECONDS,
)
return result.returncode == 0
except (
subprocess.SubprocessError,
FileNotFoundError,
subprocess.TimeoutExpired,
):
return False

def _get_current_orientation(self, image_path: str) -> int:
"""Get current EXIF orientation value (1-8, default 1)."""
Expand Down Expand Up @@ -144,77 +121,6 @@ def _calculate_new_orientation(

return current_orientation

def _rotate_jpeg_lossless(
self, image_path: str, direction: RotationDirection
) -> bool:
"""
Perform lossless JPEG rotation using jpegtran.
Returns True if successful, False otherwise.
"""
if not self.jpegtran_available:
return False

try:
# Map rotation direction to jpegtran parameters
if direction == "clockwise":
transform = "-rotate 90"
elif direction == "counterclockwise":
transform = "-rotate 270"
elif direction == "180":
transform = "-rotate 180"
else:
return False

# Create temporary file for output
with tempfile.NamedTemporaryFile(suffix=".jpg", delete=False) as temp_file:
temp_path = temp_file.name

# Execute jpegtran
cmd = f'jpegtran -copy all -perfect {transform} "{image_path}"'
result = subprocess.run(
cmd,
shell=True,
capture_output=True,
stdout=open(temp_path, "wb"),
timeout=30,
)

if result.returncode == 0 and os.path.getsize(temp_path) > 0:
# Replace original with rotated version
success, msg = ImageFileOperations.replace_file(temp_path, image_path)
if success:
logger.info(
f"Lossless JPEG rotation successful: {os.path.basename(image_path)}."
)
else:
logger.error(
f"Lossless JPEG rotation failed during file replacement for '{os.path.basename(image_path)}': {msg}"
)
return success
else:
logger.warning(
"jpegtran command failed for '%s'. Stderr: %s",
os.path.basename(image_path),
result.stderr.decode(),
)
return False

except Exception as e:
logger.error(
"Error during lossless JPEG rotation for '%s': %s",
os.path.basename(image_path),
e,
exc_info=True,
)
return False
finally:
# Clean up temp file if it exists
if "temp_path" in locals() and os.path.exists(temp_path):
try:
os.unlink(temp_path)
except OSError:
pass

def _rotate_image_standard(
self, image_path: str, direction: RotationDirection
) -> bool:
Expand Down Expand Up @@ -429,15 +335,21 @@ def rotate_image(
method_used = "metadata-only"
else:
# Rotate the actual image pixels
if file_ext in self._LOSSLESS_JPEG_FORMATS and self.jpegtran_available:
# Try lossless JPEG rotation first
success = self._rotate_jpeg_lossless(image_path, direction)
method_used = "lossless JPEG"

if not success:
# Fallback to standard rotation (lossy for JPEG)
if file_ext in self._LOSSLESS_JPEG_FORMATS:
# For JPEG, try metadata rotation first (lossless), then pixel rotation as fallback
metadata_success, pixel_available, msg = (
self.try_metadata_rotation_first(image_path, direction)
)
if metadata_success:
success = True
method_used = "metadata-only (lossless)"
elif pixel_available:
# Fallback to pixel rotation
success = self._rotate_image_standard(image_path, direction)
method_used = "standard (lossy fallback)"
method_used = "standard JPEG (quality=95, lossy fallback)"
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

The hardcoded "quality=95" in the method description string doesn't match the actual quality setting used in _rotate_image_standard(). Either remove the specific quality value from the string or ensure it matches the actual implementation.

Suggested change
method_used = "standard JPEG (quality=95, lossy fallback)"
method_used = "standard JPEG (lossy fallback)"

Copilot uses AI. Check for mistakes.
else:
success = False
method_used = "no available rotation method"
elif file_ext in self._LOSSLESS_HEIF_FORMATS:
# Use metadata-only rotation for HEIF/HEIC (lossless)
success = self._update_xmp_orientation(image_path, new_orientation)
Expand Down Expand Up @@ -496,8 +408,6 @@ def get_supported_formats(self) -> list[str]:
.union(self._STANDARD_PIXEL_ROTATION_FORMATS)
.union(self._RAW_FORMATS_EXIF_ONLY)
)
if self.jpegtran_available:
formats.append(".jpg (lossless via jpegtran)")
formats.append(
".heif (lossless via metadata)"
) # Add specific mention for HEIF/HEIC lossless
Expand Down Expand Up @@ -551,11 +461,8 @@ def try_metadata_rotation_first(
raw_formats = self._RAW_FORMATS_EXIF_ONLY

if file_ext in pixel_rotation_formats:
# Pixel rotation is possible but will be lossy (except lossless JPEG)
if file_ext in [".jpg", ".jpeg"] and self.jpegtran_available:
message = f"Metadata rotation failed for {filename}. Lossless JPEG rotation available."
else:
message = f"Metadata rotation failed for {filename}. Lossy pixel rotation available."
# Pixel rotation is possible (will be lossy for JPEG)
message = f"Metadata rotation failed for {filename}. Pixel rotation available."
return False, True, message
elif file_ext in raw_formats:
# RAW files should only use metadata rotation
Expand Down
9 changes: 6 additions & 3 deletions src/ui/menu_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from PyQt6.QtGui import QAction, QIcon, QKeySequence
from PyQt6.QtWidgets import QMenu, QStyle, QWidget, QWidgetAction, QHBoxLayout, QLabel

from core.metadata_processor import MetadataProcessor
from core.image_processing.image_rotator import ImageRotator
from core.app_settings import get_recent_folders

logger = logging.getLogger(__name__)
Expand All @@ -24,6 +24,9 @@ def __init__(self, main_window: "MainWindow"):
self.dialog_manager = main_window.dialog_manager
self.app_state = main_window.app_state

# Initialize ImageRotator for fast rotation support checking
self.image_rotator = ImageRotator()

# --- Actions ---
# File Menu
self.open_folder_action: QAction
Expand Down Expand Up @@ -526,8 +529,8 @@ def show_image_context_menu(self, position: QPoint):

menu = QMenu(main_win)

# Rotation
if MetadataProcessor.is_rotation_supported(file_path):
# Rotation - use ImageRotator's fast extension-based check to avoid PyExiv2 operations in windowed builds
if self.image_rotator.is_rotation_supported(file_path):
selected_paths = main_win._get_selected_file_paths_from_view()
num_selected = len(selected_paths) if len(selected_paths) > 1 else 1
label_suffix = f" {num_selected} Images" if num_selected > 1 else ""
Expand Down
92 changes: 90 additions & 2 deletions tests/test_image_rotator.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ def _create_temp_copy(self, source_path: str) -> str:
def test_rotator_initialization(self):
"""Test that ImageRotator initializes correctly."""
assert isinstance(self.rotator, ImageRotator)
# jpegtran availability might vary by system
assert isinstance(self.rotator.jpegtran_available, bool)
# Verify that ImageRotator initializes successfully without requiring external dependencies or configuration

def test_get_supported_formats(self):
"""Test that supported formats are returned correctly."""
Expand Down Expand Up @@ -196,6 +195,95 @@ def test_rotation_error_handling_invalid_direction(self):
)
assert not success

def test_jpeg_metadata_first_approach(self):
"""Test that JPEG files attempt metadata rotation first."""
if not any(
img.lower().endswith((".jpg", ".jpeg")) for img in self.sample_images
):
pytest.skip("No JPEG sample images available")

# Find a JPEG sample
jpeg_sample = next(
(
img
for img in self.sample_images
if img.lower().endswith((".jpg", ".jpeg"))
),
None,
)
if not jpeg_sample:
pytest.skip("No JPEG sample found")

temp_image = self._create_temp_copy(jpeg_sample)

# Test the try_metadata_rotation_first method directly
metadata_success, pixel_available, message = (
self.rotator.try_metadata_rotation_first(temp_image, "clockwise")
)

# Should either succeed with metadata or indicate pixel rotation is available
assert metadata_success or pixel_available
assert "metadata" in message.lower() or "pixel" in message.lower()
logging.info(f"JPEG metadata-first test: {message}")

def test_jpeg_rotation_method_reporting(self):
"""Test that JPEG rotation reports the correct method used."""
if not any(
img.lower().endswith((".jpg", ".jpeg")) for img in self.sample_images
):
pytest.skip("No JPEG sample images available")

# Find a JPEG sample
jpeg_sample = next(
(
img
for img in self.sample_images
if img.lower().endswith((".jpg", ".jpeg"))
),
None,
)
if not jpeg_sample:
pytest.skip("No JPEG sample found")

temp_image = self._create_temp_copy(jpeg_sample)

# Perform rotation and check the method reported
success, message = self.rotator.rotate_clockwise(temp_image)

assert success
# Should report either metadata-only (lossless) or quality=95 fallback
assert "metadata-only" in message.lower() or "quality=95" in message.lower()
logging.info(f"JPEG rotation method: {message}")

def test_raw_format_metadata_only(self):
"""Test that RAW formats only use metadata rotation."""
raw_extensions = [".arw", ".cr2", ".nef", ".dng"]
raw_sample = None

for img in self.sample_images:
if any(img.lower().endswith(ext) for ext in raw_extensions):
raw_sample = img
break

if not raw_sample:
pytest.skip("No RAW sample images available")

temp_image = self._create_temp_copy(raw_sample)
original_size = os.path.getsize(temp_image)

# Perform rotation on RAW file
success, message = self.rotator.rotate_clockwise(temp_image)

# Should succeed and use metadata-only
assert success
assert "metadata-only" in message.lower()
assert "raw format" in message.lower()

# File size should be nearly identical (only metadata changed)
new_size = os.path.getsize(temp_image)
assert abs(new_size - original_size) < 1024 # Less than 1KB difference
logging.info(f"RAW rotation: {message}")


if __name__ == "__main__":
# Run tests with verbose output
Expand Down