Update GStreamer version to 1.28.1 across specifications and scripts#750
Update GStreamer version to 1.28.1 across specifications and scripts#750
Conversation
…on function for import handling
There was a problem hiding this comment.
Pull request overview
Updates the repository’s GStreamer baseline to 1.28.1 across packaging specs, build scripts, Docker build args, and the CMake-based GStreamer dependency fetch, along with patchset adjustments to align with the new upstream version.
Changes:
- Bump GStreamer version references from 1.26.11 → 1.28.1 across specs, scripts, Dockerfiles, and
dependencies/gstreamer.cmake. - Adjust/refresh GStreamer VA-related patch metadata and remove several no-longer-needed “bk_*” patch files.
- Update the
draw_face_attributessample CMake to rely more on discovered include/library variables (and add GST allocator linkage).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| SPECS/versions.env | Updates central GSTREAMER_VERSION to 1.28.1. |
| SPECS/intel-dlstreamer/intel-dlstreamer.spec | Raises build/runtime dependency minimums for GStreamer to 1.28.1. |
| SPECS/gstreamer/gstreamer.spec | Bumps the packaged GStreamer version to 1.28.1. |
| scripts/setup_dls_env.ps1 | Updates Windows environment setup to use GStreamer 1.28.1. |
| scripts/build_dlstreamer_dlls.ps1 | Updates DLL build script to use GStreamer 1.28.1. |
| docker/windows/windows_builder.Dockerfile | Updates build ARG default for GStreamer to 1.28.1. |
| docker/ubuntu/ubuntu24.Dockerfile | Updates GST_VERSION ARG to 1.28.1. |
| docker/ubuntu/ubuntu22.Dockerfile | Updates GST_VERSION ARG to 1.28.1. |
| docker/fedora41/fedora41.Dockerfile | Updates GST_VERSION ARG to 1.28.1. |
| dependencies/gstreamer.cmake | Updates DESIRED_VERSION tag to 1.28.1 for ExternalProject fetch/build. |
| samples/gstreamer/cpp/draw_face_attributes/CMakeLists.txt | Removes hardcoded include/link paths; switches to discovered OpenCV/GStreamer variables and adds GSTALLOC usage. |
| dependencies/patches/0015-va-allocator-add-gst_va_buffer_prepare_for_import-he.patch | Updates patch metadata and exports gst_va_buffer_prepare_for_import in the header. |
| dependencies/patches/0012-va-Do-not-reuse-vadisplay-if-it-is-used-too-many-tim.patch | Updates patch metadata for new upstream baseline. |
| dependencies/patches/0001-va-encoder-disable-usage-hint-because-of-iHD-bug.patch | Updates patch metadata for new upstream baseline. |
| dependencies/patches/0003-bk_0003-vabasetransform-copy-buffer-s-metadata-at-copy-when-.patch | Removes obsolete patch file. |
| dependencies/patches/0002-bk_0002-vaav1enc-add-upscaledwidth-value-for-SCC-encoding.patch | Removes obsolete patch file. |
| dependencies/patches/0001-bk_0001-vaav1enc-Enable-intrablock-copy-and-palette-mode.patch | Removes obsolete patch file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| %define debug_package %{nil} | ||
| Name: gstreamer | ||
| Version: 1.26.11 | ||
| Version: 1.28.1 | ||
| Release: 1%{?dist} | ||
| Summary: Intel optimized GStreamer build with VAAPI support |
There was a problem hiding this comment.
Version was bumped to 1.28.1, but the %changelog entries still reference older 1.26.x updates. Please add a new changelog entry for the 1.28.1 update (or update the existing top entry) to keep the spec metadata consistent.
| target_include_directories(${TARGET_NAME} | ||
| PRIVATE | ||
| ${GSTALLOC_INCLUDE_DIRS} | ||
| ${GSTVIDEO_INCLUDE_DIRS} | ||
| ${GSTREAMER_INCLUDE_DIRS} |
There was a problem hiding this comment.
GSTALLOC_INCLUDE_DIRS/GSTALLOC_LIBRARIES are referenced here, but this CMakeLists doesn’t define GSTALLOC via pkg_check_modules(...) (unlike other project CMakeLists that use these variables). As-is, a standalone configure/build of this sample will fail; add pkg_check_modules(GSTALLOC gstreamer-allocators-1.0 ... REQUIRED) or avoid using these vars when building standalone.
| PRIVATE | ||
| ${DLSTREAMER_BASE_DIR}/include/dlstreamer/gst | ||
| ${DLSTREAMER_BASE_DIR}/include/ | ||
| ${GSTREAMER_INCLUDE_DIR} | ||
| ${OPENCV_INCLUDE_DIR} | ||
| ${OpenCV_INCLUDE_DIRS} | ||
| ) |
There was a problem hiding this comment.
OpenCV is found with OPTIONAL_COMPONENTS, but the target then unconditionally consumes ${OpenCV_INCLUDE_DIRS}/${OpenCV_LIBS}. If OpenCV isn’t installed, CMake may configure successfully but the build will fail; consider making OpenCV REQUIRED (or conditionally disabling this sample when OpenCV_FOUND is false).
| - | ||
| +GST_VA_API | ||
| +GstBuffer * gst_va_buffer_prepare_for_import (GstBuffer * buffer, | ||
| + GstVaDisplay * display); | ||
| + | ||
| GST_VA_API |
There was a problem hiding this comment.
This header change exports gst_va_buffer_prepare_for_import, but the implementation (in the same patch) returns the input buffer without taking a ref on the passthrough path. If callers treat this as a newly public API and unref the returned buffer, it can prematurely free the original; ensure the function returns a new ref in all cases or document/annotate it as transfer-none.
| - | ||
| +GST_VA_API | ||
| +GstBuffer * gst_va_buffer_prepare_for_import (GstBuffer * buffer, | ||
| + GstVaDisplay * display); |
There was a problem hiding this comment.
When exporting this new helper, consider fixing the wrapped-DMABUF creation logic in the same patch: it allocates using gst_buffer_get_size(buffer) while only wrapping memory 0, and it doesn’t copy core buffer timing/flags (PTS/DTS/duration) to the new buffer. Both can cause incorrect behavior with multi-memory buffers and break downstream timing.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
samples/gstreamer/cpp/draw_face_attributes/CMakeLists.txt:50
${GSTALLOC_INCLUDE_DIRS}/${GSTALLOC_LIBRARIES}are used butGSTALLOCis never defined in this CMakeLists (nopkg_check_modules(GSTALLOC gstreamer-allocators-1.0 ...)). This will result in empty include/lib lists when building the sample standalone via this file.
target_include_directories(${TARGET_NAME}
PRIVATE
${GSTALLOC_INCLUDE_DIRS}
${GSTVIDEO_INCLUDE_DIRS}
${GSTREAMER_INCLUDE_DIRS}
${GLIB2_INCLUDE_DIRS}
${DLSTREAMER_INCLUDE_DIRS}
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /*===================== GstVaAllocator / GstVaMemory =========================*/ | ||
|
|
||
| /** | ||
| #define GST_VA_ALLOCATOR(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), GST_TYPE_VA_ALLOCATOR, GstVaAllocator)) |
There was a problem hiding this comment.
This patch contains an unmarked context line #define GST_VA_ALLOCATOR(obj) ... immediately before starting the next diff --git section. Because it’s not prefixed with +/-, git apply will require this exact line to already exist at that location in gstvaallocator.c for the patch to apply. If the intent was to add or move this macro, it should be part of a proper hunk (with +), otherwise it should be removed from the patch context to avoid patch-application failures when bumping GStreamer versions.
| #define GST_VA_ALLOCATOR(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), GST_TYPE_VA_ALLOCATOR, GstVaAllocator)) |
…d metaaggregate tests
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
samples/gstreamer/cpp/draw_face_attributes/CMakeLists.txt:49
GSTALLOC_INCLUDE_DIRS/GSTALLOC_LIBRARIESare referenced, but this CMakeLists doesn’t callpkg_check_modules(GSTALLOC gstreamer-allocators-1.0 ...)(unlike other parts of the repo, e.g.src/gst/CMakeLists.txt). As a result, building this sample standalone via pkg-config will fail with undefined variables. Add the missingpkg_check_modules(GSTALLOC gstreamer-allocators-1.0>=... REQUIRED)(or rename toGSTALLOCATORSconsistently) and use the corresponding _INCLUDE_DIRS/_LIBRARIES variables.
target_include_directories(${TARGET_NAME}
PRIVATE
${GSTALLOC_INCLUDE_DIRS}
${GSTVIDEO_INCLUDE_DIRS}
${GSTREAMER_INCLUDE_DIRS}
${GLIB2_INCLUDE_DIRS}
${DLSTREAMER_INCLUDE_DIRS}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| target_link_libraries(${TARGET_NAME} | ||
| PUBLIC | ||
| dlstreamer_gst_meta | ||
| PRIVATE | ||
| ${OpenCV_LIBS} | ||
| ${GLIB2_LIBRARIES} | ||
| ${GSTREAMER_LIBRARIES} | ||
| ${GSTALLOC_LIBRARIES} | ||
| ${GSTVIDEO_LIBRARIES} |
There was a problem hiding this comment.
The link line uses ${GSTALLOC_LIBRARIES}, but this file never defines GSTALLOC via pkg_check_modules. This will break standalone builds of the sample. Ensure the allocators module is discovered (e.g., pkg_check_modules(GSTALLOC gstreamer-allocators-1.0 REQUIRED)) and then link against ${GSTALLOC_LIBRARIES}.
| %define debug_package %{nil} | ||
| Name: gstreamer | ||
| Version: 1.26.11 | ||
| Version: 1.28.1 | ||
| Release: 1%{?dist} | ||
| Summary: Intel optimized GStreamer build with VAAPI support |
There was a problem hiding this comment.
The spec bumps GStreamer to 1.28.1, but the %build meson options still include -Dvaapi=enabled. In this PR’s other build definitions (e.g. dependencies/gstreamer.cmake) VA support is enabled via -Dgst-plugins-bad:va=enabled, suggesting vaapi is no longer the correct option/module for this version. Please update the spec’s meson flags accordingly so the RPM build doesn’t fail on an unknown/removed option or build the wrong VA backend.
| D_VAAPI_PIPELINE_STR = f""" | ||
| appsrc name=mysrc | ||
| ! jpegparse ! vaapijpegdec ! video/x-raw(memory:VASurface) | ||
| ! gvadetect pre-process-backend=vaapi device=GPU model={D_MODEL_PATH} threshold=0.9 | ||
| ! jpegparse ! vajpegdec ! video/x-raw(memory:VAMemory) | ||
| ! gvadetect pre-process-backend=va device=GPU model={D_MODEL_PATH} threshold=0.9 | ||
| ! appsink name=mysink emit-signals=true sync=false | ||
| """ | ||
|
|
||
| D_VAAPI_SURFACE_SHARING_PIPELINE_STR = f""" | ||
| appsrc name=mysrc | ||
| ! jpegparse ! vaapijpegdec ! video/x-raw(memory:VASurface) | ||
| ! gvadetect pre-process-backend=vaapi-surface-sharing device=GPU model={D_MODEL_PATH} threshold=0.9 | ||
| ! jpegparse ! vajpegdec ! video/x-raw(memory:VAMemory) | ||
| ! gvadetect pre-process-backend=va-surface-sharing device=GPU model={D_MODEL_PATH} threshold=0.9 | ||
| ! appsink name=mysink emit-signals=true sync=false |
There was a problem hiding this comment.
These constants/test names still say “VAAPI” (e.g. D_VAAPI_PIPELINE_STR, test_custom_vaapi_*), but the updated pipelines use the new VA elements/backend (vajpegdec, pre-process-backend=va, memory:VAMemory). Renaming the constants/tests to reflect va (and va-surface-sharing) would prevent confusion when debugging failures and align terminology with the actual pipeline being executed.
Description
Please include a summary of the changes and the related issue. List any dependencies that are required for this change.
Fixes # (issue)
Any Newly Introduced Dependencies
Please describe any newly introduced 3rd party dependencies in this change. List their name, license information and how they are used in the project.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: