| description | C++ instructions | |||||
|---|---|---|---|---|---|---|
| applyTo |
|
- 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
- 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
- Apply DRY (Don't Repeat Yourself) principles throughout the codebase
- Ensure cyclomatic complexity is minimised both for new code and when refactoring
- Use design patterns where possible
- Use const correctness throughout the codebase
- Prefer auto for type deduction when it improves readability
- Use constexpr for compile-time constants and functions when possible
- Leverage range-based for loops and STL algorithms
- Use explicit constructors to prevent implicit conversions
- Include the RDK copyright header in all new files
- Use Doxygen tags for documentation
- Use universal initialisation in all generated code
This project uses a strict set of C++ coding standards designed for embedded systems and video streaming performance requirements.
- Use
camelCasefor variables. - Use
PascalCasefor class and function names. - Use
UPPER_CASEfor constants. - Prefix member variables with
m(e.g.,mBufferSize). - Prefix global variables with
g(e.g.,gConfigValue).
- All classes must have declarations in
.hfiles and implementations in.cppfiles. - Use namespaces to group functionality and avoid collisions.
- Prefer forward declarations when possible.
- Use
constandconstexprappropriately. - Pass by reference or pointer to avoid unnecessary copies.
Use block-style (/** ... */) Doxygen comments for all public API documentation — classes, public functions, constructors, macros, and file headers. This is the canonical style for AAMP.
Use trailing-line (///<) Doxygen comments for struct/class members, enum values, and short field annotations. Keeping the comment on the same line as the declaration prevents "vertical drift" between the field and its description.
Use plain // for non-Doxygen inline notes on implementation logic that does not require extraction.
Discouraged patterns:
- Using
///(without<) for member documentation — prefer///<so Doxygen attributes the comment to the preceding member. - Mixing
/** */and///<arbitrarily within the same class or struct. - Huge
/** ... */banners for trivial one-line members. - Plain
//where Doxygen extraction is intended. - Comments that merely restate the variable name or type.
- Place function and class documentation with the declaration in the header file; do not duplicate it in the
.cppdefinition. - Keep member comments trailing on the same line as the declaration. Use a preceding
/** ... */block only when a multi-line explanation cannot fit on one line.
-
Document meaning, not type. Do not restate what the name or type already expresses. Document: units, valid range, semantics, ownership, lifecycle, and invariants.
-
Keep documentation adjacent to the member. Use
///<trailing comments so documentation stays physically close to the declaration it describes. -
Use
///<for struct/class/enum members — the de facto Doxygen standard for after-member documentation:struct Session { int id; ///< Unique session identifier bool isActive; ///< True if session is currently active };
-
Be concise by default. One trailing line is the target. Expand only when constraints require it:
int retries; ///< Number of retry attempts before failure (>= 0; -1 disables)
-
Document non-obvious constraints, especially in systems code: units (ms, bytes, frames), sentinel values, ownership/lifetime, and thread-safety expectations.
-
Prefer grouping for related members with a brief leading comment rather than repeating context on every line.
-
Avoid redundant comments. A comment that restates only the name adds noise:
int count; ///< Number of active connections // Good: adds meaning int count; ///< Count // Bad: restates the name
-
Keep style consistent within a class or struct. Do not alternate between
/** */and///<for members in the same block. Consistency matters more than exact syntax choice. -
Document only relevant members. Always document public and protected members. Document private members only when they carry non-obvious invariants, ownership semantics, or lifecycle constraints.
-
Keep comments synchronised with code. Update or remove documentation when behaviour changes. Stale comments are actively harmful.
- All major classes must include a brief "Purpose" description in their class-level
/** ... */block.
- Prefer modern C++ smart pointers for ownership (
std::unique_ptr,std::shared_ptr). - Use
std::unique_ptras the default for single ownership; usestd::shared_ptronly 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/deleteexcept 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.
- 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
privatewhere possible; provide accessor methods when needed. - Avoid
friendfunctions and classes unless there is a strong justification. - Use
boolfor 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 onceor traditional include guards in all header files.
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.
When defining data structures or function signatures that will be accessed from other languages (like Python's ctypes library), avoid using built-in C++ types like int, long, or unsigned int, as their size can vary across different platforms and compilers.
Instead, use the fixed-width integer types from the <cstdint> header to ensure a consistent memory layout.
int8_t,int16_t,int32_t,int64_tuint8_t,uint16_t,uint32_t,uint64_t
#include <cstdint>
// This structure is designed to be shared with Python.
// Using fixed-width types ensures a predictable layout.
struct PlayerStats {
uint64_t frames_decoded;
uint32_t frames_dropped;
int32_t current_bitrate;
uint8_t audio_stream_id;
uint8_t video_stream_id;
// Note: Add padding if necessary for alignment on some platforms.
};
// C-style function to be exported for ctypes
extern "C" {
void get_player_stats(PlayerStats* stats);
}- Smart pointers (unique_ptr, shared_ptr, weak_ptr)
- Range-based for loops
- Auto keyword for type deduction
- Lambda expressions
- Move semantics and rvalue references
- Initializer lists
- nullptr instead of NULL
- enum class instead of enum
- Generic lambdas
- std::make_unique (Remind that this is not available in C++11)
- Variable templates
- Return type deduction for functions
- std::optional for nullable values
- std::variant for type-safe unions
- Structured bindings
- if constexpr for conditional compilation
- std::string_view for efficient string handling
- Raw pointer ownership → Smart pointers
- Manual memory management → RAII
- C-style casts → static_cast, dynamic_cast, const_cast
- char* strings → std::string or std::string_view
- Manual loops → STL algorithms
- Macros for constants → constexpr variables
- void* for generic programming → Templates
- When modifying existing functions, suggest modern C++ equivalents
- Provide migration paths that maintain binary compatibility when needed
- Highlight opportunities to reduce complexity through modern features
- Suggest incremental improvements rather than complete rewrites
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.
// Unique ownership
std::unique_ptr<Resource> resource = std::make_unique<Resource>();
// Shared ownership
std::shared_ptr<Resource> shared_resource = std::make_shared<Resource>();
// Non-owning reference (prefer to raw pointers)
Resource& ref = *resource;class ResourceManager {
public:
ResourceManager() : resource_(acquire_resource()) {}
~ResourceManager() { release_resource(resource_); }
// Non-copyable, movable
ResourceManager(const ResourceManager&) = delete;
ResourceManager& operator=(const ResourceManager&) = delete;
ResourceManager(ResourceManager&&) = default;
ResourceManager& operator=(ResourceManager&&) = default;
private:
ResourceHandle resource_;
};// Good: Use exceptions for truly exceptional situations
void parse_config(const std::string& filename) {
std::ifstream file(filename);
if (!file) {
throw std::runtime_error("Cannot open config file: " + filename);
}
// ... parsing logic
}// Good: Use optional for operations that may legitimately fail
std::optional<int> parse_integer(const std::string& str) {
int ret:
try
{
ret = std::stoi(str);
}
catch (const std::exception&)
{
ret = std::nullopt;
}
return ret;
}NEVER use assert() in production code. Assert statements are disabled in release builds and should not be relied upon for error handling or validation in production environments.
assert()is compiled out in release builds (whenNDEBUGis defined)- Production code must handle all error conditions gracefully
- Proper error handling and logging must be used instead
- Unit tests should validate conditions, not production assertions
// BAD: Using assert in production code
assert(ptr != nullptr);
assert(index < container.size());
// GOOD: Proper error handling with logging
if (ptr == nullptr) {
AAMPLOG_ERR("Null pointer detected in %s", __FUNCTION__);
return ERROR_NULL_POINTER;
}
if (index >= container.size()) {
AAMPLOG_WARN("Index %d out of bounds (size: %d)", index, container.size());
return ERROR_INDEX_OUT_OF_BOUNDS;
}
// GOOD: Use exceptions for truly exceptional cases
if (critical_resource == nullptr) {
throw std::runtime_error("Critical resource initialization failed");
}
// GOOD: Use std::optional for operations that may fail
std::optional<Value> get_value_safely(int index) {
if (index >= 0 && index < container.size()) {
return container[index];
}
return std::nullopt;
}If you need debug-time validation, use conditional compilation:
#ifdef DEBUG
if (condition_that_should_never_fail) {
AAMPLOG_ERR("Debug assertion failed: %s", "condition description");
// Handle gracefully even in debug builds
}
#endifMove semantics are critical for performance in embedded systems. Always prefer moving over copying when transferring ownership or when temporary objects are involved.
- Use
std::move()to explicitly move from lvalue references when appropriate - Implement move constructors and move assignment operators for resource-owning classes
- Return large objects by value and rely on move semantics for efficiency
- Use perfect forwarding in templates to preserve value categories
class StreamBuffer {
public:
// Move constructor
StreamBuffer(StreamBuffer&& other) noexcept
: data_(std::exchange(other.data_, nullptr))
, size_(std::exchange(other.size_, 0))
, capacity_(std::exchange(other.capacity_, 0)) {
}
// Move assignment operator
StreamBuffer& operator=(StreamBuffer&& other) noexcept {
if (this != &other) {
delete[] data_;
data_ = std::exchange(other.data_, nullptr);
size_ = std::exchange(other.size_, 0);
capacity_ = std::exchange(other.capacity_, 0);
}
return *this;
}
private:
uint8_t* data_;
size_t size_;
size_t capacity_;
};template<typename T, typename... Args>
std::unique_ptr<T> make_stream_component(Args&&... args) {
return std::make_unique<T>(std::forward<Args>(args)...);
}// Prefer emplace operations to avoid unnecessary copies
std::vector<StreamData> streams;
streams.emplace_back(stream_id, std::move(buffer_data));
// Use move when transferring ownership
auto new_stream = std::move(old_stream);
// Return by value - move semantics handle efficiency
StreamConfig create_default_config() {
StreamConfig config; // Local object
// ... configure object
return config; // Moved automatically (NRVO/move)
}- Prefer stack allocation over heap allocation
- Use object pools for frequently allocated/deallocated objects
- Consider std::array over std::vector for fixed-size collections
- Use reserve() for std::vector when size is known
// Prefer string_view for read-only string parameters
void process_string(std::string_view str);
// Use string concatenation efficiently
std::string result;
result.reserve(estimated_size); // Avoid reallocations- Use templates for zero-cost abstractions
- Prefer constexpr functions for compile-time computation
- Use SFINAE or concepts to constrain templates appropriately
/**
* @brief Manages video stream playback with embedded system optimizations
*
* This class provides efficient video stream handling optimized for
* embedded systems with limited resources. It implements RAII principles
* for automatic resource cleanup.
*
* @note Thread-safe for concurrent read operations
* @warning Not thread-safe for write operations
*/
class VideoStreamManager {
public:
/**
* @brief Constructs a video stream manager with specified buffer size
*
* @param buffer_size Size of the internal buffer in bytes
* @param stream_url URL of the video stream to manage
*
* @throws std::invalid_argument if buffer_size is zero
* @throws std::runtime_error if stream_url is malformed
*/
VideoStreamManager(size_t buffer_size, std::string_view stream_url);
/**
* @brief Starts playback of the configured stream
*
* @return true if playback started successfully, false otherwise
*
* @pre Stream must be configured and resources allocated
* @post If successful, playback state is active
*/
bool start_playback() noexcept;
private:
std::unique_ptr<StreamBuffer> buffer_; ///< Internal stream buffer
std::string stream_url_; ///< URL of the current stream
};Use /** ... */ block comments for API documentation (classes, functions, macros, file headers) and ///< trailing comments for member documentation (struct fields, enum values, class members). See section 3 for full guidance.
/**
* @brief A brief description of what the function does.
*
* A more detailed description of the function's behavior, purpose,
* and any relevant context.
*
* @param param1 Description of the first parameter.
* @param param2 Description of the second parameter.
*
* @return A description of the return value.
*
* @note Optional note about the function.
* @warning Optional warning about potential issues or side effects.
*/
int exampleFunction(int param1, const std::string& param2);/**
* @class MyClass
* @brief A brief description of the class.
*
* More detailed information about the class's responsibilities
* and how it should be used.
*/
class MyClass {
public:
// ...
};/**
* @file AampConfig.h
* @brief Configuration management for AAMP player.
*
* Provides centralized handling of player configuration
* settings loaded from file or set programmatically.
*/class StreamManager {
private:
std::unique_ptr<StreamBuffer> mBuffer; ///< Internal stream buffer; owns the allocation
std::string mStreamUrl; ///< URL of the current stream
size_t mBufferSize; ///< Buffer size in bytes (must be > 0)
};/**
* @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
};/**
* @def AAMP_MAX_BUFFER_SIZE
* @brief Maximum buffer size in bytes for stream buffering.
*/
#define AAMP_MAX_BUFFER_SIZE (4 * 1024 * 1024)/**
* @brief Default timeout for network requests in milliseconds.
*/
static constexpr int kDefaultNetworkTimeoutMs = 10000;