Skip to content

Commit 68ee076

Browse files
committed
fix: comprehensive permission error handling and UI improvements
- Enhanced directory creation with proper permissions (0o755) - Added pre-conversion permission checks and test writes - Improved error messages for permission issues - Fixed File column to show output filename with new extension - Added right-click context menu for Active Conversions table: * Play Media - opens converted file with default player * Open Output Folder - opens folder containing output file - Added permission verification before FFmpeg execution - Attempts to fix permissions automatically when possible - Better handling of existing files during overwrite operations Fixes permission denied errors when writing to output directory
1 parent e357bf9 commit 68ee076

3 files changed

Lines changed: 192 additions & 6 deletions

File tree

handforge/core/orchestrator.py

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,17 @@ def run(self):
7474
return
7575

7676
# Build output path
77-
dst = out_path(self.job.dst_dir, self.job.src, self.job.format)
77+
try:
78+
dst = out_path(self.job.dst_dir, self.job.src, self.job.format)
79+
except PermissionError as e:
80+
self.sig_done.emit(self.wid, False, f"Permission error: {str(e)}", "")
81+
return
82+
83+
# Verify output directory is writable before proceeding
84+
output_dir = os.path.dirname(dst)
85+
if not os.access(output_dir, os.W_OK):
86+
self.sig_done.emit(self.wid, False, f"Output directory is not writable: {output_dir}. Please check permissions or choose a different output directory.", "")
87+
return
7888

7989
# Check if output exists
8090
if os.path.exists(dst):
@@ -88,6 +98,15 @@ def run(self):
8898
while os.path.exists(dst):
8999
dst = f"{base} ({counter}){ext}"
90100
counter += 1
101+
elif on_exists == "overwrite":
102+
# Try to make existing file writable if we're overwriting
103+
try:
104+
import stat
105+
current_mode = os.stat(dst).st_mode
106+
os.chmod(dst, current_mode | stat.S_IWUSR)
107+
except (PermissionError, OSError):
108+
# If we can't fix permissions, FFmpeg will fail with a clear error
109+
pass
91110

92111
# For two-pass encoding, first pass outputs to null device
93112
first_pass_dst = dst
@@ -150,6 +169,22 @@ def run(self):
150169
# Run FFmpeg
151170
start_time = time.time()
152171

172+
# Verify we can write to the output location before starting FFmpeg
173+
output_dir = os.path.dirname(dst)
174+
try:
175+
# Test write permission by trying to create a temporary file
176+
import tempfile
177+
test_file = os.path.join(output_dir, f".handforge_test_{os.getpid()}")
178+
try:
179+
with open(test_file, 'w') as f:
180+
f.write('test')
181+
os.remove(test_file)
182+
except (PermissionError, OSError) as e:
183+
self.sig_done.emit(self.wid, False, f"Cannot write to output directory '{output_dir}': {e}. Please check permissions or choose a different output directory.", "")
184+
return
185+
except Exception as e:
186+
self.sig_log.emit(self.wid, f"Warning: Could not verify write permissions: {e}")
187+
153188
# Log command details for debugging
154189
self.sig_log.emit(self.wid, f"Starting conversion: {os.path.basename(self.job.src)}")
155190
self.sig_log.emit(self.wid, f"Format: {self.job.format}, Mode: {self.job.mode}")

handforge/ui/main_window.py

Lines changed: 120 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,10 @@ def init_ui(self):
281281
# Connect cell clicked signal for View Log button
282282
self.workers_table.cellClicked.connect(self._on_workers_table_cell_clicked)
283283

284+
# Enable context menu for Active Conversions table
285+
self.workers_table.setContextMenuPolicy(Qt.ContextMenuPolicy.CustomContextMenu)
286+
self.workers_table.customContextMenuRequested.connect(self.show_workers_context_menu)
287+
284288
splitter.addWidget(workers_group)
285289
splitter.setSizes([450, 750])
286290

