Skip to content

Conversation

@ktaube26
Copy link
Contributor

@ktaube26 ktaube26 commented Jan 9, 2026

Description

This PR simplifies Vippet’s encoded video output configuration by removing explicit encoder device selection from the API and instead choosing an appropriate encoder device automatically based on the pipeline’s caps (notably VAAPI memory caps).

What Changed

  • OpenAPI / docs updates

    • Removed video_output.encoder_device from request examples and descriptions.
    • Dropped the EncoderDeviceConfig schema from the API.
  • Automatic encoder device selection

    • Added logic to recommend encoder device by inspecting caps nodes:
      • If the pipeline includes video/x-raw(memory:VAMemory)GPU encoder
      • Otherwise (or if no video/x-raw caps node exists) → CPU encoder
    • Implemented via Graph.get_recommended_encoder_device() and used when preparing pipelines with video output enabled.
  • VideoEncoder interface simplification

    • Encoder selection now uses simple device constants (CPU / GPU) rather than a structured device config (no GPU id parsing).
  • Tests updated

    • Updated unit tests to match the simplified API and revised encoder selection behavior.
    • Added/updated coverage for encoder-device recommendation based on caps.

User-Facing Impact

  • Requests that previously provided an explicit encoder device are no longer supported.
  • Enabling encoded output now requires only:
    { "video_output": { "enabled": true } }

Breaking Change

  • API breaking change: video_output.encoder_device is removed.
    • Clients sending encoder_device must be updated to omit it.

Checklist:

  • I agree to use the APACHE-2.0 license for my code changes.
  • I have not introduced any 3rd party components incompatible with APACHE-2.0.
  • I have not included any company confidential information, trade secret, password or security token.
  • I have performed a self-review of my code.

@ktaube26 ktaube26 added the vippet label Jan 9, 2026
@ktaube26 ktaube26 requested a review from Copilot January 9, 2026 17:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies ViPPET's encoded video output configuration by removing explicit encoder device selection from the API. The system now automatically selects an appropriate encoder (GPU or CPU) based on the pipeline's capabilities, specifically by detecting VAAPI memory caps in the pipeline graph.

Changes:

  • Removed encoder_device configuration from the API and replaced it with automatic encoder device selection
  • Simplified encoder selection logic to use device type constants (CPU/GPU) instead of complex device configurations with GPU ID parsing
  • Updated tests to reflect the simplified API and validate the new automatic encoder recommendation logic

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
video_encoder.py Removed GPU ID parsing logic and multi-GPU encoder configurations; simplified to CPU/GPU constants
video_encoder_test.py Updated tests to remove GPU selection tests and use simplified encoder device constants
pipeline_manager_test.py Removed encoder_device specifications from test configurations
graph_test.py Added comprehensive tests for the new automatic encoder device recommendation logic
tests_test.py Removed assertions checking encoder device configuration fields
pipeline_manager.py Added call to get_recommended_encoder_device() when video output is enabled
graph.py Implemented get_recommended_encoder_device() to automatically select encoder based on VAMemory caps
tests.py Updated API documentation to remove encoder_device parameter
api_schemas.py Removed EncoderDeviceConfig schema and encoder_device field from VideoOutputConfig
vippet.json Updated OpenAPI specification to remove encoder device configuration from all examples and schemas

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ktaube26 ktaube26 marked this pull request as ready for review January 12, 2026 09:17
p-zak
p-zak previously approved these changes Jan 12, 2026
p-zak
p-zak previously approved these changes Jan 12, 2026
@ktaube26 ktaube26 merged commit 6893c0d into main Jan 13, 2026
30 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants