mirror of
https://github.com/jamiepine/voicebox.git
synced 2026-10-03 09:05:17 -07:00
Fix review findings: toggle logic, preset saving, version lookup, async audio ops
- Fix inverted effects toggle in FloatingGenerateBox - Add Save button + API method for editing custom effect presets - Return early on effects save failure in ProfileForm - Fix no-op ternary in effectsStore - Handle duplicate preset names with proper 400 response - Use effects_chain is None instead of label for clean version lookup - Move blocking audio ops to asyncio.to_thread in async endpoints - Log warnings instead of silently swallowing parse errors
This commit is contained in:
@@ -142,6 +142,29 @@ export function EffectsDetail() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function handleSaveExisting() {
|
||||||
|
if (!selectedPresetId || !name.trim()) return;
|
||||||
|
setSaving(true);
|
||||||
|
try {
|
||||||
|
await apiClient.updateEffectPreset(selectedPresetId, {
|
||||||
|
name: name.trim(),
|
||||||
|
description: description.trim() || undefined,
|
||||||
|
effects_chain: workingChain,
|
||||||
|
});
|
||||||
|
queryClient.invalidateQueries({ queryKey: ['effect-presets'] });
|
||||||
|
queryClient.invalidateQueries({ queryKey: ['effect-preset', selectedPresetId] });
|
||||||
|
toast({ title: 'Preset updated' });
|
||||||
|
} catch (error) {
|
||||||
|
toast({
|
||||||
|
title: 'Failed to save',
|
||||||
|
description: error instanceof Error ? error.message : 'Unknown error',
|
||||||
|
variant: 'destructive',
|
||||||
|
});
|
||||||
|
} finally {
|
||||||
|
setSaving(false);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async function handleSaveAsNew() {
|
async function handleSaveAsNew() {
|
||||||
await handleSaveNew();
|
await handleSaveNew();
|
||||||
}
|
}
|
||||||
@@ -186,16 +209,27 @@ export function EffectsDetail() {
|
|||||||
</h2>
|
</h2>
|
||||||
<div className="flex items-center gap-2">
|
<div className="flex items-center gap-2">
|
||||||
{!isBuiltIn && !isCreatingNew && (
|
{!isBuiltIn && !isCreatingNew && (
|
||||||
<Button
|
<>
|
||||||
variant="ghost"
|
<Button
|
||||||
size="sm"
|
variant="ghost"
|
||||||
className="h-8 text-destructive hover:text-destructive gap-1.5"
|
size="sm"
|
||||||
onClick={handleDelete}
|
className="h-8 text-destructive hover:text-destructive gap-1.5"
|
||||||
disabled={deleting}
|
onClick={handleDelete}
|
||||||
>
|
disabled={deleting}
|
||||||
<Trash2 className="h-3.5 w-3.5" />
|
>
|
||||||
{deleting ? 'Deleting...' : 'Delete'}
|
<Trash2 className="h-3.5 w-3.5" />
|
||||||
</Button>
|
{deleting ? 'Deleting...' : 'Delete'}
|
||||||
|
</Button>
|
||||||
|
<Button
|
||||||
|
size="sm"
|
||||||
|
className="h-8 gap-1.5"
|
||||||
|
onClick={handleSaveExisting}
|
||||||
|
disabled={saving || workingChain.length === 0}
|
||||||
|
>
|
||||||
|
<Save className="h-3.5 w-3.5" />
|
||||||
|
{saving ? 'Saving...' : 'Save'}
|
||||||
|
</Button>
|
||||||
|
</>
|
||||||
)}
|
)}
|
||||||
{isCreatingNew && (
|
{isCreatingNew && (
|
||||||
<Button
|
<Button
|
||||||
|
|||||||
@@ -393,8 +393,9 @@ export function FloatingGenerateBox({
|
|||||||
variant="ghost"
|
variant="ghost"
|
||||||
size="icon"
|
size="icon"
|
||||||
onClick={() => {
|
onClick={() => {
|
||||||
setIsEffectsMode(!isEffectsMode);
|
const next = !isEffectsMode;
|
||||||
if (isEffectsMode) setIsInstructMode(false);
|
setIsEffectsMode(next);
|
||||||
|
if (next) setIsInstructMode(false);
|
||||||
}}
|
}}
|
||||||
className={cn(
|
className={cn(
|
||||||
'h-10 w-10 rounded-full transition-all duration-200',
|
'h-10 w-10 rounded-full transition-all duration-200',
|
||||||
|
|||||||
@@ -456,6 +456,7 @@ export function ProfileForm() {
|
|||||||
fxError instanceof Error ? fxError.message : 'Failed to save effects chain',
|
fxError instanceof Error ? fxError.message : 'Failed to save effects chain',
|
||||||
variant: 'destructive',
|
variant: 'destructive',
|
||||||
});
|
});
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -606,6 +606,16 @@ class ApiClient {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async updateEffectPreset(
|
||||||
|
presetId: string,
|
||||||
|
data: { name?: string; description?: string; effects_chain?: EffectConfig[] },
|
||||||
|
): Promise<EffectPresetResponse> {
|
||||||
|
return this.request<EffectPresetResponse>(`/effects/presets/${presetId}`, {
|
||||||
|
method: 'PUT',
|
||||||
|
body: JSON.stringify(data),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
async deleteEffectPreset(presetId: string): Promise<void> {
|
async deleteEffectPreset(presetId: string): Promise<void> {
|
||||||
await this.request<void>(`/effects/presets/${presetId}`, {
|
await this.request<void>(`/effects/presets/${presetId}`, {
|
||||||
method: 'DELETE',
|
method: 'DELETE',
|
||||||
|
|||||||
@@ -296,6 +296,12 @@ export interface EffectPresetCreate {
|
|||||||
effects_chain: EffectConfig[];
|
effects_chain: EffectConfig[];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export interface EffectPresetUpdate {
|
||||||
|
name?: string;
|
||||||
|
description?: string;
|
||||||
|
effects_chain?: EffectConfig[];
|
||||||
|
}
|
||||||
|
|
||||||
export interface AvailableEffectParam {
|
export interface AvailableEffectParam {
|
||||||
default: number;
|
default: number;
|
||||||
min: number;
|
min: number;
|
||||||
|
|||||||
@@ -22,5 +22,5 @@ export const useEffectsStore = create<EffectsStore>((set) => ({
|
|||||||
setWorkingChain: (chain) => set({ workingChain: chain }),
|
setWorkingChain: (chain) => set({ workingChain: chain }),
|
||||||
|
|
||||||
isCreatingNew: false,
|
isCreatingNew: false,
|
||||||
setIsCreatingNew: (v) => set({ isCreatingNew: v, selectedPresetId: v ? null : null }),
|
setIsCreatingNew: (v) => set({ isCreatingNew: v, ...(v && { selectedPresetId: null }) }),
|
||||||
}));
|
}));
|
||||||
|
|||||||
+11
-1
@@ -9,6 +9,7 @@ import uuid
|
|||||||
from typing import List, Optional
|
from typing import List, Optional
|
||||||
|
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
from sqlalchemy.exc import IntegrityError
|
||||||
|
|
||||||
from .database import EffectPreset as DBEffectPreset
|
from .database import EffectPreset as DBEffectPreset
|
||||||
from .models import EffectPresetResponse, EffectPresetCreate, EffectPresetUpdate, EffectConfig
|
from .models import EffectPresetResponse, EffectPresetCreate, EffectPresetUpdate, EffectConfig
|
||||||
@@ -58,6 +59,11 @@ def create_preset(data: EffectPresetCreate, db: Session) -> EffectPresetResponse
|
|||||||
if error:
|
if error:
|
||||||
raise ValueError(error)
|
raise ValueError(error)
|
||||||
|
|
||||||
|
# Check for duplicate name before insert
|
||||||
|
existing = db.query(DBEffectPreset).filter_by(name=data.name).first()
|
||||||
|
if existing:
|
||||||
|
raise ValueError(f"A preset named '{data.name}' already exists")
|
||||||
|
|
||||||
preset = DBEffectPreset(
|
preset = DBEffectPreset(
|
||||||
id=str(uuid.uuid4()),
|
id=str(uuid.uuid4()),
|
||||||
name=data.name,
|
name=data.name,
|
||||||
@@ -66,7 +72,11 @@ def create_preset(data: EffectPresetCreate, db: Session) -> EffectPresetResponse
|
|||||||
is_builtin=False,
|
is_builtin=False,
|
||||||
)
|
)
|
||||||
db.add(preset)
|
db.add(preset)
|
||||||
db.commit()
|
try:
|
||||||
|
db.commit()
|
||||||
|
except IntegrityError:
|
||||||
|
db.rollback()
|
||||||
|
raise ValueError(f"A preset named '{data.name}' already exists")
|
||||||
db.refresh(preset)
|
db.refresh(preset)
|
||||||
return _preset_response(preset)
|
return _preset_response(preset)
|
||||||
|
|
||||||
|
|||||||
+16
-15
@@ -1297,7 +1297,7 @@ async def transcribe_audio(
|
|||||||
try:
|
try:
|
||||||
# Get audio duration
|
# Get audio duration
|
||||||
from .utils.audio import load_audio
|
from .utils.audio import load_audio
|
||||||
audio, sr = load_audio(tmp_path)
|
audio, sr = await asyncio.to_thread(load_audio, tmp_path)
|
||||||
duration = len(audio) / sr
|
duration = len(audio) / sr
|
||||||
|
|
||||||
# Transcribe
|
# Transcribe
|
||||||
@@ -1585,21 +1585,21 @@ async def preview_effects(
|
|||||||
if error:
|
if error:
|
||||||
raise HTTPException(status_code=400, detail=error)
|
raise HTTPException(status_code=400, detail=error)
|
||||||
|
|
||||||
# Find clean version
|
# Find the original unprocessed version (no effects applied)
|
||||||
all_versions = versions_mod.list_versions(generation_id, db)
|
all_versions = versions_mod.list_versions(generation_id, db)
|
||||||
clean_version = next((v for v in all_versions if v.label == "clean"), None)
|
clean_version = next((v for v in all_versions if v.effects_chain is None), None)
|
||||||
source_path = clean_version.audio_path if clean_version else gen.audio_path
|
source_path = clean_version.audio_path if clean_version else gen.audio_path
|
||||||
if not source_path or not Path(source_path).exists():
|
if not source_path or not Path(source_path).exists():
|
||||||
raise HTTPException(status_code=404, detail="Source audio file not found")
|
raise HTTPException(status_code=404, detail="Source audio file not found")
|
||||||
|
|
||||||
# Process in memory
|
# Process in memory (off the event loop)
|
||||||
audio, sample_rate = load_audio(source_path)
|
audio, sample_rate = await asyncio.to_thread(load_audio, source_path)
|
||||||
processed = apply_effects(audio, sample_rate, chain_dicts)
|
processed = await asyncio.to_thread(apply_effects, audio, sample_rate, chain_dicts)
|
||||||
|
|
||||||
# Write to in-memory buffer
|
# Write to in-memory buffer
|
||||||
import soundfile as sf
|
import soundfile as sf
|
||||||
buf = io.BytesIO()
|
buf = io.BytesIO()
|
||||||
sf.write(buf, processed, sample_rate, format="WAV")
|
await asyncio.to_thread(sf.write, buf, processed, sample_rate, "WAV")
|
||||||
buf.seek(0)
|
buf.seek(0)
|
||||||
|
|
||||||
return StreamingResponse(
|
return StreamingResponse(
|
||||||
@@ -1723,10 +1723,10 @@ async def apply_effects_to_generation(
|
|||||||
if error:
|
if error:
|
||||||
raise HTTPException(status_code=400, detail=error)
|
raise HTTPException(status_code=400, detail=error)
|
||||||
|
|
||||||
# Find the clean version to apply effects to
|
# Find the original unprocessed version (no effects applied)
|
||||||
all_versions = versions_mod.list_versions(generation_id, db)
|
all_versions = versions_mod.list_versions(generation_id, db)
|
||||||
clean_version = next(
|
clean_version = next(
|
||||||
(v for v in all_versions if v.label == "clean"), None
|
(v for v in all_versions if v.effects_chain is None), None
|
||||||
)
|
)
|
||||||
if not clean_version:
|
if not clean_version:
|
||||||
# Fallback: use the generation's audio_path directly
|
# Fallback: use the generation's audio_path directly
|
||||||
@@ -1737,14 +1737,14 @@ async def apply_effects_to_generation(
|
|||||||
if not source_path or not Path(source_path).exists():
|
if not source_path or not Path(source_path).exists():
|
||||||
raise HTTPException(status_code=404, detail="Source audio file not found")
|
raise HTTPException(status_code=404, detail="Source audio file not found")
|
||||||
|
|
||||||
# Load, process, save
|
# Load, process, save (off the event loop)
|
||||||
audio, sample_rate = load_audio(source_path)
|
audio, sample_rate = await asyncio.to_thread(load_audio, source_path)
|
||||||
processed_audio = apply_effects(audio, sample_rate, chain_dicts)
|
processed_audio = await asyncio.to_thread(apply_effects, audio, sample_rate, chain_dicts)
|
||||||
|
|
||||||
# Generate a unique filename
|
# Generate a unique filename
|
||||||
version_id = str(uuid.uuid4())
|
version_id = str(uuid.uuid4())
|
||||||
processed_path = config.get_generations_dir() / f"{generation_id}_{version_id[:8]}.wav"
|
processed_path = config.get_generations_dir() / f"{generation_id}_{version_id[:8]}.wav"
|
||||||
save_audio(processed_audio, str(processed_path), sample_rate)
|
await asyncio.to_thread(save_audio, processed_audio, str(processed_path), sample_rate)
|
||||||
|
|
||||||
# Auto-label
|
# Auto-label
|
||||||
label = data.label or f"version-{len(all_versions) + 1}"
|
label = data.label or f"version-{len(all_versions) + 1}"
|
||||||
@@ -1857,14 +1857,15 @@ async def update_profile_effects(
|
|||||||
def _profile_to_response(profile) -> models.VoiceProfileResponse:
|
def _profile_to_response(profile) -> models.VoiceProfileResponse:
|
||||||
"""Convert a DB profile to a VoiceProfileResponse with parsed effects_chain."""
|
"""Convert a DB profile to a VoiceProfileResponse with parsed effects_chain."""
|
||||||
import json as _json
|
import json as _json
|
||||||
|
import logging
|
||||||
|
|
||||||
effects_chain = None
|
effects_chain = None
|
||||||
if profile.effects_chain:
|
if profile.effects_chain:
|
||||||
try:
|
try:
|
||||||
raw = _json.loads(profile.effects_chain)
|
raw = _json.loads(profile.effects_chain)
|
||||||
effects_chain = [models.EffectConfig(**e) for e in raw]
|
effects_chain = [models.EffectConfig(**e) for e in raw]
|
||||||
except Exception:
|
except Exception as e:
|
||||||
pass
|
logging.warning(f"Failed to parse effects_chain for profile {profile.id}: {e}")
|
||||||
|
|
||||||
return models.VoiceProfileResponse(
|
return models.VoiceProfileResponse(
|
||||||
id=profile.id,
|
id=profile.id,
|
||||||
|
|||||||
+3
-2
@@ -36,8 +36,9 @@ def _profile_to_response(profile: DBVoiceProfile) -> VoiceProfileResponse:
|
|||||||
try:
|
try:
|
||||||
raw = _json.loads(profile.effects_chain)
|
raw = _json.loads(profile.effects_chain)
|
||||||
effects_chain = [EffectConfig(**e) for e in raw]
|
effects_chain = [EffectConfig(**e) for e in raw]
|
||||||
except Exception:
|
except Exception as e:
|
||||||
pass
|
import logging
|
||||||
|
logging.warning(f"Failed to parse effects_chain for profile {profile.id}: {e}")
|
||||||
return VoiceProfileResponse(
|
return VoiceProfileResponse(
|
||||||
id=profile.id,
|
id=profile.id,
|
||||||
name=profile.name,
|
name=profile.name,
|
||||||
|
|||||||
Reference in New Issue
Block a user