@@ -900,6 +904,12 @@ def _on_worker_done(self, wid: int, success: bool, message: str, output_path: st
900904
self.workers_data[wid]["status"] = "Done" if success else "Failed"
901905
self.workers_data[wid]["message"] = message
902906
self.workers_data[wid]["output_path"] = output_path
907+
# Update file name to show output filename with new extension
908+
if output_path and os.path.exists(output_path):
909+
self.workers_data[wid]["file"] = os.path.basename(output_path)
910+
elif output_path:
911+
# Even if file doesn't exist (failed), show intended output filename
912+
self.workers_data[wid]["file"] = os.path.basename(output_path)
903913
self.update_workers_table()
904914

905915
# Show notification with file location if successful
@@ -967,9 +977,11 @@ def _on_tray_activated(self, reason):
967977

968978
def _on_worker_started(self, wid: int, file_path: str):
969979
"""Handle worker started - initialize worker data with file name."""
980+
# Store input file path for reference
970981
file_name = os.path.basename(file_path)
971982
self.workers_data[wid] = {
972-
"file": file_name,
983+
"file": file_name, # Will be updated to output filename when conversion completes
984+
"input_path": file_path, # Store input path
973985
"elapsed": 0,
974986
"eta": 0,
975987
"speed": "0.0x",
@@ -985,12 +997,15 @@ def _on_worker_progress(self, wid: int, progress: float, elapsed: float, eta: fl
985997
if wid not in self.workers_data:
986998
# Get file name from orchestrator if available
987999
file_name = ""
1000+
input_path = ""
9881001
if wid in self.orchestrator.workers:
9891002
worker = self.orchestrator.workers[wid]
990-
file_name = os.path.basename(worker.job.src)
1003+
input_path = worker.job.src
1004+
file_name = os.path.basename(input_path)
9911005

9921006
self.workers_data[wid] = {
9931007
"file": file_name,
1008+
"input_path": input_path,
9941009
"elapsed": 0,
9951010
"eta": 0,
9961011
"speed": "",
@@ -1012,6 +1027,7 @@ def _on_worker_log(self, wid: int, log_line: str):
10121027
if wid not in self.workers_data:
10131028
self.workers_data[wid] = {
10141029
"file": "",
1030+
"input_path": "",
10151031
"elapsed": 0,
10161032
"eta": 0,
10171033
"speed": "",
@@ -1023,6 +1039,28 @@ def _on_worker_log(self, wid: int, log_line: str):
10231039
self.workers_data[wid]["log"].append(log_line)
10241040
if len(self.workers_data[wid]["log"]) > 100:
10251041
self.workers_data[wid]["log"].pop(0)
1042+
1043+
# Try to extract output path from log if available
1044+
if "Output:" in log_line and not self.workers_data[wid].get("output_path"):
1045+
try:
1046+
output_path = log_line.split("Output:", 1)[1].strip()
1047+
if output_path:
1048+
self.workers_data[wid]["output_path"] = output_path
1049+
# Update file name to output filename if file exists
1050+
if os.path.exists(output_path):
1051+
self.workers_data[wid]["file"] = os.path.basename(output_path)
1052+
except:
1053+
pass
1054+
1055+
# Try to extract output path from log if available
1056+
if "Output:" in log_line and not self.workers_data[wid].get("output_path"):
1057+
try:
1058+
output_path = log_line.split("Output:", 1)[1].strip()
1059+
if output_path and os.path.exists(output_path):
1060+
self.workers_data[wid]["output_path"] = output_path
1061+
self.workers_data[wid]["file"] = os.path.basename(output_path)
1062+
except:
1063+
pass
10261064

10271065
def update_workers_table(self):
10281066
"""Update the workers table."""
@@ -1380,6 +1418,86 @@ def remove_selected_from_queue(self):
13801418
self.update_queue_table()
13811419
self.update_queue_count()
13821420

1421+
def show_workers_context_menu(self, position):
1422+
"""Show context menu for Active Conversions table."""
1423+
menu = QMenu(self)
1424+
1425+
# Get selected rows
1426+
selected_rows = self.workers_table.selectionModel().selectedRows()
1427+
if not selected_rows:
1428+
menu.exec(self.workers_table.viewport().mapToGlobal(position))
1429+
return
1430+
1431+
# Get the first selected row's worker data
1432+
row = selected_rows[0].row()
1433+
wid_item = self.workers_table.item(row, 0)
1434+
if not wid_item:
1435+
menu.exec(self.workers_table.viewport().mapToGlobal(position))
1436+
return
1437+
1438+
wid = int(wid_item.text())
1439+
if wid not in self.workers_data:
1440+
menu.exec(self.workers_table.viewport().mapToGlobal(position))
1441+
return
1442+
1443+
data = self.workers_data[wid]
1444+
output_path = data.get("output_path", "")
1445+
status = data.get("status", "")
1446+
1447+
# Only show menu options for completed conversions with valid output
1448+
if status == "Done" and output_path and os.path.exists(output_path):
1449+
menu.addAction("▶️ Play Media", lambda: self.play_media(output_path))
1450+
menu.addSeparator()
1451+
menu.addAction("📁 Open Output Folder", lambda: self.open_output_folder(output_path))
1452+
elif output_path:
1453+
# Even if failed, allow opening the folder
1454+
menu.addAction("📁 Open Output Folder", lambda: self.open_output_folder(output_path))
1455+
else:
1456+
menu.addAction("No output file available")
1457+
1458+
menu.exec(self.workers_table.viewport().mapToGlobal(position))
1459+
1460+
def play_media(self, file_path: str):
1461+
"""Open media file with default system player."""
1462+
import subprocess
1463+
import platform
1464+
1465+
if not os.path.exists(file_path):
1466+
QMessageBox.warning(self, "File Not Found", f"The file does not exist:\n{file_path}")
1467+
return
1468+
1469+
try:
1470+
system = platform.system()
1471+
if system == "Windows":
1472+
os.startfile(file_path)
1473+
elif system == "Darwin": # macOS
1474+
subprocess.run(["open", file_path], check=True)
1475+
else: # Linux and others
1476+
subprocess.run(["xdg-open", file_path], check=True)
1477+
except Exception as e:
1478+
QMessageBox.critical(self, "Error", f"Failed to open media file:\n{str(e)}")
1479+
1480+
def open_output_folder(self, file_path: str):
1481+
"""Open the folder containing the output file."""
1482+
import subprocess
1483+
import platform
1484+
1485+
folder_path = os.path.dirname(file_path)
1486+
if not os.path.exists(folder_path):
1487+
QMessageBox.warning(self, "Folder Not Found", f"The folder does not exist:\n{folder_path}")
1488+
return
1489+
1490+
try:
1491+
system = platform.system()
1492+
if system == "Windows":
1493+
os.startfile(folder_path)
1494+
elif system == "Darwin": # macOS
1495+
subprocess.run(["open", folder_path], check=True)
1496+
else: # Linux and others
1497+
subprocess.run(["xdg-open", folder_path], check=True)
1498+
except Exception as e:
1499+
QMessageBox.critical(self, "Error", f"Failed to open folder:\n{str(e)}")
1500+
13831501
def clear_completed_workers(self):
13841502
"""Clear completed and failed workers from the Active Conversions table."""
13851503
# Find workers that are done or failed

handforge/util/ffmpeg.py

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -810,8 +810,28 @@ def out_path(dst_dir: str, src: str, fmt: str) -> str:
810810
# Expand user directory
811811
dst_dir = os.path.expanduser(dst_dir)
812812

813-
# Create directory if it doesn't exist
814-
os.makedirs(dst_dir, exist_ok=True)
813+
# Create directory if it doesn't exist with proper permissions
814+
try:
815+
os.makedirs(dst_dir, mode=0o755, exist_ok=True)
816+
except PermissionError:
817+
# If permission denied, try with default mode
818+
try:
819+
os.makedirs(dst_dir, exist_ok=True)
820+
except PermissionError as e:
821+
raise PermissionError(f"Cannot create output directory '{dst_dir}': {e}. Please check permissions or choose a different output directory.")
822+
823+
# Verify directory is writable
824+
if not os.access(dst_dir, os.W_OK):
825+
# Try to fix permissions if possible
826+
try:
827+
import stat
828+
current_mode = os.stat(dst_dir).st_mode
829+
# Add write permission for owner
830+
os.chmod(dst_dir, current_mode | stat.S_IWUSR)
831+
if not os.access(dst_dir, os.W_OK):
832+
raise PermissionError(f"Output directory '{dst_dir}' is not writable. Please check permissions or choose a different output directory.")
833+
except (PermissionError, OSError) as e:
834+
raise PermissionError(f"Output directory '{dst_dir}' is not writable: {e}. Please check permissions or choose a different output directory.")
815835

816836
# Get source filename without extension
817837
src_basename = os.path.basename(src)
@@ -853,4 +873,17 @@ def out_path(dst_dir: str, src: str, fmt: str) -> str:
853873
}
854874

855875
ext = ext_map.get(fmt.lower(), f".{fmt}")
856-
return os.path.join(dst_dir, f"{src_name}{ext}")
876+
output_file = os.path.join(dst_dir, f"{src_name}{ext}")
877+
878+
# Check if output file exists and has wrong permissions
879+
if os.path.exists(output_file):
880+
try:
881+
# Try to make file writable if it exists
882+
import stat
883+
current_mode = os.stat(output_file).st_mode
884+
os.chmod(output_file, current_mode | stat.S_IWUSR)
885+
except (PermissionError, OSError):
886+
# If we can't fix permissions, that's okay - FFmpeg will handle overwrite
887+
pass
888+
889+
return output_file

0 commit comments

Comments
 (0)