diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 591fea83a..6c5ffa289 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -9,9 +9,11 @@ They apply to all code suggestions, documentation, tests, diagrams, and refactor # ============================== ## 1. Code Formatting -- Indentation uses hard tabs (4-space width). +- Indentation uses hard tabs (tab width 4, indent width 4). - Maximum line length target is 80 characters. - Add spaces around operators and after commas. +- Use Allman brace style (opening brace on its own line) where applicable. +- Follow `clang-format` alignment where the repository already documents formatting. ## 2. Testing - All public functions require unit tests. @@ -101,8 +103,9 @@ Language-specific patterns live in `.github/instructions/`. This is a streaming video player. Low latency, correct buffering, and real-time behavior are critical. 3. **Modernization Goal** - New code should be modern C++ (RAII, smart pointers, interfaces). - Legacy code should be gently refactored toward modern patterns. + The existing AAMP codebase is predominantly C++11. + New code must target C++17, using modern C++ idioms (RAII, smart pointers, interfaces). + Legacy code should be gently refactored toward modern C++17 patterns. 4. **Testing First** Always reference `instructions/testing.instructions.md` before writing tests. @@ -146,7 +149,8 @@ Language-specific patterns live in `.github/instructions/`. # ============================== # DOCUMENTATION & DIAGRAMS # ============================== -- Use Doxygen-style comments for all APIs. +- Use C-style Doxygen comment blocks (`/** ... */`) for all APIs. +- Do not use `///` style for AAMP public API documentation. - Generate diagrams with PlantUML. - See `instructions/diagrams.instructions.md` for details. diff --git a/.github/instructions/aamp.instructions.md b/.github/instructions/aamp.instructions.md index fd3ba6d20..7387986a4 100644 --- a/.github/instructions/aamp.instructions.md +++ b/.github/instructions/aamp.instructions.md @@ -18,7 +18,7 @@ AAMP (Advanced Adaptive Media Player) is a high-performance, embedded systems-fo ## 2. Core Technologies -- **Language:** C++11 is the primary language; there is a small amount of C++17, but the majority is built as C++11. While the goal is modern C++, the codebase contains significant legacy C-style code (e.g., `memcpy`, raw pointers). Refactoring this to use modern C++ features (smart pointers, STL containers, RAII) is an ongoing task. +- **Language:** The existing AAMP codebase is predominantly C++11, but new code must target C++17. While the goal is modern C++, the codebase contains significant legacy C-style code (e.g., `memcpy`, raw pointers). Refactoring this to use modern C++ features (smart pointers, STL containers, RAII) is an ongoing task. - **Media Framework:** GStreamer is used for the underlying media pipeline, including demuxing, decoding, and rendering. AAMP interacts with GStreamer to manage the flow of media data. - **Build System:** CMake is the primary build system. All source files, dependencies, and build targets are defined in `CMakeLists.txt` files. - **Testing:** Google Test & Google Mock are the frameworks used for unit and functional testing. @@ -82,3 +82,40 @@ AAMP maintains a comprehensive testing suite located in the `test/` directory. A - **Mocks and Fakes**: The codebase makes extensive use of mocks and fakes to achieve component isolation. - `test/utests/mocks/`: Contains mock objects generated using Google Mock (`MOCK_METHOD`). These are used to verify interactions between the class under test and its dependencies. - `test/utests/fakes/`: Contains fake implementations of classes. These provide a more lightweight, functional stub than a full mock and are used when the dependency's behavior is simple to simulate or when a mock is overly complex. This is a very common and important pattern in the codebase. + +## 6. AAMP File Naming Conventions + +- Class files should use an `Aamp` prefix where applicable (e.g., `AampConfig.cpp`, `AampScheduler.h`). +- File names must match the primary class implementation they contain. +- Use CamelCase for file names. +- Avoid underscores in new AAMP class file names. + +## 7. Include Guard Convention + +Use `#pragma once` as the preferred include guard for new header files. If traditional include guards are required, follow the pattern: + +```cpp +#ifndef AAMP_CLASSNAME_H +#define AAMP_CLASSNAME_H + +// ... header content ... + +#endif /* AAMP_CLASSNAME_H */ +``` + +## 8. AAMP Logging Guidance + +Prefer AAMP logging macros over `logprintf` for all new code. The supported macros and their intended usage: + +| Macro | Level | Usage | +|-------|-------|-------| +| `AAMPLOG_TRACE` | Trace | Development and triage-level detail; verbose output for deep debugging. | +| `AAMPLOG_INFO` | Info | Informative and debug messages; especially useful during tune operations. | +| `AAMPLOG_WARN` | Warn | Recoverable warnings or important runtime conditions worth noting. | +| `AAMPLOG_ERR` | Error | Severe or unexpected conditions that warrant investigation. | + +### Logging Best Practices +- Use the appropriate log level for the message severity. +- Include relevant context in log messages (function name, identifiers, values). +- Avoid excessive logging in hot paths (e.g., per-fragment processing); prefer `AAMPLOG_TRACE` for these. +- Use correct printf format specifiers (see `cpp.instructions.md` for reference). diff --git a/.github/instructions/cpp.instructions.md b/.github/instructions/cpp.instructions.md index 5e864dd51..3197b1deb 100644 --- a/.github/instructions/cpp.instructions.md +++ b/.github/instructions/cpp.instructions.md @@ -12,7 +12,7 @@ applyTo: ## C++ Guidelines -- Target C++17 for new code. Use C++20+ features only when they are supported by the toolchain, clearly documented, and they provide a meaningful improvement in code quality. +- The existing AAMP codebase is predominantly C++11. New code must target C++17. Use C++20+ features only when they are supported by the toolchain, clearly documented, and they provide a meaningful improvement in code quality. - Always follow the guidelines at [C++ Core Guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) - Highlight when existing code being studied does not follow the core guidelines and suggest improvements - Discourage the use of C-style code within C++ (e.g. avoid memcpy(), memcmp() and char* for strings). Emphasise memory safety @@ -45,14 +45,46 @@ This project uses a strict set of C++ coding standards designed for embedded sys - Pass by reference or pointer to avoid unnecessary copies. ## 3. Commenting & Documentation -- Use Doxygen `///` style comments for public API. +- Use C-style Doxygen comment blocks (`/** ... */`) for all public API documentation. +- Do not use `///` style for AAMP public API documentation. +- Place function documentation with the declaration in the header file; do not duplicate it in the `.cpp` definition. - Document non-obvious logic with concise `//` inline comments. - All major classes must include a brief “Purpose” description. -## 4. Memory Management -- Prefer modern C++ smart pointers (`std::unique_ptr`, `std::shared_ptr`). -- Avoid raw new/delete except when dealing with legacy code paths. +## 4. Memory Management & Ownership +- Prefer modern C++ smart pointers for ownership (`std::unique_ptr`, `std::shared_ptr`). +- Use `std::unique_ptr` as the default for single ownership; use `std::shared_ptr` only when shared ownership is genuinely required. +- Use raw pointers or references only for non-owning access; never use raw owning pointers in new code. +- Avoid raw `new`/`delete` except when dealing with legacy code paths. - Use RAII for all resource management. +- Follow the Rule of Zero: prefer classes that use smart pointers and containers so that no custom destructor, copy, or move operations are needed. + +## 5. Coding Rules +- Braces are required for all conditional and loop blocks, including single-line bodies. +- Use constructor initializer lists to initialize data members where appropriate. +- Keep data members `private` where possible; provide accessor methods when needed. +- Avoid `friend` functions and classes unless there is a strong justification. +- Use `bool` for variables representing logical true/false state. +- Use appropriate standard container size and index types (e.g., `size_t`, `std::vector::size_type`) when indexing or sizing containers. +- Use `#pragma once` or traditional include guards in all header files. + +## 6. Printf Format Specifier Reference +When formatting log output or diagnostic strings, use the correct specifiers: + +| Type | Specifier | +|------|----------| +| `int` | `%d` | +| `unsigned int` | `%u` | +| `long` | `%ld` | +| `unsigned long` | `%lu` | +| `long long` | `%lld` | +| `unsigned long long` | `%llu` | +| `float` | `%f` | +| `double` | `%lf` | +| `size_t` | `%zu` | +| `uint64_t` | `PRIu64` (from ``) | + +Use `PRIu64` and related macros from `` for fixed-width types to ensure portability. ## Cross-Language Interoperability (ctypes) @@ -133,6 +165,8 @@ extern "C" { ## Memory Safety Patterns +Smart pointers express ownership intent. Use `std::unique_ptr` by default for single ownership. Reserve `std::shared_ptr` for genuinely shared ownership. Use raw pointers or references only for non-owning access. + ### Ownership Models ```cpp // Unique ownership @@ -412,3 +446,56 @@ public: // ... }; ``` + +### File Documentation Example +```cpp +/** + * @file AampConfig.h + * @brief Configuration management for AAMP player. + * + * Provides centralized handling of player configuration + * settings loaded from file or set programmatically. + */ +``` + +### Data Member Documentation Example +```cpp +class StreamManager { +private: + std::unique_ptr mBuffer; /**< Internal stream buffer */ + std::string mStreamUrl; /**< URL of the current stream */ + size_t mBufferSize; /**< Buffer size in bytes */ +}; +``` + +### Enum Documentation Example +```cpp +/** + * @enum PlaybackState + * @brief Represents the current state of the player. + */ +enum class PlaybackState +{ + eIDLE, /**< Player is idle */ + ePLAYING, /**< Playback is active */ + ePAUSED, /**< Playback is paused */ + eSTOPPED /**< Playback has stopped */ +}; +``` + +### Macro Documentation Example +```cpp +/** + * @def AAMP_MAX_BUFFER_SIZE + * @brief Maximum buffer size in bytes for stream buffering. + */ +#define AAMP_MAX_BUFFER_SIZE (4 * 1024 * 1024) +``` + +### Static / Global Variable Documentation Example +```cpp +/** + * @brief Default timeout for network requests in milliseconds. + */ +static constexpr int kDefaultNetworkTimeoutMs = 10000; +``` diff --git a/.github/instructions/legacy-cpp-patterns.instructions.md b/.github/instructions/legacy-cpp-patterns.instructions.md index ada784193..a1e87d4ea 100644 --- a/.github/instructions/legacy-cpp-patterns.instructions.md +++ b/.github/instructions/legacy-cpp-patterns.instructions.md @@ -10,6 +10,10 @@ applyTo: # Legacy C++ Modernization Instructions +> **Context:** The existing AAMP codebase is predominantly C++11. +> Modernization efforts should target C++17. +> The patterns below identify legacy and C++11-era anti-patterns and their modern C++17 replacements. + ## Analyzing Legacy Code When reviewing, refactoring or updating existing complex C++ code, focus on identifying these common legacy patterns and suggest modern alternatives: