fix: Address all issues from Antoine's review
UI Feedback: - Show what's being captured (screen + mic) during recording - Design/Analysis toggle now shows target KB path - Color-coded status messages (red=error, green=success) Multi-Screen Support: - Added screen selector dropdown - Enumerate displays using Windows API - Pass screen geometry to FFmpeg for specific monitor capture Button State Fixes: - Disable Record button immediately on click (prevent double-click) - Properly enable Pause button when recording starts - All inputs disabled during recording Error Recovery: - Reset UI properly after recording failure - Added 'Reset' button that appears on errors - force_reset() method for emergency recovery - Can always get back to idle state Other: - Better error messages - Capture info label shows screen + mic being used
This commit is contained in:
@@ -237,6 +237,15 @@ class KBCaptureGUI:
|
||||
)
|
||||
self.status_label.pack(pady=(8, 0))
|
||||
|
||||
# Recording info (what's being captured)
|
||||
self.capture_info_label = CTkLabel(
|
||||
timer_inner,
|
||||
text="",
|
||||
font=("", 9),
|
||||
text_color=COLORS["text_muted"],
|
||||
)
|
||||
self.capture_info_label.pack(pady=(4, 0))
|
||||
|
||||
# Recording controls
|
||||
controls = CTkFrame(main, fg_color="transparent")
|
||||
controls.pack(fill="x", pady=12)
|
||||
@@ -351,6 +360,34 @@ class KBCaptureGUI:
|
||||
)
|
||||
self.analysis_btn.grid(row=0, column=1, padx=(4, 0), sticky="ew")
|
||||
|
||||
# Screen selector (for multi-monitor setups)
|
||||
screen_frame = CTkFrame(main, fg_color="transparent")
|
||||
screen_frame.pack(fill="x", pady=(12, 0))
|
||||
|
||||
CTkLabel(
|
||||
screen_frame,
|
||||
text="Capture",
|
||||
font=("Segoe UI Semibold", 11),
|
||||
text_color=COLORS["text_secondary"],
|
||||
).pack(side="left")
|
||||
|
||||
# Get available screens
|
||||
self.screens = self._get_screens()
|
||||
screen_values = ["All Screens"] + [f"Screen {i+1}" for i in range(len(self.screens))]
|
||||
|
||||
self.screen_menu = CTkOptionMenu(
|
||||
screen_frame,
|
||||
values=screen_values,
|
||||
width=120,
|
||||
height=28,
|
||||
font=("", 10),
|
||||
fg_color=COLORS["bg_card"],
|
||||
button_color=COLORS["bg_elevated"],
|
||||
button_hover_color=COLORS["border"],
|
||||
)
|
||||
self.screen_menu.pack(side="right")
|
||||
self.screen_menu.set("All Screens")
|
||||
|
||||
# Spacer
|
||||
CTkFrame(main, fg_color="transparent").pack(fill="both", expand=True)
|
||||
|
||||
@@ -365,13 +402,29 @@ class KBCaptureGUI:
|
||||
self.folder_label.pack(pady=(12, 0))
|
||||
self.folder_label.bind("<Button-1>", lambda e: self._browse_folder())
|
||||
|
||||
# Hotkey hints
|
||||
# Bottom row: hints + reset
|
||||
bottom_row = CTkFrame(main, fg_color="transparent")
|
||||
bottom_row.pack(fill="x", pady=(8, 0))
|
||||
|
||||
CTkLabel(
|
||||
main,
|
||||
text="Ctrl+Shift+R: Record/Stop • Ctrl+Shift+P: Pause/Resume",
|
||||
bottom_row,
|
||||
text="Ctrl+Shift+R: Record/Stop",
|
||||
font=("", 9),
|
||||
text_color=COLORS["text_muted"],
|
||||
).pack(pady=(8, 0))
|
||||
).pack(side="left")
|
||||
|
||||
# Reset button (hidden until needed)
|
||||
self.reset_btn = CTkButton(
|
||||
bottom_row,
|
||||
text="🔄 Reset",
|
||||
width=60,
|
||||
height=24,
|
||||
font=("", 9),
|
||||
fg_color=COLORS["red"],
|
||||
hover_color="#dc2626",
|
||||
command=self._force_reset,
|
||||
)
|
||||
# Don't pack initially - will show when stuck
|
||||
|
||||
def _set_type(self, type_id: str):
|
||||
"""Set session type."""
|
||||
@@ -379,9 +432,50 @@ class KBCaptureGUI:
|
||||
if type_id == "design":
|
||||
self.design_btn.configure(fg_color=COLORS["blue"])
|
||||
self.analysis_btn.configure(fg_color=COLORS["border"])
|
||||
self.status_label.configure(text="Design session → KB/Design/", text_color=COLORS["blue"])
|
||||
else:
|
||||
self.design_btn.configure(fg_color=COLORS["border"])
|
||||
self.analysis_btn.configure(fg_color=COLORS["orange"])
|
||||
self.status_label.configure(text="Analysis session → KB/Analysis/", text_color=COLORS["orange"])
|
||||
|
||||
def _get_screens(self) -> list:
|
||||
"""Get list of available screens."""
|
||||
screens = []
|
||||
try:
|
||||
if sys.platform == "win32":
|
||||
import ctypes
|
||||
user32 = ctypes.windll.user32
|
||||
|
||||
# EnumDisplayMonitors callback
|
||||
def callback(hMonitor, hdcMonitor, lprcMonitor, dwData):
|
||||
screens.append({
|
||||
"handle": hMonitor,
|
||||
"left": lprcMonitor.contents.left,
|
||||
"top": lprcMonitor.contents.top,
|
||||
"right": lprcMonitor.contents.right,
|
||||
"bottom": lprcMonitor.contents.bottom,
|
||||
})
|
||||
return True
|
||||
|
||||
# Define RECT structure
|
||||
class RECT(ctypes.Structure):
|
||||
_fields_ = [
|
||||
("left", ctypes.c_long),
|
||||
("top", ctypes.c_long),
|
||||
("right", ctypes.c_long),
|
||||
("bottom", ctypes.c_long),
|
||||
]
|
||||
|
||||
MONITORENUMPROC = ctypes.WINFUNCTYPE(
|
||||
ctypes.c_bool, ctypes.c_ulong, ctypes.c_ulong,
|
||||
ctypes.POINTER(RECT), ctypes.c_double
|
||||
)
|
||||
|
||||
user32.EnumDisplayMonitors(None, None, MONITORENUMPROC(callback), 0)
|
||||
except Exception as e:
|
||||
print(f"Error getting screens: {e}")
|
||||
|
||||
return screens if screens else [{"name": "Primary"}]
|
||||
|
||||
def _update_indicator(self):
|
||||
"""Animate recording indicator."""
|
||||
@@ -586,6 +680,10 @@ class KBCaptureGUI:
|
||||
|
||||
def _start_recording(self):
|
||||
"""Start recording session."""
|
||||
# Prevent double-click
|
||||
if self.app and self.app.state != AppState.IDLE:
|
||||
return
|
||||
|
||||
project = self.project_menu.get()
|
||||
if project.startswith("("):
|
||||
messagebox.showwarning("No Project", "Select a project first")
|
||||
@@ -594,20 +692,72 @@ class KBCaptureGUI:
|
||||
name = self.name_entry.get().strip() or f"Session {datetime.now().strftime('%H:%M')}"
|
||||
session_type = SessionType.DESIGN if self.type_var.get() == "design" else SessionType.ANALYSIS
|
||||
|
||||
# Disable Record button immediately to prevent double-click
|
||||
self.record_btn.configure(state="disabled", text="Starting...")
|
||||
self.window.update()
|
||||
|
||||
try:
|
||||
self.app.start_session(name, project, session_type)
|
||||
# Get selected screen
|
||||
screen_selection = self.screen_menu.get()
|
||||
screen_index = None
|
||||
if screen_selection != "All Screens":
|
||||
try:
|
||||
screen_index = int(screen_selection.split()[-1]) - 1
|
||||
except:
|
||||
pass
|
||||
|
||||
self.record_btn.configure(text="Stop", fg_color=COLORS["border"])
|
||||
self.app.start_session(name, project, session_type, screen_index=screen_index)
|
||||
|
||||
# Update UI for recording state
|
||||
self.record_btn.configure(text="Stop", fg_color=COLORS["border"], state="normal")
|
||||
self.pause_btn.configure(state="normal", fg_color=COLORS["orange"])
|
||||
self.project_menu.configure(state="disabled")
|
||||
self.name_entry.configure(state="disabled")
|
||||
self.design_btn.configure(state="disabled")
|
||||
self.analysis_btn.configure(state="disabled")
|
||||
self.screen_menu.configure(state="disabled")
|
||||
self.timer_label.configure(text_color=COLORS["text"])
|
||||
self.status_label.configure(text="Recording...", text_color=COLORS["red"])
|
||||
|
||||
# Show what's being captured
|
||||
capture_info = f"🖥️ {screen_selection}"
|
||||
audio_devices = self.app.recorder.list_audio_devices() if hasattr(self.app, 'recorder') else []
|
||||
if audio_devices:
|
||||
capture_info += f" + 🎤 {audio_devices[0][:20]}"
|
||||
else:
|
||||
capture_info += " (no mic)"
|
||||
self.capture_info_label.configure(text=capture_info)
|
||||
|
||||
except Exception as e:
|
||||
messagebox.showerror("Error", str(e))
|
||||
# Reset UI on failure
|
||||
self._reset_ui_to_idle()
|
||||
messagebox.showerror("Recording Failed", str(e))
|
||||
|
||||
def _reset_ui_to_idle(self):
|
||||
"""Reset UI to idle state (for error recovery)."""
|
||||
self.record_btn.configure(text="Record", fg_color=COLORS["red"], state="normal")
|
||||
self.pause_btn.configure(state="disabled", text="Pause", fg_color=COLORS["border"])
|
||||
self.project_menu.configure(state="normal")
|
||||
self.name_entry.configure(state="normal")
|
||||
self.design_btn.configure(state="normal")
|
||||
self.analysis_btn.configure(state="normal")
|
||||
self.screen_menu.configure(state="normal")
|
||||
self.timer_label.configure(text="00:00:00", text_color=COLORS["text_muted"])
|
||||
self.capture_info_label.configure(text="")
|
||||
self.status_label.configure(text="Ready to record", text_color=COLORS["text_secondary"])
|
||||
# Hide reset button
|
||||
self.reset_btn.pack_forget()
|
||||
|
||||
def _show_reset_button(self):
|
||||
"""Show reset button when stuck."""
|
||||
self.reset_btn.pack(side="right")
|
||||
|
||||
def _force_reset(self):
|
||||
"""Force reset the app to idle state."""
|
||||
if self.app:
|
||||
self.app.force_reset()
|
||||
self._reset_ui_to_idle()
|
||||
self.status_label.configure(text="Reset complete - ready to record", text_color=COLORS["green"])
|
||||
|
||||
def _stop_recording(self):
|
||||
"""Stop recording and transcribe."""
|
||||
@@ -651,19 +801,25 @@ class KBCaptureGUI:
|
||||
secs = secs % 60
|
||||
self.timer_label.configure(text=f"{hours:02d}:{mins:02d}:{secs:02d}")
|
||||
|
||||
# Back to idle
|
||||
if status.state == AppState.IDLE and self.record_btn.cget("text") != "Record":
|
||||
self.record_btn.configure(text="Record", fg_color=COLORS["red"], state="normal")
|
||||
self.pause_btn.configure(state="disabled", text="Pause", fg_color=COLORS["border"])
|
||||
self.project_menu.configure(state="normal")
|
||||
self.name_entry.configure(state="normal")
|
||||
self.design_btn.configure(state="normal")
|
||||
self.analysis_btn.configure(state="normal")
|
||||
# Back to idle (including after errors)
|
||||
if status.state == AppState.IDLE:
|
||||
self._reset_ui_to_idle()
|
||||
self.name_entry.delete(0, "end")
|
||||
# Keep the timer showing final time if we just finished
|
||||
if "Stopped" in status.message or "Done" in status.message:
|
||||
self.timer_label.configure(text_color=COLORS["green"])
|
||||
|
||||
# Status message
|
||||
if status.message:
|
||||
self.status_label.configure(text=status.message, text_color=COLORS["text_secondary"])
|
||||
# Color code the message
|
||||
if "failed" in status.message.lower() or "error" in status.message.lower():
|
||||
self.status_label.configure(text=status.message, text_color=COLORS["red"])
|
||||
# Show reset button on errors
|
||||
self._show_reset_button()
|
||||
elif "done" in status.message.lower() or "saved" in status.message.lower():
|
||||
self.status_label.configure(text=status.message, text_color=COLORS["green"])
|
||||
else:
|
||||
self.status_label.configure(text=status.message, text_color=COLORS["text_secondary"])
|
||||
|
||||
def _on_close(self):
|
||||
"""Handle window close."""
|
||||
|
||||
@@ -111,6 +111,7 @@ class KBCaptureApp:
|
||||
name: str,
|
||||
project: str,
|
||||
session_type: SessionType = SessionType.DESIGN,
|
||||
screen_index: int = None,
|
||||
) -> Session:
|
||||
"""Start a new session and begin recording."""
|
||||
if self.state != AppState.IDLE:
|
||||
@@ -123,6 +124,7 @@ class KBCaptureApp:
|
||||
config = RecordingConfig(
|
||||
output_path=self.session_manager.get_video_path(),
|
||||
framerate=30,
|
||||
screen_index=screen_index,
|
||||
)
|
||||
|
||||
if self.recorder.start(config):
|
||||
@@ -131,7 +133,7 @@ class KBCaptureApp:
|
||||
self._log(f"Session started: {name}")
|
||||
else:
|
||||
self.session_manager.cancel_session()
|
||||
raise RuntimeError("Failed to start recording")
|
||||
raise RuntimeError("Failed to start recording - check FFmpeg is installed")
|
||||
|
||||
return session
|
||||
|
||||
@@ -177,12 +179,17 @@ class KBCaptureApp:
|
||||
duration = self.recorder.get_duration()
|
||||
|
||||
# Stop recording
|
||||
output = self.recorder.stop()
|
||||
try:
|
||||
output = self.recorder.stop()
|
||||
except Exception as e:
|
||||
self._log(f"Stop error: {e}")
|
||||
output = None
|
||||
|
||||
if not output or not output.exists():
|
||||
self._log("Recording failed - no output")
|
||||
self._log("Recording failed - no output file")
|
||||
self.session_manager.cancel_session()
|
||||
self.state = AppState.IDLE
|
||||
self._update_status("Recording failed - try again")
|
||||
return None
|
||||
|
||||
# Update session
|
||||
@@ -275,13 +282,41 @@ class KBCaptureApp:
|
||||
self._stop_duration_thread()
|
||||
|
||||
if self.recorder.is_recording:
|
||||
self.recorder.stop()
|
||||
try:
|
||||
self.recorder.stop()
|
||||
except:
|
||||
pass
|
||||
|
||||
self.session_manager.cancel_session()
|
||||
self.state = AppState.IDLE
|
||||
self._log("Session cancelled")
|
||||
self._update_status("Cancelled")
|
||||
|
||||
def force_reset(self) -> None:
|
||||
"""Force reset to idle state (emergency recovery)."""
|
||||
self._stop_duration_thread()
|
||||
|
||||
# Force stop recorder
|
||||
if self.recorder.process:
|
||||
try:
|
||||
self.recorder.process.terminate()
|
||||
except:
|
||||
pass
|
||||
|
||||
self.recorder.is_recording = False
|
||||
self.recorder.is_paused = False
|
||||
|
||||
# Cancel any session
|
||||
if self.session_manager.current_session:
|
||||
try:
|
||||
self.session_manager.cancel_session()
|
||||
except:
|
||||
self.session_manager.current_session = None
|
||||
|
||||
self.state = AppState.IDLE
|
||||
self._log("Force reset complete")
|
||||
self._update_status("Ready to record")
|
||||
|
||||
def get_status(self) -> AppStatus:
|
||||
"""Get current status."""
|
||||
session = self.session_manager.current_session
|
||||
|
||||
@@ -26,6 +26,7 @@ class RecordingConfig:
|
||||
audio_codec: str = "aac"
|
||||
crf: int = 23
|
||||
preset: str = "ultrafast"
|
||||
screen_index: Optional[int] = None # None = all screens, 0 = first, 1 = second, etc.
|
||||
|
||||
|
||||
class ScreenRecorder:
|
||||
@@ -65,9 +66,23 @@ class ScreenRecorder:
|
||||
"-f", "gdigrab",
|
||||
"-framerate", str(config.framerate),
|
||||
"-draw_mouse", "1",
|
||||
"-i", "desktop",
|
||||
])
|
||||
|
||||
# Handle screen selection
|
||||
if config.screen_index is not None:
|
||||
# Get screen geometry
|
||||
screens = self._get_screen_geometries()
|
||||
if config.screen_index < len(screens):
|
||||
screen = screens[config.screen_index]
|
||||
# Use offset to capture specific screen
|
||||
cmd.extend([
|
||||
"-offset_x", str(screen["left"]),
|
||||
"-offset_y", str(screen["top"]),
|
||||
"-video_size", f"{screen['width']}x{screen['height']}",
|
||||
])
|
||||
|
||||
cmd.extend(["-i", "desktop"])
|
||||
|
||||
# Audio: dshow - try to find a working device
|
||||
audio_device = config.audio_device
|
||||
if not audio_device:
|
||||
@@ -337,6 +352,48 @@ class ScreenRecorder:
|
||||
return 0.0
|
||||
return time.time() - self.start_time
|
||||
|
||||
@staticmethod
|
||||
def _get_screen_geometries() -> list[dict]:
|
||||
"""Get geometry of all screens (Windows)."""
|
||||
screens = []
|
||||
|
||||
if sys.platform != "win32":
|
||||
return [{"left": 0, "top": 0, "width": 1920, "height": 1080}]
|
||||
|
||||
try:
|
||||
import ctypes
|
||||
|
||||
class RECT(ctypes.Structure):
|
||||
_fields_ = [
|
||||
("left", ctypes.c_long),
|
||||
("top", ctypes.c_long),
|
||||
("right", ctypes.c_long),
|
||||
("bottom", ctypes.c_long),
|
||||
]
|
||||
|
||||
def callback(hMonitor, hdcMonitor, lprcMonitor, dwData):
|
||||
rect = lprcMonitor.contents
|
||||
screens.append({
|
||||
"left": rect.left,
|
||||
"top": rect.top,
|
||||
"width": rect.right - rect.left,
|
||||
"height": rect.bottom - rect.top,
|
||||
})
|
||||
return True
|
||||
|
||||
MONITORENUMPROC = ctypes.WINFUNCTYPE(
|
||||
ctypes.c_bool, ctypes.c_ulong, ctypes.c_ulong,
|
||||
ctypes.POINTER(RECT), ctypes.c_double
|
||||
)
|
||||
|
||||
user32 = ctypes.windll.user32
|
||||
user32.EnumDisplayMonitors(None, None, MONITORENUMPROC(callback), 0)
|
||||
|
||||
except Exception as e:
|
||||
print(f"Error getting screen geometries: {e}")
|
||||
|
||||
return screens if screens else [{"left": 0, "top": 0, "width": 1920, "height": 1080}]
|
||||
|
||||
@staticmethod
|
||||
def list_audio_devices() -> list[str]:
|
||||
"""List available audio input devices (Windows)."""
|
||||
|
||||
Reference in New Issue
Block a user