Expand D-Bus API: audio, subtitles, markers, effects, keyframes#1
Expand D-Bus API: audio, subtitles, markers, effects, keyframes#1D-Ogi wants to merge 3 commits into
Conversation
Major expansion of the D-Bus client with new methods: - Audio: volume, fade in/out, track mute - Subtitles: CRUD, export to .ass - Clip markers: add, get, delete (per bin clip) - Effects: add/remove with params, list clip effects, opacity - Keyframes: get/set on effect parameters - Compositions: list timeline compositions - Clip speed: set playback speed - Groups: group/ungroup clips - Render: frame preview, bin frame, contact sheet, crop QC - Proxy: toggle proxy mode - Zones: in/out points Also fixes: - Array serialization: remove quotes around dbus-send string arrays - Add int32 array support for D-Bus calls - Add _result_to_dict() helper for parsing D-Bus dict responses - Improve subprocess output parsing robustness
…oded paths - Remove docs/kdenlive-api.md and docs/mcp-kdenlive-moscow.md (Polish text, internal MV references, hardcoded paths) - Replace hardcoded C:\CraftRoot fallback in dbus_client.py with env var lookup (DBUS_TOOLS_DIR -> CRAFT_ROOT -> PATH) - Generalize build_timeline.py: remove hardcoded 38-scene count (auto-detect or --num-scenes flag), remove mv/ workspace path, require --video-dir - Fix replace_scene.py: replace "1-38" help text with generic "1-based" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR significantly expands the kdenlive_api D-Bus client with 30+ new methods, providing comprehensive control over audio, subtitles, markers, effects, keyframes, and more. The changes also include bug fixes for D-Bus array handling and generalization of the build_timeline.py script.
Changes:
- Added extensive new D-Bus API methods for audio control, subtitle management, clip markers, effects, keyframes, compositions, preview rendering, selections, zones, sequences, and proxy management
- Fixed D-Bus array string formatting by removing quotes (bug fix for silent lookup failures)
- Added int32 array support and encoding parameter to subprocess calls
- Migrated clip marker storage from in-memory to D-Bus backend
- Generalized build_timeline.py from fixed 38-scene to configurable scene count
- Removed two Polish documentation files (mcp-kdenlive-moscow.md and kdenlive-api.md)
- Updated comment style from Unicode box-drawing characters to ASCII double-dash for better portability
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/replace_scene.py | Updated example path in usage documentation from specific output directory to generic path |
| scripts/build_timeline.py | Generalized from hardcoded 38 scenes to configurable scene count with auto-detection, improved CLI arguments, updated comment style to ASCII |
| kdenlive_api/media_pool.py | Migrated clip marker implementation from in-memory storage to D-Bus backend calls, properly documented limitations for custom data fields |
| kdenlive_api/dbus_client.py | Major expansion: added _result_to_dict helper, new D-Bus tool discovery, 30+ new API methods (audio, subtitles, markers, effects, keyframes, compositions, zones, selections, proxies, sequences), fixed array formatting bug, added encoding parameters |
| docs/mcp-kdenlive-moscow.md | Deleted Polish MoSCoW requirements document |
| docs/kdenlive-api.md | Deleted Polish API documentation file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if isinstance(result, dict): | ||
| return result | ||
| if isinstance(result, list): | ||
| d = {} | ||
| for item in result: | ||
| if isinstance(item, tuple) and len(item) == 2: | ||
| d[item[0]] = item[1] | ||
| elif isinstance(item, dict): | ||
| d.update(item) | ||
| return d if d else {"id": bin_id} | ||
| return {"id": bin_id} |
There was a problem hiding this comment.
This method duplicates the logic from the _result_to_dict helper method. Consider refactoring to use _result_to_dict for consistency and maintainability. The current implementation has the same dict-to-dict and list-to-dict conversion logic that _result_to_dict already provides.
| if isinstance(result, dict): | |
| return result | |
| if isinstance(result, list): | |
| d = {} | |
| for item in result: | |
| if isinstance(item, tuple) and len(item) == 2: | |
| d[item[0]] = item[1] | |
| elif isinstance(item, dict): | |
| d.update(item) | |
| return d if d else {"id": bin_id} | |
| return {"id": bin_id} | |
| props = self._result_to_dict(result) | |
| return props if props else {"id": bin_id} |
| def get_active_sequence(self) -> dict: | ||
| """Get info about the currently active sequence. | ||
|
|
||
| Returns dict with keys: uuid, name, duration, tracks. | ||
| """ | ||
| result = self._call("scriptGetActiveSequence") | ||
| if isinstance(result, dict): | ||
| return result | ||
| if isinstance(result, list): | ||
| d = {} | ||
| for pair in result: | ||
| if isinstance(pair, tuple) and len(pair) == 2: | ||
| d[pair[0]] = pair[1] | ||
| return d | ||
| return {} |
There was a problem hiding this comment.
This method also duplicates logic from _result_to_dict. While it has slightly different handling for the list case (iterating directly over pairs rather than items in a list), the core conversion pattern is the same. Consider using _result_to_dict or refactoring to avoid duplication.
| num_scenes = max(s["number"] for s in scenes_meta) | ||
| else: | ||
| # Count scene files in directory | ||
| import glob as globmod |
There was a problem hiding this comment.
The glob module is imported inline within the function body. For consistency and clarity, consider moving this import to the top of the file with other imports (lines 12-31).
| valid = self._get_valid_track_ids() | ||
| if valid and track_id not in valid: | ||
| return [] # Non-existent track | ||
| raise ValueError(f"Invalid track_id {track_id}. Valid: {sorted(valid)}") |
There was a problem hiding this comment.
There's an inconsistency in error handling between insert_clip and insert_clips_sequentially. insert_clip (line 537) returns -1 for invalid track_id, while insert_clips_sequentially (line 545) raises ValueError. Consider standardizing the error handling approach - either both should raise exceptions or both should return error values.
| raise ValueError(f"Invalid track_id {track_id}. Valid: {sorted(valid)}") | |
| return [] # Non-existent track |
- Remove dead doc links in README (docs/ files were deleted) - Generalize parse_script_scenes: configurable heading_pattern and field_map params, English field names (framing/pose/mood/camera) - Make collect_scene_videos num_scenes required (no hardcoded default) - Rename SCENE_DURATION_FRAMES to DEFAULT_CLIP_DURATION_FRAMES - Change default resolution from 1536x864 to standard 1920x1080 - Remove project-specific .gitignore entries (img/scenes, ComfyUI, etc.) - Fix hardcoded "1-38" in preview.py help text - Update tests and mock to match new defaults
Summary
Major expansion of the D-Bus client with 30+ new methods covering audio control, subtitle management, clip markers, effects, keyframes, compositions, preview rendering, and more.
New API methods
SetClipVolume,GetClipVolume,SetAudioFade,SetTrackMute,GetTrackMute,GetAudioLevelsGetSubtitles,AddSubtitle,EditSubtitle,DeleteSubtitle,ExportSubtitlesAddClipMarker,GetClipMarkers,DeleteClipMarker,DeleteClipMarkersByColorAddEffect,RemoveEffect,GetClipEffects,SetClipOpacityGetKeyframes,SetKeyframesGetCompositionsRenderFrame,RenderBinFrame,RenderContactSheet,RenderCropSetClipSpeed,GroupClips,UngroupClips,SetZoneIn/Out,ToggleProxyBug fixes
_result_to_dict()for reliable dict response parsing