Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.

Expand Down
39 changes: 38 additions & 1 deletion .github/instructions/aamp.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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. |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This list is missing AAMPLOG_MIL. Originally this level did not exist in AAMP and WARN was used for log lines that were important enough to be printed in the log by default, even though they were not warnings. Now we should be using AAMPLOG_MIL instead for those lines.


### 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).
97 changes: 92 additions & 5 deletions .github/instructions/cpp.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 `<cinttypes>`) |

Use `PRIu64` and related macros from `<cinttypes>` for fixed-width types to ensure portability.

## Cross-Language Interoperability (ctypes)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<StreamBuffer> 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;
```
4 changes: 4 additions & 0 deletions .github/instructions/legacy-cpp-patterns.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading