Skip to content

Conversation

p-zak
Copy link
Contributor

@p-zak p-zak commented Sep 18, 2025

Common Pipeline Element Selection

  • Unified the logic for selecting pipeline elements (encoder, decoder, postprocessing, compositor) based on the chosen device (CPU, GPU.0, GPU.N, OTHER) using the new PipelineElementsSelector and PipelineElementSelectionInstructions classes from _common/common.py.
  • Replaced hardcoded and duplicated element selection code in simplevs/pipeline.py and smartnvr/pipeline.py with a shared, maintainable approach.
  • The selection now supports dynamic mapping for VAAPI suffixes and fallback to default elements.

New Features

  • Compositor Device Selection:
    • Added support for selecting the compositor device in the UI (app.py), configuration (config.yaml), and pipeline logic.
    • The compositor_device parameter is now propagated and used for element selection.

Fixes and Improvements

  • Fixed incorrect or inconsistent element selection for various device types, especially for GPU.N cases.
  • Updated tests to reflect the new element selection logic and ensure correct pipeline construction for all device scenarios.
  • Improved error handling and logging for missing pipeline elements.
  • Minor fixes in parameter propagation and meta file writing.

Tests

  • Updated and extended unit tests for both pipelines to cover new selection logic and device scenarios.
  • Ensured tests check for correct element usage, parameter propagation, and output structure.

@p-zak p-zak marked this pull request as ready for review September 18, 2025 10:13
@p-zak p-zak requested review from lamalexck and Copilot September 18, 2025 10:13
Copy link
Contributor

@Copilot 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 implements a common pipeline element selection system to unify and improve how pipeline elements (encoder, decoder, postprocessing, compositor) are selected based on device types across the visual pipeline evaluation tool.

  • Introduces shared PipelineElementsSelector and PipelineElementSelectionInstructions classes for consistent element selection logic
  • Adds compositor device selection support throughout the UI, configuration, and pipeline implementation
  • Replaces hardcoded element selection with dynamic mapping that supports VAAPI suffixes and fallback mechanisms

Reviewed Changes

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

Show a summary per file
File Description
pipelines/_common/common.py New shared classes for pipeline element selection logic with device-specific mappings
pipelines/smartnvr/pipeline.py Refactored to use common element selector, added compositor device support, improved error handling
pipelines/simplevs/pipeline.py Refactored to use common element selector, streamlined decoder/postprocessing logic
tests/pipelines_tests/smartnvr_tests/pipeline_test.py Updated test expectations to match new element selection behavior
tests/pipelines_tests/simplevs_tests/pipeline_test.py Updated test expectations for new decoder elements and added GPU-specific tests
app.py Added compositor device dropdown to UI with proper rendering logic
pipelines/smartnvr/config.yaml Enabled compositor device parameter
pipelines/simplevs/config.yaml Disabled compositor device parameter (not applicable)
utils.py Added compositor_device parameter handling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@p-zak p-zak added the vippet label Sep 18, 2025
tmatenko
tmatenko previously approved these changes Sep 18, 2025
self._shmsink_branch = (
"tee name=livetee "
"livetee. ! queue2 ! {encoder} ! h264parse ! mp4mux ! filesink location={VIDEO_OUTPUT_PATH} async=false "
"livetee. ! queue2 ! videoconvert ! video/x-raw,format=BGR,width={output_width},height={output_height} ! {shmsink} "
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe videoconvert is CPU based.
On platform with GPU, we want to use vapostproc instead

decoder={
GPU_0: [
(
"vaapidecodebin",
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets not use vaapiXXXX elements. They have been deprecated.

("va", "decodebin3", "..."),
("va", "vah264enc", "..."),
("va", "vah264dec", "..."),
("va", "vaapidecodebin", "..."),
Copy link
Contributor

Choose a reason for hiding this comment

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

do not use vaapi elements - they are deprecated for >1 year and will be removed in upcoming GStreamer releaes

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.

4 participants