diff --git a/requirements/metadata_editor.md b/requirements/metadata_editor.md index 4b34ebb..00b2f65 100644 --- a/requirements/metadata_editor.md +++ b/requirements/metadata_editor.md @@ -104,6 +104,11 @@ The main missing pieces are: - `METADATA_EDITOR-0013`: The command shall write changes through an ffmpeg stream-copy remux workflow only. No transcoding shall be performed as part of `ffx edit`. +- `METADATA_EDITOR-0013A`: The ffmpeg invocation used by `ffx edit` shall map + all source streams with `-map 0` and shall copy all mapped streams with a + single `-c copy`. It shall not emit conversion-style per-stream `-map` or + `-c:*` options that could drop, reorder, or transcode streams during a + metadata-only edit. - `METADATA_EDITOR-0014`: Because ffmpeg cannot rewrite the source file in place, `ffx edit` shall write to a temporary output file on the same filesystem as the source file and shall replace the original path only after @@ -130,13 +135,19 @@ The main missing pieces are: configuration shall be surfaced clearly in the UI and in the planned-changes view. If those rules are applied during save, the operator shall be able to tell that the file will be cleaned in addition to any manual edits. -- `METADATA_EDITOR-0022`: The command shall provide an invocation-level way to - disable config-driven cleanup when the operator wants a pure manual metadata - edit without automatic tag removal. +- `METADATA_EDITOR-0022`: Edit mode shall provide an in-screen operator toggle + for config-driven cleanup so a user can switch between pure manual metadata + edits and metadata edits plus configured tag cleanup without leaving the + editor. - `METADATA_EDITOR-0023`: The existing global `--dry-run` behavior shall apply to `ffx edit`. In dry-run mode the command shall not replace the original file and shall expose the planned write operation clearly enough for the user to understand what would happen. +- `METADATA_EDITOR-0024`: Every ffmpeg invocation performed by `ffx edit` + shall be surfaced to the operator as a notification in the edit UI. +- `METADATA_EDITOR-0025`: When application verbosity is greater than zero, the + notification for an `ffx edit` ffmpeg invocation shall include the concrete + ffmpeg command line. ## Acceptance @@ -154,6 +165,8 @@ The main missing pieces are: - The planned-changes view reflects manual edits relative to the original file and, when enabled, any configured cleanup removals. - No rendered Rich or Textual color markup appears in the saved file metadata. +- Saving metadata with files that contain PGS subtitle tracks or other + non-text subtitle codecs preserves those streams instead of dropping them. - If ffmpeg fails while saving, the original file remains present and readable at the original path. - In dry-run mode, the original file remains untouched. diff --git a/src/ffx/confirm_screen.py b/src/ffx/confirm_screen.py index 10b7994..94ae8c8 100644 --- a/src/ffx/confirm_screen.py +++ b/src/ffx/confirm_screen.py @@ -3,6 +3,7 @@ from textual.screen import Screen from textual.widgets import Button, Footer, Header, Static from .i18n import t +from .screen_support import build_screen_log_pane class ConfirmScreen(Screen): @@ -58,6 +59,7 @@ class ConfirmScreen(Screen): yield Button(self.__confirmLabel, id="confirm_button") yield Button(self.__cancelLabel, id="cancel_button") + yield build_screen_log_pane() yield Footer() def on_button_pressed(self, event: Button.Pressed) -> None: diff --git a/src/ffx/ffx_app.py b/src/ffx/ffx_app.py index c293fb4..8796395 100644 --- a/src/ffx/ffx_app.py +++ b/src/ffx/ffx_app.py @@ -4,6 +4,7 @@ from .i18n import set_current_language, t from .shows_screen import ShowsScreen from .inspect_details_screen import InspectDetailsScreen from .media_edit_screen import MediaEditScreen +from .screen_support import toggle_screen_log_pane class FfxApp(App): @@ -13,6 +14,7 @@ class FfxApp(App): BINDINGS = [ ("q", "quit()", t("Quit")), ("h", "switch_mode('help')", t("Help")), + ("l", "toggle_log_pane", t("Log")), ] @@ -41,3 +43,6 @@ class FfxApp(App): def getContext(self): """Data 'output' method""" return self.context + + def action_toggle_log_pane(self) -> None: + toggle_screen_log_pane(self.screen) diff --git a/src/ffx/help_screen.py b/src/ffx/help_screen.py index d162f6e..5475b84 100644 --- a/src/ffx/help_screen.py +++ b/src/ffx/help_screen.py @@ -3,7 +3,7 @@ from textual.screen import Screen from textual.widgets import Footer, Placeholder from .i18n import t -from .screen_support import go_back_or_exit +from .screen_support import build_screen_log_pane, go_back_or_exit class HelpScreen(Screen): BINDINGS = [ @@ -17,6 +17,7 @@ class HelpScreen(Screen): def compose(self) -> ComposeResult: # Row 1 yield Placeholder(t("Help Screen")) + yield build_screen_log_pane() yield Footer() def action_back(self): diff --git a/src/ffx/inspect_details_screen.py b/src/ffx/inspect_details_screen.py index dc08d6d..fb6ec67 100644 --- a/src/ffx/inspect_details_screen.py +++ b/src/ffx/inspect_details_screen.py @@ -18,6 +18,7 @@ from .pattern_details_screen import PatternDetailsScreen from .screen_support import ( add_auto_table_column, build_screen_controllers, + build_screen_log_pane, go_back_or_exit, localized_column_width, update_table_column_label, @@ -190,6 +191,7 @@ class InspectDetailsScreen(MediaWorkflowScreenBase): yield self.tracksTable yield Static(" ") + yield build_screen_log_pane() yield Footer() def _update_grid_layout(self) -> None: diff --git a/src/ffx/media_descriptor_change_set.py b/src/ffx/media_descriptor_change_set.py index 282e504..7151b4a 100644 --- a/src/ffx/media_descriptor_change_set.py +++ b/src/ffx/media_descriptor_change_set.py @@ -203,7 +203,7 @@ class MediaDescriptorChangeSet(): if ( self.__applyMetadataNormalization and trackDescriptor is not None - and trackDescriptor.getType() == TrackType.SUBTITLE + and trackDescriptor.getType() in (TrackType.VIDEO, TrackType.AUDIO, TrackType.SUBTITLE) ): trackTitle = str(normalizedTrackTags.get("title", "")).strip() fallbackTitle = str((fallbackTrackTags or {}).get("title", "")).strip() @@ -260,6 +260,8 @@ class MediaDescriptorChangeSet(): # else: # dispositionTokens += [f"-disposition:{streamIndicator}:{subIndex}", '0'] for ttd in self.__targetTrackDescriptors: + if ttd.getType() == TrackType.ATTACHMENT: + continue targetDispositions = ttd.getDispositionSet() streamIndicator = ttd.getType().indicator() @@ -344,7 +346,7 @@ class MediaDescriptorChangeSet(): for tagKey, tagValue in self.normalizeTrackTags( outputTrackTags, trackDescriptor=trackDescriptor, - fallbackTrackTags=unchangedTrackTags | removedTrackTags, + fallbackTrackTags=trackDescriptor.getTags(), ).items(): metadataTokens += [f"-metadata:s:{trackDescriptor.getType().indicator()}" + f":{trackDescriptor.getSubIndex()}", @@ -366,6 +368,7 @@ class MediaDescriptorChangeSet(): for tagKey, tagValue in self.normalizeTrackTags( preservedTrackTags, trackDescriptor=trackDescriptor, + fallbackTrackTags=trackDescriptor.getTags(), ).items(): metadataTokens += [f"-metadata:s:{trackDescriptor.getType().indicator()}" + f":{trackDescriptor.getSubIndex()}", diff --git a/src/ffx/media_edit_screen.py b/src/ffx/media_edit_screen.py index 4d02c39..acb6981 100644 --- a/src/ffx/media_edit_screen.py +++ b/src/ffx/media_edit_screen.py @@ -1,6 +1,9 @@ import os +from time import monotonic +from textual import work from textual.containers import Grid +from textual.worker import Worker, WorkerState from textual.widgets import Button, Footer, Header, Static from ffx.metadata_editor import apply_metadata_edits @@ -9,7 +12,7 @@ from ffx.track_descriptor import TrackDescriptor from .i18n import t from .confirm_screen import ConfirmScreen from .media_workflow_screen_base import MediaWorkflowScreenBase -from .screen_support import localized_column_width +from .screen_support import build_screen_log_pane, localized_column_width, write_screen_log from .tag_delete_screen import TagDeleteScreen from .tag_details_screen import TagDetailsScreen from .track_details_screen import TrackDetailsScreen @@ -169,6 +172,7 @@ class MediaEditScreen(MediaWorkflowScreenBase): yield Button(t("Quit"), id="quit_button") yield Static(" ") + yield build_screen_log_pane() yield Footer() def on_mount(self): @@ -177,6 +181,7 @@ class MediaEditScreen(MediaWorkflowScreenBase): self.updateTracks() self.updateDifferences() self.updateToggleButtons() + self._applyChangesWorker = None def _update_grid_layout(self) -> None: leftColumnWidth = max( @@ -195,6 +200,30 @@ class MediaEditScreen(MediaWorkflowScreenBase): if self._messageText: self.notify(self._messageText) + def _notify_from_worker(self, message: str) -> None: + self.app.call_from_thread(write_screen_log, self, str(message)) + self.app.call_from_thread(self.notify, str(message)) + + def _report_apply_timings(self, applyResult: dict, reloadSeconds: float = 0.0) -> None: + timings = dict(applyResult.get("timings", {})) + ffmpegSeconds = float(timings.get("ffmpeg_seconds", 0.0)) + replaceSeconds = float(timings.get("replace_seconds", 0.0)) + writeSeconds = float(timings.get("write_seconds", ffmpegSeconds + replaceSeconds)) + reloadSeconds = float(reloadSeconds) + totalSeconds = writeSeconds + reloadSeconds + + timingSummary = ( + f"ffx edit timings: ffmpeg={ffmpegSeconds:.2f}s " + + f"replace={replaceSeconds:.2f}s " + + f"reload={reloadSeconds:.2f}s " + + f"total={totalSeconds:.2f}s" + ) + self.context["logger"].info(timingSummary) + write_screen_log(self, timingSummary) + + if int(self.context.get("verbosity", 0) or 0) > 0: + self.notify(timingSummary) + def updateToggleButtons(self): self._set_toggle_button_state( "#cleanup_toggle_button", @@ -296,6 +325,7 @@ class MediaEditScreen(MediaWorkflowScreenBase): def action_toggle_normalization(self): self.setApplyNormalization(not self._applyNormalization) self.updateToggleButtons() + self.updateTracks() self.updateDifferences() self.setMessage( t("Normalization enabled.") @@ -356,33 +386,80 @@ class MediaEditScreen(MediaWorkflowScreenBase): self.setMessage(t("No changes to apply.")) return - try: - applyResult = apply_metadata_edits( - self.context, - self._mediaFilename, - self._baselineMediaDescriptor, - self._sourceMediaDescriptor, - ) - except Exception as ex: - self.context["logger"].exception( - "Failed to apply metadata edits for %s", - self._mediaFilename, - ) - self.setMessage(t("Apply failed: {error}", error=ex)) + if self._applyChangesWorker is not None and self._applyChangesWorker.is_running: + self.setMessage(t("Apply already running.")) return + write_screen_log( + self, + t("Starting metadata apply for {filename}.", filename=self._mediaFilename), + ) + self._applyChangesWorker = self.run_apply_changes_worker() + + @work( + thread=True, + exclusive=True, + group="media-edit-apply", + exit_on_error=False, + ) + def run_apply_changes_worker(self): + return apply_metadata_edits( + self.context, + self._mediaFilename, + self._baselineMediaDescriptor, + self._sourceMediaDescriptor, + notify=self._notify_from_worker, + ) + + def on_worker_state_changed(self, event: Worker.StateChanged) -> None: + if event.worker is not self._applyChangesWorker: + return + + if event.state == WorkerState.ERROR: + error = event.worker.error + if error is not None: + self.context["logger"].error( + "Failed to apply metadata edits for %s", + self._mediaFilename, + exc_info=(type(error), error, error.__traceback__), + ) + write_screen_log(self, t("Apply failed: {error}", error=error)) + self.setMessage(t("Apply failed: {error}", error=error)) + self._applyChangesWorker = None + return + + if event.state != WorkerState.SUCCESS: + return + + applyResult = event.worker.result or {} + if applyResult.get("dry_run", False): + self._report_apply_timings(applyResult, reloadSeconds=0.0) + write_screen_log( + self, + t( + "Dry-run prepared temporary output {target_path}.", + target_path=applyResult["target_path"], + ), + ) self.setMessage( t( "Dry-run: would rewrite via temporary file {target_path}", target_path=applyResult["target_path"], ) ) + self._applyChangesWorker = None return + reloadStart = monotonic() + write_screen_log(self, t("Reloading file after metadata write.")) self.reloadProperties(reset_draft=True) self.refreshAfterDraftChange() + reloadSeconds = monotonic() - reloadStart + self._report_apply_timings(applyResult, reloadSeconds=reloadSeconds) + write_screen_log(self, t("Changes applied and file reloaded.")) self.setMessage(t("Changes applied and file reloaded.")) + self._applyChangesWorker = None def action_revert_changes(self): if not self.hasPendingChanges(): diff --git a/src/ffx/media_workflow_screen_base.py b/src/ffx/media_workflow_screen_base.py index a46805c..152b390 100644 --- a/src/ffx/media_workflow_screen_base.py +++ b/src/ffx/media_workflow_screen_base.py @@ -9,6 +9,7 @@ from textual.widgets._data_table import CellDoesNotExist from ffx.audio_layout import AudioLayout from ffx.file_properties import FileProperties from ffx.helper import DIFF_ADDED_KEY, DIFF_CHANGED_KEY, DIFF_REMOVED_KEY +from ffx.iso_language import IsoLanguage from ffx.media_descriptor_change_set import MediaDescriptorChangeSet from ffx.track_descriptor import TrackDescriptor from ffx.track_disposition import TrackDisposition @@ -185,6 +186,7 @@ class MediaWorkflowScreenBase(Screen): trackDescriptorList = self._sourceMediaDescriptor.getTrackDescriptors() typeCounter = {} + applyNormalization = bool(getattr(self, "_applyNormalization", False)) for trackDescriptor in trackDescriptorList: trackType = trackDescriptor.getType() @@ -193,6 +195,15 @@ class MediaWorkflowScreenBase(Screen): dispositionSet = trackDescriptor.getDispositionSet() audioLayout = trackDescriptor.getAudioLayout() + trackTitle = trackDescriptor.getTitle() + if ( + applyNormalization + and not str(trackTitle).strip() + and trackType in (TrackType.VIDEO, TrackType.AUDIO, TrackType.SUBTITLE) + ): + trackLanguage = trackDescriptor.getLanguage() + if trackLanguage != IsoLanguage.UNDEFINED: + trackTitle = trackLanguage.label() row = ( trackDescriptor.getIndex(), t(trackType.label()), @@ -203,7 +214,7 @@ class MediaWorkflowScreenBase(Screen): and audioLayout != AudioLayout.LAYOUT_UNDEFINED else " ", trackDescriptor.getLanguage().label(), - trackDescriptor.getTitle(), + trackTitle, t("Yes") if TrackDisposition.DEFAULT in dispositionSet else t("No"), t("Yes") if TrackDisposition.FORCED in dispositionSet else t("No"), ) diff --git a/src/ffx/metadata_editor.py b/src/ffx/metadata_editor.py index da0f2ac..0f665e7 100644 --- a/src/ffx/metadata_editor.py +++ b/src/ffx/metadata_editor.py @@ -1,15 +1,19 @@ from __future__ import annotations +import click import os import tempfile +from time import monotonic from .constants import ( DEFAULT_AC3_BANDWIDTH, DEFAULT_DTS_BANDWIDTH, DEFAULT_STEREO_BANDWIDTH, + FFMPEG_COMMAND_TOKENS, ) -from .ffx_controller import FfxController from .media_descriptor import MediaDescriptor +from .media_descriptor_change_set import MediaDescriptorChangeSet +from .process import executeProcess, formatCommandSequence from .video_encoder import VideoEncoder @@ -49,41 +53,110 @@ def build_metadata_edit_context(context: dict) -> dict: return editContext +def build_metadata_edit_command( + context: dict, + source_path: str, + target_path: str, + baseline_descriptor: MediaDescriptor, + draft_descriptor: MediaDescriptor, +) -> list[str]: + changeSet = MediaDescriptorChangeSet(context, draft_descriptor, baseline_descriptor) + + return ( + list(FFMPEG_COMMAND_TOKENS) + + ["-i", source_path, "-map", "0", "-c", "copy"] + + changeSet.generateMetadataTokens() + + changeSet.generateDispositionTokens() + + [target_path] + ) + + +def notify_ffmpeg_invocation( + context: dict, + command_sequence: list[str], + *, + notify=None, + dry_run: bool = False, +) -> None: + notify_callback = notify or context.get("notify_callback") + if not callable(notify_callback): + return + + verbosity = int(context.get("verbosity", 0) or 0) + if verbosity > 0: + if dry_run: + notify_callback(f"ffmpeg dry-run: {formatCommandSequence(command_sequence)}") + else: + notify_callback(f"ffmpeg: {formatCommandSequence(command_sequence)}") + return + + notify_callback("ffmpeg dry-run prepared.") if dry_run else notify_callback( + "ffmpeg metadata write started." + ) + + def apply_metadata_edits( context: dict, source_path: str, baseline_descriptor: MediaDescriptor, draft_descriptor: MediaDescriptor, + *, + notify=None, ) -> dict[str, object]: temporaryOutputPath = create_temporary_output_path(source_path) editContext = build_metadata_edit_context(context) - controller = FfxController(editContext, draft_descriptor, baseline_descriptor) + commandSequence = build_metadata_edit_command( + editContext, + source_path, + temporaryOutputPath, + baseline_descriptor, + draft_descriptor, + ) + ffmpegSeconds = 0.0 + replaceSeconds = 0.0 try: - controller.runJob( - source_path, - temporaryOutputPath, - targetFormat="", - chainIteration=[], - ) - if editContext.get("dry_run", False): + notify_ffmpeg_invocation( + editContext, + commandSequence, + notify=notify, + dry_run=True, + ) return { "applied": False, "dry_run": True, "target_path": temporaryOutputPath, + "command_sequence": commandSequence, + "timings": { + "ffmpeg_seconds": ffmpegSeconds, + "replace_seconds": replaceSeconds, + "write_seconds": ffmpegSeconds + replaceSeconds, + }, } + notify_ffmpeg_invocation(editContext, commandSequence, notify=notify) + ffmpegStart = monotonic() + _out, err, rc = executeProcess(commandSequence, context=editContext) + ffmpegSeconds = monotonic() - ffmpegStart + if rc: + raise click.ClickException(f"ffmpeg edit failed: rc={rc} error={err}") + + replaceStart = monotonic() os.replace(temporaryOutputPath, source_path) + replaceSeconds = monotonic() - replaceStart return { "applied": True, "dry_run": False, "target_path": source_path, + "command_sequence": commandSequence, + "timings": { + "ffmpeg_seconds": ffmpegSeconds, + "replace_seconds": replaceSeconds, + "write_seconds": ffmpegSeconds + replaceSeconds, + }, } except Exception: if os.path.exists(temporaryOutputPath): os.remove(temporaryOutputPath) raise - finally: - if editContext.get("dry_run", False) and os.path.exists(temporaryOutputPath): - os.remove(temporaryOutputPath) diff --git a/src/ffx/pattern_delete_screen.py b/src/ffx/pattern_delete_screen.py index 29ce8f5..d786978 100644 --- a/src/ffx/pattern_delete_screen.py +++ b/src/ffx/pattern_delete_screen.py @@ -7,7 +7,7 @@ from textual.containers import Grid from .i18n import t from .show_controller import ShowController from .pattern_controller import PatternController -from .screen_support import go_back_or_exit +from .screen_support import build_screen_log_pane, go_back_or_exit from ffx.model.pattern import Pattern @@ -103,6 +103,7 @@ class PatternDeleteScreen(Screen): yield Button(t("Delete"), id="delete_button") yield Button(t("Cancel"), id="cancel_button") + yield build_screen_log_pane() yield Footer() diff --git a/src/ffx/pattern_details_screen.py b/src/ffx/pattern_details_screen.py index 74a0945..e9ca51e 100644 --- a/src/ffx/pattern_details_screen.py +++ b/src/ffx/pattern_details_screen.py @@ -18,6 +18,7 @@ from .screen_support import ( add_auto_table_column, build_screen_bootstrap, build_screen_controllers, + build_screen_log_pane, go_back_or_exit, populate_tag_table, ) @@ -482,6 +483,7 @@ class PatternDetailsScreen(Screen): # Row 20 yield Static(" ", classes="seven") + yield build_screen_log_pane() yield Footer() diff --git a/src/ffx/screen_support.py b/src/ffx/screen_support.py index bfe565b..a60e497 100644 --- a/src/ffx/screen_support.py +++ b/src/ffx/screen_support.py @@ -6,8 +6,10 @@ from dataclasses import dataclass from rich.cells import cell_len from rich.measure import measure_renderables from rich.text import Text +from textual.widgets import Collapsible, RichLog from .helper import formatRichColor +from .i18n import t from .pattern_controller import PatternController from .show_controller import ShowController from .shifted_season_controller import ShiftedSeasonController @@ -16,6 +18,10 @@ from .tmdb_controller import TmdbController from .track_controller import TrackController +SCREEN_LOG_PANE_ID = "screen_log_pane" +SCREEN_LOG_VIEW_ID = "screen_log_view" + + @dataclass(frozen=True) class ScreenBootstrap: context: dict @@ -143,6 +149,60 @@ def update_table_column_label(table, column_key, label) -> None: table.refresh() +def build_screen_log_pane() -> Collapsible: + """Create a shared collapsible log pane for screen-local diagnostics.""" + + logView = RichLog( + id=SCREEN_LOG_VIEW_ID, + wrap=True, + markup=False, + highlight=False, + auto_scroll=True, + ) + logView.styles.height = 8 + logView.styles.width = "100%" + + logPane = Collapsible( + logView, + title=t("Log"), + collapsed=True, + id=SCREEN_LOG_PANE_ID, + ) + logPane.styles.width = "100%" + return logPane + + +def toggle_screen_log_pane(screen) -> bool: + """Toggle the current screen log pane when present.""" + + try: + logPane = screen.query_one(f"#{SCREEN_LOG_PANE_ID}", Collapsible) + except Exception: + return False + + logPane.collapsed = not bool(logPane.collapsed) + return True + + +def write_screen_log(screen, message: str) -> bool: + """Append a line to the current screen log pane when present.""" + + if message is None: + return False + + text = str(message).strip() + if not text: + return False + + try: + logView = screen.query_one(f"#{SCREEN_LOG_VIEW_ID}", RichLog) + except Exception: + return False + + logView.write(text) + return True + + def go_back_or_exit(screen) -> None: """Pop the current screen when possible, otherwise exit the app.""" diff --git a/src/ffx/settings_screen.py b/src/ffx/settings_screen.py index 4ea5eb5..99131e3 100644 --- a/src/ffx/settings_screen.py +++ b/src/ffx/settings_screen.py @@ -3,7 +3,7 @@ from textual.screen import Screen from textual.widgets import Footer, Placeholder from .i18n import t -from .screen_support import go_back_or_exit +from .screen_support import build_screen_log_pane, go_back_or_exit class SettingsScreen(Screen): @@ -17,6 +17,7 @@ class SettingsScreen(Screen): def compose(self) -> ComposeResult: # Row 1 yield Placeholder(t("Settings Screen")) + yield build_screen_log_pane() yield Footer() def action_back(self): diff --git a/src/ffx/shifted_season_delete_screen.py b/src/ffx/shifted_season_delete_screen.py index 21f2ede..704182c 100644 --- a/src/ffx/shifted_season_delete_screen.py +++ b/src/ffx/shifted_season_delete_screen.py @@ -6,7 +6,7 @@ from textual.containers import Grid from .i18n import t from .shifted_season_controller import ShiftedSeasonController -from .screen_support import go_back_or_exit +from .screen_support import build_screen_log_pane, go_back_or_exit from ffx.model.shifted_season import ShiftedSeason @@ -127,6 +127,7 @@ class ShiftedSeasonDeleteScreen(Screen): yield Button(t("Delete"), id="delete_button") yield Button(t("Cancel"), id="cancel_button") + yield build_screen_log_pane() yield Footer() diff --git a/src/ffx/shifted_season_details_screen.py b/src/ffx/shifted_season_details_screen.py index 3384142..43047b3 100644 --- a/src/ffx/shifted_season_details_screen.py +++ b/src/ffx/shifted_season_details_screen.py @@ -6,7 +6,7 @@ from textual.containers import Grid from .i18n import t from .shifted_season_controller import ShiftedSeasonController -from .screen_support import go_back_or_exit +from .screen_support import build_screen_log_pane, go_back_or_exit from ffx.model.shifted_season import ShiftedSeason @@ -175,6 +175,7 @@ class ShiftedSeasonDetailsScreen(Screen): # Row 10 yield Static(" ", classes="three") + yield build_screen_log_pane() yield Footer() diff --git a/src/ffx/show_delete_screen.py b/src/ffx/show_delete_screen.py index 4622a50..77bf1d9 100644 --- a/src/ffx/show_delete_screen.py +++ b/src/ffx/show_delete_screen.py @@ -4,7 +4,7 @@ from textual.containers import Grid from .i18n import t from .show_controller import ShowController -from .screen_support import go_back_or_exit +from .screen_support import build_screen_log_pane, go_back_or_exit # Screen[dict[int, str, int]] class ShowDeleteScreen(Screen): @@ -89,6 +89,7 @@ class ShowDeleteScreen(Screen): yield Button(t("Cancel"), id="cancel_button") + yield build_screen_log_pane() yield Footer() diff --git a/src/ffx/show_details_screen.py b/src/ffx/show_details_screen.py index 983aaad..85ed8d2 100644 --- a/src/ffx/show_details_screen.py +++ b/src/ffx/show_details_screen.py @@ -21,6 +21,7 @@ from .screen_support import ( add_auto_table_column, build_screen_bootstrap, build_screen_controllers, + build_screen_log_pane, go_back_or_exit, ) @@ -433,6 +434,7 @@ class ShowDetailsScreen(Screen): yield Button(t("Cancel"), id="cancel_button") + yield build_screen_log_pane() yield Footer() diff --git a/src/ffx/shows_screen.py b/src/ffx/shows_screen.py index 66aa54d..ca0e91a 100644 --- a/src/ffx/shows_screen.py +++ b/src/ffx/shows_screen.py @@ -5,7 +5,12 @@ from rich.text import Text from .i18n import t from .show_controller import ShowController -from .screen_support import add_auto_table_column, go_back_or_exit, update_table_column_label +from .screen_support import ( + add_auto_table_column, + build_screen_log_pane, + go_back_or_exit, + update_table_column_label, +) from .show_details_screen import ShowDetailsScreen from .show_delete_screen import ShowDeleteScreen @@ -278,4 +283,5 @@ class ShowsScreen(Screen): f = Footer() f.description = "yolo" + yield build_screen_log_pane() yield f diff --git a/src/ffx/tag_delete_screen.py b/src/ffx/tag_delete_screen.py index d0fc477..5382e17 100644 --- a/src/ffx/tag_delete_screen.py +++ b/src/ffx/tag_delete_screen.py @@ -3,7 +3,7 @@ from textual.widgets import Header, Footer, Static, Button from textual.containers import Grid from .i18n import t -from .screen_support import go_back_or_exit +from .screen_support import build_screen_log_pane, go_back_or_exit # Screen[dict[int, str, int]] @@ -92,6 +92,7 @@ class TagDeleteScreen(Screen): yield Button(t("Delete"), id="delete_button") yield Button(t("Cancel"), id="cancel_button") + yield build_screen_log_pane() yield Footer() diff --git a/src/ffx/tag_details_screen.py b/src/ffx/tag_details_screen.py index 7feb3fc..ccdda79 100644 --- a/src/ffx/tag_details_screen.py +++ b/src/ffx/tag_details_screen.py @@ -3,7 +3,7 @@ from textual.widgets import Header, Footer, Static, Button, Input from textual.containers import Grid from .i18n import t -from .screen_support import go_back_or_exit +from .screen_support import build_screen_log_pane, go_back_or_exit # Screen[dict[int, str, int]] @@ -121,6 +121,7 @@ class TagDetailsScreen(Screen): # Row 6 yield Static(" ", classes="five", id="messagestatic") + yield build_screen_log_pane() yield Footer(id="footer") diff --git a/src/ffx/track_delete_screen.py b/src/ffx/track_delete_screen.py index a681fec..edd8dd2 100644 --- a/src/ffx/track_delete_screen.py +++ b/src/ffx/track_delete_screen.py @@ -6,7 +6,7 @@ from textual.containers import Grid from ffx.track_descriptor import TrackDescriptor from .i18n import t -from .screen_support import go_back_or_exit +from .screen_support import build_screen_log_pane, go_back_or_exit # Screen[dict[int, str, int]] @@ -118,6 +118,7 @@ class TrackDeleteScreen(Screen): yield Button(t("Delete"), id="delete_button") yield Button(t("Cancel"), id="cancel_button") + yield build_screen_log_pane() yield Footer() diff --git a/src/ffx/track_details_screen.py b/src/ffx/track_details_screen.py index a6df8ff..aca5071 100644 --- a/src/ffx/track_details_screen.py +++ b/src/ffx/track_details_screen.py @@ -14,7 +14,13 @@ from .track_descriptor import TrackDescriptor from .track_disposition import TrackDisposition from .track_type import TrackType from .i18n import t -from .screen_support import add_auto_table_column, build_screen_bootstrap, go_back_or_exit, populate_tag_table +from .screen_support import ( + add_auto_table_column, + build_screen_bootstrap, + build_screen_log_pane, + go_back_or_exit, + populate_tag_table, +) class TrackDetailsScreen(Screen): @@ -128,6 +134,9 @@ class TrackDetailsScreen(Screen): self.__patternLabel = str(patternLabel) self.__siblingTrackDescriptors = list(siblingTrackDescriptors or []) self.__metadataOnly = bool(metadata_only) + self.__applyNormalization = bool( + self.context.get("apply_metadata_normalization", True) + ) if self.__isNew: self.__trackType = trackType @@ -152,8 +161,13 @@ class TrackDetailsScreen(Screen): initial_language = trackDescriptor.getLanguage() initial_title = trackDescriptor.getTitle() - self.__titleAutoManaged = ( - initial_language == IsoLanguage.UNDEFINED and not str(initial_title).strip() + initialTitleEmpty = not str(initial_title).strip() + self.__titleAutoManaged = bool( + initialTitleEmpty + and ( + initial_language == IsoLanguage.UNDEFINED + or (self.__metadataOnly and self.__applyNormalization) + ) ) self.__suppressTitleChanged = False self.__lastAutoTitle = "" @@ -256,6 +270,8 @@ class TrackDetailsScreen(Screen): self.__trackDescriptor.getLanguage() ) self.query_one("#title_input", Input).value = self.__trackDescriptor.getTitle() + if self.__titleAutoManaged and not self.__trackDescriptor.getTitle().strip(): + self._apply_auto_title_for_language(self.__trackDescriptor.getLanguage()) self.updateTags() if self.__metadataOnly: @@ -387,6 +403,7 @@ class TrackDetailsScreen(Screen): # Row 24 yield Static(" ", classes="five", id="messagestatic") + yield build_screen_log_pane() yield Footer(id="footer") def getTrackDescriptorFromInput(self): diff --git a/tests/unit/test_media_descriptor_change_set.py b/tests/unit/test_media_descriptor_change_set.py index 8e196ec..9e475c1 100644 --- a/tests/unit/test_media_descriptor_change_set.py +++ b/tests/unit/test_media_descriptor_change_set.py @@ -247,7 +247,8 @@ class MediaDescriptorChangeSetTests(unittest.TestCase): self.assertIn("title=German", metadata_tokens) self.assertNotIn("title=Deutsch", metadata_tokens) - def test_non_subtitle_track_without_title_does_not_get_language_name(self): + def test_audio_track_without_title_gets_language_name_when_normalization_enabled(self): + set_current_language("de") context = { "logger": get_ffx_logger(), "config": StaticConfig({}), @@ -278,6 +279,73 @@ class MediaDescriptorChangeSetTests(unittest.TestCase): self.assertIn("-metadata:s:a:0", metadata_tokens) self.assertIn("language=deu", metadata_tokens) + self.assertIn("title=Deutsch", metadata_tokens) + + def test_video_track_without_title_gets_language_name_when_normalization_enabled(self): + set_current_language("de") + context = { + "logger": get_ffx_logger(), + "config": StaticConfig({}), + } + + source_track = TrackDescriptor( + index=0, + source_index=0, + sub_index=0, + track_type=TrackType.VIDEO, + tags={"language": "ger"}, + ) + target_track = TrackDescriptor( + index=0, + source_index=0, + sub_index=0, + track_type=TrackType.VIDEO, + tags={"language": "ger"}, + ) + + change_set = MediaDescriptorChangeSet( + context, + MediaDescriptor(track_descriptors=[target_track]), + MediaDescriptor(track_descriptors=[source_track]), + ) + + metadata_tokens = change_set.generateMetadataTokens() + + self.assertIn("language=deu", metadata_tokens) + self.assertIn("title=Deutsch", metadata_tokens) + + def test_changed_track_language_does_not_autofill_title_when_title_already_exists(self): + set_current_language("de") + context = { + "logger": get_ffx_logger(), + "config": StaticConfig({}), + } + + source_track = TrackDescriptor( + index=0, + source_index=0, + sub_index=0, + track_type=TrackType.SUBTITLE, + tags={"language": "ger", "title": "Deutsch [FN]"}, + ) + target_track = TrackDescriptor( + index=0, + source_index=0, + sub_index=0, + track_type=TrackType.SUBTITLE, + tags={"language": "jpn", "title": "Deutsch [FN]"}, + ) + + change_set = MediaDescriptorChangeSet( + context, + MediaDescriptor(track_descriptors=[target_track]), + MediaDescriptor(track_descriptors=[source_track]), + ) + + metadata_tokens = change_set.generateMetadataTokens() + + self.assertIn("language=jpn", metadata_tokens) + self.assertNotIn("title=Japanisch", metadata_tokens) self.assertNotIn("title=Deutsch", metadata_tokens) def test_target_only_tracks_still_emit_remove_tokens_for_configured_stream_keys(self): diff --git a/tests/unit/test_metadata_editor.py b/tests/unit/test_metadata_editor.py index 753d544..bcbb0fa 100644 --- a/tests/unit/test_metadata_editor.py +++ b/tests/unit/test_metadata_editor.py @@ -18,6 +18,7 @@ from ffx.logging_utils import get_ffx_logger # noqa: E402 from ffx.media_descriptor import MediaDescriptor # noqa: E402 from ffx.metadata_editor import ( # noqa: E402 apply_metadata_edits, + build_metadata_edit_command, build_metadata_edit_context, create_temporary_output_path, ) @@ -77,15 +78,45 @@ class MetadataEditorTests(unittest.TestCase): self.assertEqual(".mkv", Path(temporary_path).suffix) self.assertEqual(Path(source_path).parent, Path(temporary_path).parent) + def test_build_metadata_edit_command_maps_all_streams_and_uses_single_copy_codec(self): + context = build_metadata_edit_context(make_context()) + baseline_descriptor = make_descriptor() + draft_descriptor = baseline_descriptor.clone(context=context) + + command = build_metadata_edit_command( + context, + "/tmp/example.mkv", + "/tmp/.edit.mkv", + baseline_descriptor, + draft_descriptor, + ) + + self.assertEqual(1, command.count("-map")) + self.assertEqual(1, command.count("-c")) + self.assertNotIn("-c:v:0", command) + self.assertNotIn("-c:a:0", command) + self.assertNotIn("-c:s:0", command) + self.assertEqual( + ["-map", "0", "-c", "copy"], + command[command.index("-map"):command.index("-c") + 2], + ) + def test_apply_metadata_edits_rewrites_via_temporary_file_then_replaces_source(self): context = make_context() baseline_descriptor = make_descriptor() draft_descriptor = baseline_descriptor.clone(context=context) source_path = "/tmp/example.mkv" + expected_command = build_metadata_edit_command( + build_metadata_edit_context(context), + source_path, + "/tmp/.edit.mkv", + baseline_descriptor, + draft_descriptor, + ) with ( patch("ffx.metadata_editor.create_temporary_output_path", return_value="/tmp/.edit.mkv"), - patch("ffx.metadata_editor.FfxController.runJob") as mocked_run_job, + patch("ffx.metadata_editor.executeProcess", return_value=("", "", 0)) as mocked_execute, patch("ffx.metadata_editor.os.replace") as mocked_replace, ): result = apply_metadata_edits( @@ -95,32 +126,43 @@ class MetadataEditorTests(unittest.TestCase): draft_descriptor, ) - mocked_run_job.assert_called_once_with( - source_path, - "/tmp/.edit.mkv", - targetFormat="", - chainIteration=[], - ) + mocked_execute.assert_called_once_with(expected_command, context=build_metadata_edit_context(context)) mocked_replace.assert_called_once_with("/tmp/.edit.mkv", source_path) self.assertEqual( { "applied": True, "dry_run": False, "target_path": source_path, + "command_sequence": expected_command, + }, + { + "applied": result["applied"], + "dry_run": result["dry_run"], + "target_path": result["target_path"], + "command_sequence": result["command_sequence"], }, - result, ) + self.assertIn("timings", result) + self.assertIn("ffmpeg_seconds", result["timings"]) + self.assertIn("replace_seconds", result["timings"]) + self.assertIn("write_seconds", result["timings"]) def test_apply_metadata_edits_dry_run_skips_replace_and_cleans_temp_path(self): context = make_context(dry_run=True) baseline_descriptor = make_descriptor() draft_descriptor = baseline_descriptor.clone(context=context) + notifications = [] + expected_command = build_metadata_edit_command( + build_metadata_edit_context(context), + "/tmp/example.mkv", + "/tmp/.edit.mkv", + baseline_descriptor, + draft_descriptor, + ) with ( patch("ffx.metadata_editor.create_temporary_output_path", return_value="/tmp/.edit.mkv"), - patch("ffx.metadata_editor.FfxController.runJob") as mocked_run_job, - patch("ffx.metadata_editor.os.path.exists", return_value=True), - patch("ffx.metadata_editor.os.remove") as mocked_remove, + patch("ffx.metadata_editor.executeProcess") as mocked_execute, patch("ffx.metadata_editor.os.replace") as mocked_replace, ): result = apply_metadata_edits( @@ -128,19 +170,57 @@ class MetadataEditorTests(unittest.TestCase): "/tmp/example.mkv", baseline_descriptor, draft_descriptor, + notify=notifications.append, ) - mocked_run_job.assert_called_once() + mocked_execute.assert_not_called() mocked_replace.assert_not_called() - mocked_remove.assert_called_once_with("/tmp/.edit.mkv") + self.assertEqual(["ffmpeg dry-run prepared."], notifications) self.assertEqual( { "applied": False, "dry_run": True, "target_path": "/tmp/.edit.mkv", + "command_sequence": expected_command, + }, + { + "applied": result["applied"], + "dry_run": result["dry_run"], + "target_path": result["target_path"], + "command_sequence": result["command_sequence"], }, - result, ) + self.assertEqual( + { + "ffmpeg_seconds": 0.0, + "replace_seconds": 0.0, + "write_seconds": 0.0, + }, + result["timings"], + ) + + def test_apply_metadata_edits_notifies_with_command_when_verbose(self): + context = make_context() + context["verbosity"] = 1 + baseline_descriptor = make_descriptor() + draft_descriptor = baseline_descriptor.clone(context=context) + notifications = [] + + with ( + patch("ffx.metadata_editor.create_temporary_output_path", return_value="/tmp/.edit.mkv"), + patch("ffx.metadata_editor.executeProcess", return_value=("", "", 0)), + patch("ffx.metadata_editor.os.replace"), + ): + apply_metadata_edits( + context, + "/tmp/example.mkv", + baseline_descriptor, + draft_descriptor, + notify=notifications.append, + ) + + self.assertEqual(1, len(notifications)) + self.assertTrue(notifications[0].startswith("ffmpeg: ffmpeg ")) if __name__ == "__main__": diff --git a/tests/unit/test_tag_table_screen_state.py b/tests/unit/test_tag_table_screen_state.py index f3230a6..2360d53 100644 --- a/tests/unit/test_tag_table_screen_state.py +++ b/tests/unit/test_tag_table_screen_state.py @@ -99,6 +99,7 @@ class FakeMediaDescriptor: class FakeValueWidget: def __init__(self, value): self.value = value + self.disabled = False class FakeInputWidget: @@ -106,10 +107,21 @@ class FakeInputWidget: self.value = value +class FakeStaticWidget: + def __init__(self, value=""): + self.value = value + + def update(self, value): + self.value = value + + class FakeSelectionListWidget: def __init__(self, selected): self.selected = selected + def add_option(self, _option): + return None + def make_track_descriptor(index, sub_index, track_type): return TrackDescriptor( @@ -244,6 +256,49 @@ class TagTableScreenStateTests(unittest.TestCase): self.assertEqual("Preset", widgets["#title_input"].value) + def test_track_details_screen_metadata_only_mount_shows_normalized_title_preview(self): + set_current_language("de") + screen = object.__new__(TrackDetailsScreen) + screen._TrackDetailsScreen__index = 2 + screen._TrackDetailsScreen__subIndex = 0 + screen._TrackDetailsScreen__patternLabel = "demo" + screen._TrackDetailsScreen__trackType = TrackType.AUDIO + screen._TrackDetailsScreen__audioLayout = AudioLayout.LAYOUT_STEREO + screen._TrackDetailsScreen__trackDescriptor = TrackDescriptor( + index=2, + source_index=2, + sub_index=0, + track_type=TrackType.AUDIO, + codec_name=TrackCodec.DTS, + audio_layout=AudioLayout.LAYOUT_STEREO, + tags={"language": "ger"}, + ) + screen._TrackDetailsScreen__metadataOnly = True + screen._TrackDetailsScreen__titleAutoManaged = True + screen._TrackDetailsScreen__suppressTitleChanged = False + screen._TrackDetailsScreen__lastAutoTitle = "" + screen._TrackDetailsScreen__removeTrackKeys = [] + screen._TrackDetailsScreen__ignoreTrackKeys = [] + screen._TrackDetailsScreen__draftTrackTags = {} + screen._TrackDetailsScreen__tagRowData = {} + screen.updateTags = lambda: None + + widgets = { + "#index_label": FakeStaticWidget(), + "#subindex_label": FakeStaticWidget(), + "#pattern_label": FakeStaticWidget(), + "#type_select": FakeValueWidget(None), + "#audio_layout_select": FakeValueWidget(None), + "#dispositions_selection_list": FakeSelectionListWidget(set()), + "#language_select": FakeValueWidget(None), + "#title_input": FakeInputWidget(""), + } + screen.query_one = lambda selector, _widget_type=None: widgets[selector] + + screen.on_mount() + + self.assertEqual("Deutsch", widgets["#title_input"].value) + def test_track_details_screen_language_options_are_sorted_by_localized_label(self): set_current_language("de") @@ -326,12 +381,84 @@ class TagTableScreenStateTests(unittest.TestCase): screen.tracksTable = FakeTagTable() screen._sourceMediaDescriptor = FakeMediaDescriptor([first_track]) screen._trackRowData = {} + screen._applyNormalization = False screen.updateTracks() self.assertEqual(9, len(screen.tracksTable.columns)) self.assertIn("A much longer updated title", screen.tracksTable.rows["row-0"]) + def test_media_edit_screen_shows_normalized_audio_title_preview(self): + set_current_language("de") + audio_track = TrackDescriptor( + index=1, + source_index=1, + sub_index=0, + track_type=TrackType.AUDIO, + codec_name=TrackCodec.DTS, + audio_layout=AudioLayout.LAYOUT_STEREO, + tags={"language": "ger"}, + ) + + screen = object.__new__(MediaEditScreen) + screen.tracksTable = FakeTagTable() + screen._sourceMediaDescriptor = FakeMediaDescriptor([audio_track]) + screen._trackRowData = {} + screen._applyNormalization = True + + screen.updateTracks() + + self.assertIn("Deutsch", screen.tracksTable.rows["row-0"]) + + def test_media_edit_screen_shows_normalized_video_title_preview(self): + set_current_language("de") + video_track = TrackDescriptor( + index=0, + source_index=0, + sub_index=0, + track_type=TrackType.VIDEO, + codec_name=TrackCodec.H264, + tags={"language": "ger"}, + ) + + screen = object.__new__(MediaEditScreen) + screen.tracksTable = FakeTagTable() + screen._sourceMediaDescriptor = FakeMediaDescriptor([video_track]) + screen._trackRowData = {} + screen._applyNormalization = True + + screen.updateTracks() + + self.assertIn("Deutsch", screen.tracksTable.rows["row-0"]) + + def test_media_edit_screen_toggle_normalization_refreshes_tracks(self): + screen = object.__new__(MediaEditScreen) + screen._applyNormalization = False + + calls = [] + + screen.setApplyNormalization = lambda enabled: ( + setattr(screen, "_applyNormalization", bool(enabled)), + calls.append("setApplyNormalization"), + ) + screen.updateToggleButtons = lambda: calls.append("updateToggleButtons") + screen.updateTracks = lambda: calls.append("updateTracks") + screen.updateDifferences = lambda: calls.append("updateDifferences") + screen.setMessage = lambda _message: calls.append("setMessage") + + screen.action_toggle_normalization() + + self.assertEqual( + [ + "setApplyNormalization", + "updateToggleButtons", + "updateTracks", + "updateDifferences", + "setMessage", + ], + calls, + ) + def test_pattern_details_screen_reads_selected_shifted_season_from_row_mapping(self): screen = object.__new__(PatternDetailsScreen) screen.shiftedSeasonsTable = FakeTagTable()