diff --git a/.clang-format b/.clang-format new file mode 100644 index 0000000..f451e9b --- /dev/null +++ b/.clang-format @@ -0,0 +1,10 @@ +BasedOnStyle: LLVM +IndentWidth: 4 +TabWidth: 4 +UseTab: Never +ColumnLimit: 120 +BreakBeforeBraces: Attach +AllowShortFunctionsOnASingleLine: InlineOnly +AllowShortIfStatementsOnASingleLine: Never +AllowShortLoopsOnASingleLine: false +SortIncludes: false diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3ecbf9d..61828aa 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -6,22 +6,111 @@ on: pull_request: branches: [ master, main ] workflow_dispatch: + inputs: + enable_cross_build: + description: "Enable RK3588 cross build job" + required: false + default: "false" jobs: host-build: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - name: Configure & build (host) + with: + fetch-depth: 0 + - name: Install dependencies run: | sudo apt-get update - sudo apt-get install -y ninja-build + sudo apt-get install -y ninja-build clang-format cppcheck + + - name: Quality gates (clang-format + cppcheck on changed files) + shell: bash + run: | + set -euo pipefail + python3 - <<'PY' + import json, os, subprocess, sys + + def sh(*args): + return subprocess.check_output(list(args), text=True).strip() + + event = os.environ.get('GITHUB_EVENT_NAME','') + event_path = os.environ.get('GITHUB_EVENT_PATH','') + payload = {} + if event_path: + with open(event_path, 'r', encoding='utf-8') as f: + payload = json.load(f) + base = None + head = os.environ.get('GITHUB_SHA','HEAD') + + if event == 'pull_request': + base = (((payload.get('pull_request') or {}).get('base') or {}).get('sha')) + else: + base = payload.get('before') + if base and set(base) == {'0'}: + base = None + + if not base: + # Fallback to previous commit if possible. + try: + base = sh('git','rev-parse','HEAD~1') + except Exception: + base = None + + if base: + diff = subprocess.check_output(['git','diff','--name-only',f'{base}...{head}'], text=True) + paths = [p.strip() for p in diff.splitlines() if p.strip()] + else: + paths = [p.strip() for p in sh('git','ls-files').splitlines() if p.strip()] + + exts = ('.c','.cc','.cpp','.cxx','.h','.hh','.hpp','.hxx') + files = [] + for p in paths: + if not p.endswith(exts): + continue + if p.startswith(('third_party/','build/','.git/')): + continue + files.append(p) + if not files: + print('No C/C++ files changed; skipping format/cppcheck.') + sys.exit(0) + + # clang-format + subprocess.check_call(['clang-format','--version']) + subprocess.check_call(['clang-format','--dry-run','--Werror',*files]) + + # cppcheck + subprocess.check_call(['cppcheck','--version']) + subprocess.check_call([ + 'cppcheck', + '--error-exitcode=1', + '--enable=warning,style,performance,portability', + '--inline-suppr', + '--suppress=missingIncludeSystem', + '--std=c++17', + '-I','include', + '-I','third_party', + *files, + ]) + PY + + - name: Configure & build (host) + env: + CMAKE_ARGS: -DBUILD_TESTS=ON + run: | ./scripts/build_host.sh + - name: Run unit tests + run: | + ctest --test-dir build/host --output-on-failure + rk3588-cross-build: + if: github.event_name == 'workflow_dispatch' && github.event.inputs.enable_cross_build == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Install toolchain run: | sudo apt-get update diff --git a/CMakeLists.txt b/CMakeLists.txt index 9424c30..7d1c639 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,6 +9,7 @@ project(rk3588_media_server set(CMAKE_EXPORT_COMPILE_COMMANDS ON) option(BUILD_SAMPLES "Build demo binaries" ON) +option(BUILD_TESTS "Build unit tests" ON) option(ENABLE_RK_DEMOS "Use official Rockchip demos that link against vendor SDKs" OFF) set(RK_MPP_ROOT "${CMAKE_SOURCE_DIR}/third_party/mpp" CACHE PATH "Path to Rockchip MPP SDK root") @@ -82,6 +83,11 @@ if(BUILD_SAMPLES) add_subdirectory(samples) endif() +if(BUILD_TESTS) + enable_testing() + add_subdirectory(tests) +endif() + add_subdirectory(plugins) include(GNUInstallDirs) diff --git a/include/graph_manager.h b/include/graph_manager.h index 6b92709..ccaf864 100644 --- a/include/graph_manager.h +++ b/include/graph_manager.h @@ -136,7 +136,7 @@ private: bool enabled = true; SimpleJson config; NodeContext context; - std::unique_ptr node; + NodePtr node; std::vector cpu_affinity; // Scheduling state for Executor. diff --git a/include/media_server_app.h b/include/media_server_app.h index 09be382..7aeef52 100644 --- a/include/media_server_app.h +++ b/include/media_server_app.h @@ -1,5 +1,7 @@ #pragma once +#include +#include #include #include "graph_manager.h" @@ -14,6 +16,9 @@ public: private: std::string config_path_; GraphManager graph_manager_{}; + + std::atomic stop_signal_thread_{false}; + std::thread signal_thread_; }; } // namespace rk3588 diff --git a/include/node.h b/include/node.h index c03f613..b4ec589 100644 --- a/include/node.h +++ b/include/node.h @@ -52,17 +52,36 @@ public: virtual void Drain() {} }; -constexpr int kNodeAbiVersion = 2; +// ABI version for dynamic node plugins. +// Bump when plugin-required symbols or contracts change. +constexpr int kNodeAbiVersion = 3; using CreateNodeFn = INode* (*)(); using DestroyNodeFn = void (*)(INode*); using GetTypeFn = const char* (*)(); using GetAbiFn = int (*)(); +struct NodeDeleter { + DestroyNodeFn destroy = nullptr; + void operator()(INode* p) const noexcept { + if (!p) return; + if (destroy) { + destroy(p); + } else { + delete p; + } + } +}; + +using NodePtr = std::unique_ptr; + #define REGISTER_NODE(NodeClass, NodeTypeStr) \ extern "C" rk3588::INode* CreateNode() { \ return new NodeClass(); \ } \ + extern "C" void DestroyNode(rk3588::INode* p) { \ + delete p; \ + } \ extern "C" const char* GetNodeType() { \ return NodeTypeStr; \ } \ diff --git a/include/plugin_loader.h b/include/plugin_loader.h index 7973258..cd59da3 100644 --- a/include/plugin_loader.h +++ b/include/plugin_loader.h @@ -19,7 +19,7 @@ public: PluginLoader(PluginLoader&& other) noexcept; PluginLoader& operator=(PluginLoader&& other) noexcept; - std::unique_ptr Create(const std::string& type, std::string& err); + NodePtr Create(const std::string& type, std::string& err); // Switch plugin directory. This will unload any cached plugins. void SetPluginDir(std::string plugin_dir); @@ -29,6 +29,7 @@ private: struct PluginHandle { void* handle = nullptr; CreateNodeFn create = nullptr; + DestroyNodeFn destroy = nullptr; GetTypeFn get_type = nullptr; GetAbiFn get_abi = nullptr; }; diff --git a/scripts/build_board.sh b/scripts/build_board.sh index 2421523..56c94b3 100644 --- a/scripts/build_board.sh +++ b/scripts/build_board.sh @@ -7,6 +7,9 @@ INSTALL_PREFIX=${INSTALL_PREFIX:-${BUILD_DIR}/install} BUILD_TYPE=${BUILD_TYPE:-Release} TOOLCHAIN_FILE=${TOOLCHAIN_FILE:-"$ROOT_DIR/cmake/toolchain/aarch64-rk3588.cmake"} +# Extra args for customization. +CMAKE_ARGS=${CMAKE_ARGS:-} + if [[ -z "${RK3588_SYSROOT:-}" ]]; then echo "RK3588_SYSROOT environment variable must be set" >&2 exit 1 @@ -15,7 +18,9 @@ fi cmake -S "$ROOT_DIR" -B "$BUILD_DIR" \ -G Ninja \ -DCMAKE_BUILD_TYPE="$BUILD_TYPE" \ - -DCMAKE_TOOLCHAIN_FILE="$TOOLCHAIN_FILE" + -DCMAKE_TOOLCHAIN_FILE="$TOOLCHAIN_FILE" \ + -DBUILD_TESTS=OFF \ + $CMAKE_ARGS cmake --build "$BUILD_DIR" cmake --install "$BUILD_DIR" --prefix "$INSTALL_PREFIX" diff --git a/scripts/build_host.sh b/scripts/build_host.sh index dc430c3..37a673b 100644 --- a/scripts/build_host.sh +++ b/scripts/build_host.sh @@ -4,8 +4,12 @@ set -euo pipefail BUILD_DIR=${BUILD_DIR:-build/host} BUILD_TYPE=${BUILD_TYPE:-RelWithDebInfo} +# Extra args for CI/local customization, e.g. -DBUILD_TESTS=ON +CMAKE_ARGS=${CMAKE_ARGS:-} + cmake -S "$(dirname "$0")/.." -B "$BUILD_DIR" \ -G Ninja \ - -DCMAKE_BUILD_TYPE="$BUILD_TYPE" + -DCMAKE_BUILD_TYPE="$BUILD_TYPE" \ + $CMAKE_ARGS cmake --build "$BUILD_DIR" diff --git a/src/graph_manager.cpp b/src/graph_manager.cpp index 0c86a67..a06b649 100644 --- a/src/graph_manager.cpp +++ b/src/graph_manager.cpp @@ -815,7 +815,7 @@ bool Graph::Start() { if (!entry.enabled || !entry.node) continue; if (is_source_like(entry)) continue; if (!entry.node->Start()) { - std::cerr << "[Graph] failed to start node: " << entry.id << "\n"; + LogError("[Graph] failed to start node: " + entry.id); Stop(); return false; } @@ -824,7 +824,7 @@ bool Graph::Start() { // 2) Start executor + attach callbacks before any source can push frames. executor_ = std::make_unique(*this); if (!executor_->Start()) { - std::cerr << "[Graph] failed to start executor\n"; + LogError("[Graph] failed to start executor"); Stop(); return false; } @@ -835,13 +835,13 @@ bool Graph::Start() { if (!entry.enabled || !entry.node) continue; if (!is_source_like(entry)) continue; if (!entry.node->Start()) { - std::cerr << "[Graph] failed to start node: " << entry.id << "\n"; + LogError("[Graph] failed to start node: " + entry.id); Stop(); return false; } } - std::cout << "[Graph] started graph " << name_ << " with " << nodes_.size() << " nodes\n"; + LogInfo("[Graph] started graph " + name_ + " with " + std::to_string(nodes_.size()) + " nodes"); return true; } diff --git a/src/media_server_app.cpp b/src/media_server_app.cpp index e137b4a..b93b69e 100644 --- a/src/media_server_app.cpp +++ b/src/media_server_app.cpp @@ -1,12 +1,11 @@ #include "media_server_app.h" -#include -#include -#include -#include #include -#include #include +#include +#include +#include +#include #include #if __has_include() @@ -15,7 +14,9 @@ namespace fs = std::filesystem; #endif #if defined(__linux__) +#include #include +#include #include #include #include @@ -30,7 +31,13 @@ namespace rk3588 { namespace { -GraphManager* g_manager = nullptr; +#if !defined(__linux__) +volatile std::sig_atomic_t g_stop_signal = 0; + +void HandleSignal(int) { + g_stop_signal = 1; +} +#endif std::string ResolvePluginDir() { if (const char* env = std::getenv("RK_PLUGIN_DIR")) { @@ -52,13 +59,6 @@ std::string ResolvePluginDir() { return std::string("plugins"); } -void HandleSignal(int) { - if (g_manager) { - LogWarn("[MediaServerApp] Caught signal, stopping..."); - g_manager->RequestStop(); - } -} - bool HotReloadEnabled(const SimpleJson& root_cfg) { if (const SimpleJson* g = root_cfg.Find("global")) { if (const SimpleJson* hr = g->Find("hot_reload")) { @@ -195,12 +195,57 @@ int MediaServerApp::Start() { return 1; } - g_manager = &graph_manager_; - signal(SIGINT, HandleSignal); - signal(SIGTERM, HandleSignal); + // Signal handling: + // - Linux: block SIGINT/SIGTERM and handle via a dedicated thread (async-signal-safe). + // - Other: minimal signal handler sets a flag; a thread observes the flag and stops graphs. +#if defined(__linux__) + sigset_t set; + sigemptyset(&set); + sigaddset(&set, SIGINT); + sigaddset(&set, SIGTERM); + pthread_sigmask(SIG_BLOCK, &set, nullptr); + + stop_signal_thread_.store(false); + signal_thread_ = std::thread([this, set]() { + while (!stop_signal_thread_.load(std::memory_order_acquire)) { + timespec ts{}; + ts.tv_sec = 0; + ts.tv_nsec = 200 * 1000 * 1000; // 200ms + + const int sig = sigtimedwait(&set, nullptr, &ts); + if (sig == SIGINT || sig == SIGTERM) { + LogWarn("[MediaServerApp] Caught signal, stopping..."); + graph_manager_.RequestStop(); + return; + } + } + }); +#else + g_stop_signal = 0; + std::signal(SIGINT, HandleSignal); + std::signal(SIGTERM, HandleSignal); + + stop_signal_thread_.store(false); + signal_thread_ = std::thread([this]() { + while (!stop_signal_thread_.load(std::memory_order_acquire)) { + if (g_stop_signal) { + LogWarn("[MediaServerApp] Caught signal, stopping..."); + graph_manager_.RequestStop(); + return; + } + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + } + }); +#endif + + auto stop_signal = [&]() { + stop_signal_thread_.store(true, std::memory_order_release); + if (signal_thread_.joinable()) signal_thread_.join(); + }; if (!graph_manager_.StartAll()) { LogError("[MediaServerApp] Failed to start graphs"); + stop_signal(); return 1; } @@ -228,6 +273,8 @@ int MediaServerApp::Start() { http.Stop(); + stop_signal(); + stop_watch.store(true); if (watcher.joinable()) watcher.join(); LogInfo("[MediaServerApp] Shutdown complete."); diff --git a/src/plugin_loader.cpp b/src/plugin_loader.cpp index 650fbb9..6b132aa 100644 --- a/src/plugin_loader.cpp +++ b/src/plugin_loader.cpp @@ -1,8 +1,9 @@ #include "plugin_loader.h" -#include #include +#include "utils/logger.h" + #ifdef _WIN32 #include #else @@ -110,9 +111,10 @@ bool PluginLoader::LoadPlugin(const std::string& type, PluginHandle& out, std::s if (!handle) return false; auto create = reinterpret_cast(LoadSymbol(handle, "CreateNode")); + auto destroy = reinterpret_cast(LoadSymbol(handle, "DestroyNode")); auto get_type = reinterpret_cast(LoadSymbol(handle, "GetNodeType")); auto get_abi = reinterpret_cast(LoadSymbol(handle, "GetAbiVersion")); - if (!create || !get_type || !get_abi) { + if (!create || !destroy || !get_type || !get_abi) { err = "Missing required symbols in plugin: " + path; CloseLibraryHandle(handle); return false; @@ -125,30 +127,40 @@ bool PluginLoader::LoadPlugin(const std::string& type, PluginHandle& out, std::s CloseLibraryHandle(handle); return false; } + + if (const char* declared = get_type()) { + if (type != declared) { + err = "Plugin type mismatch: requested '" + type + "' but plugin reports '" + + std::string(declared) + "'"; + CloseLibraryHandle(handle); + return false; + } + } + out.handle = handle; out.create = create; + out.destroy = destroy; out.get_type = get_type; out.get_abi = get_abi; return true; } -std::unique_ptr PluginLoader::Create(const std::string& type, std::string& err) { +NodePtr PluginLoader::Create(const std::string& type, std::string& err) { auto it = cache_.find(type); if (it == cache_.end()) { PluginHandle handle; if (!LoadPlugin(type, handle, err)) { - std::cerr << "[PluginLoader] Failed to load plugin '" << type << "': " << err - << "\n"; - return nullptr; + LogError("[PluginLoader] Failed to load plugin '" + type + "': " + err); + return NodePtr(nullptr, NodeDeleter{}); } it = cache_.emplace(type, std::move(handle)).first; } PluginHandle& handle = it->second; - std::unique_ptr node(handle.create()); + NodePtr node(handle.create(), NodeDeleter{handle.destroy}); if (!node) { err = "CreateNode returned null for type " + type; - return nullptr; + return NodePtr(nullptr, NodeDeleter{handle.destroy}); } return node; } diff --git a/src/utils/dma_alloc.cpp b/src/utils/dma_alloc.cpp index 9aeeab2..d2185b1 100644 --- a/src/utils/dma_alloc.cpp +++ b/src/utils/dma_alloc.cpp @@ -3,11 +3,12 @@ #include #include #include -#include #include #include #include +#include "utils/logger.h" + #if defined(__linux__) #include #include @@ -218,23 +219,24 @@ DmaBufferPtr DmaAlloc(size_t alloc_size) { } if (dma_fd < 0) { - std::cerr << "[DmaAlloc] DMA_HEAP_IOCTL_ALLOC failed on all heaps: " << strerror(last_errno) << "\n"; - std::cerr << "[DmaAlloc] tried: "; + LogError(std::string("[DmaAlloc] DMA_HEAP_IOCTL_ALLOC failed on all heaps: ") + strerror(last_errno)); + std::string tried = "[DmaAlloc] tried:"; for (int i = 0; heap_paths[i] != nullptr; ++i) { - std::cerr << heap_paths[i] << " "; + tried += " "; + tried += heap_paths[i]; } - std::cerr << "\n"; + LogError(tried); return nullptr; } if (first_success) { - std::cout << "[DmaAlloc] using heap: " << used_heap << "\n"; + LogInfo(std::string("[DmaAlloc] using heap: ") + (used_heap ? used_heap : "(unknown)")); first_success = false; } void* ptr = mmap(nullptr, alloc_size, PROT_READ | PROT_WRITE, MAP_SHARED, dma_fd, 0); if (ptr == MAP_FAILED) { - std::cerr << "[DmaAlloc] mmap failed: " << strerror(errno) << "\n"; + LogError(std::string("[DmaAlloc] mmap failed: ") + strerror(errno)); close(dma_fd); return nullptr; } @@ -246,7 +248,7 @@ DmaBufferPtr DmaAlloc(size_t alloc_size) { return DmaBufferPtr(buf, [](DmaBuffer* b) { Pool().Put(b); }); #else (void)alloc_size; - std::cerr << "[DmaAlloc] not supported on this platform\n"; + LogWarn("[DmaAlloc] not supported on this platform"); return nullptr; #endif } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt new file mode 100644 index 0000000..0a8a14b --- /dev/null +++ b/tests/CMakeLists.txt @@ -0,0 +1,13 @@ +add_executable(rk3588_tests + test_main.cpp + ${CMAKE_SOURCE_DIR}/src/utils/config_expand.cpp +) + +target_include_directories(rk3588_tests PRIVATE + ${CMAKE_SOURCE_DIR}/include + ${CMAKE_SOURCE_DIR}/third_party +) + +target_link_libraries(rk3588_tests PRIVATE project_options Threads::Threads) + +add_test(NAME rk3588_tests COMMAND rk3588_tests) diff --git a/tests/test_main.cpp b/tests/test_main.cpp new file mode 100644 index 0000000..811a84b --- /dev/null +++ b/tests/test_main.cpp @@ -0,0 +1,121 @@ +#include +#include +#include +#include + +#include "utils/config_expand.h" +#include "utils/config_schema.h" +#include "utils/simple_json.h" +#include "utils/spsc_queue.h" + +namespace { + +int Fail(const char* expr, const char* file, int line) { + std::cerr << "TEST FAIL: " << expr << " at " << file << ":" << line << "\n"; + return 1; +} + +#define CHECK(x) do { if (!(x)) return Fail(#x, __FILE__, __LINE__); } while (0) + +using rk3588::ParseSimpleJson; +using rk3588::SimpleJson; + +int TestSimpleJsonBasic() { + SimpleJson v; + std::string err; + CHECK(ParseSimpleJson("{\"a\":1,\"b\":true,\"c\":[\"x\",null]}", v, err)); + CHECK(v.IsObject()); + CHECK(v.ValueOr("a", 0) == 1); + CHECK(v.ValueOr("b", false) == true); + const SimpleJson* c = v.Find("c"); + CHECK(c && c->IsArray()); + CHECK(c->AsArray().size() == 2); + CHECK(c->AsArray()[0].AsString("") == "x"); + CHECK(c->AsArray()[1].IsNull()); + + SimpleJson bad; + std::string err2; + CHECK(!ParseSimpleJson("{\"a\":1} trailing", bad, err2)); + CHECK(!err2.empty()); + return 0; +} + +int TestConfigExpandAndValidate() { + // Root with templates/instances -> graphs + const std::string src = R"JSON( +{ + "queue": {"size": 8, "strategy": "drop_oldest"}, + "templates": { + "t": { + "nodes": [ + {"id": "n1", "type": "input_file", "role": "source", "path": "${path}"}, + {"id": "n2", "type": "det_post", "role": "filter"} + ], + "edges": [ + ["n1", "n2", {"queue": {"size": 4}}] + ] + } + }, + "instances": [ + { + "name": "cam1", + "template": "t", + "params": {"path": "/tmp/a.mp4"}, + "override": {"nodes": {"n2": {"enable": true}}} + } + ] +} +)JSON"; + + SimpleJson root; + std::string perr; + CHECK(ParseSimpleJson(src, root, perr)); + + SimpleJson expanded; + std::string eerr; + CHECK(rk3588::ExpandRootConfig(root, expanded, eerr)); + + std::string verr; + CHECK(rk3588::ValidateExpandedRootConfig(expanded, verr)); + + const SimpleJson* graphs = expanded.Find("graphs"); + CHECK(graphs && graphs->IsArray()); + CHECK(graphs->AsArray().size() == 1); + const SimpleJson& g0 = graphs->AsArray()[0]; + CHECK(g0.ValueOr("name", "") == "cam1"); + const SimpleJson* nodes = g0.Find("nodes"); + CHECK(nodes && nodes->IsArray()); + CHECK(nodes->AsArray().size() == 2); + // ids should be prefixed. + CHECK(nodes->AsArray()[0].ValueOr("id", "") == "cam1_n1"); + CHECK(nodes->AsArray()[1].ValueOr("id", "") == "cam1_n2"); + // placeholder replaced. + CHECK(nodes->AsArray()[0].ValueOr("path", "") == "/tmp/a.mp4"); + return 0; +} + +int TestSpscQueueBasic() { + using rk3588::QueueDropStrategy; + rk3588::SpscQueue q(2, QueueDropStrategy::DropOldest); + CHECK(q.Push(1)); + CHECK(q.Push(2)); + // Full: DropOldest should drop 1 and accept 3. + CHECK(q.Push(3)); + + int v = 0; + CHECK(q.TryPop(v)); + CHECK(v == 2); + CHECK(q.TryPop(v)); + CHECK(v == 3); + return 0; +} + +} // namespace + +int main() { + if (int rc = TestSimpleJsonBasic(); rc != 0) return rc; + if (int rc = TestConfigExpandAndValidate(); rc != 0) return rc; + if (int rc = TestSpscQueueBasic(); rc != 0) return rc; + std::cout << "All tests passed\n"; + return 0; +}