feat(capture): give CaptureFactory a proper error channel
Replace the nullable unique_ptr returned by CaptureFactory::create() with CaptureResult<std::unique_ptr<CaptureSession>> using the new CaptureError/CaptureResult pattern, mirroring codec/error.h. The stub now reports 'not implemented yet' as an error instead of returning nullptr. Update handoff memory with the review outcome and forward-looking notes for phases 3, 5, and 7.
This commit is contained in:
+39
-11
@@ -1,19 +1,31 @@
|
||||
# Project Memory — screen_cast
|
||||
|
||||
Last updated: initial scaffold.
|
||||
Last updated: Phase 2 codec review fixes applied.
|
||||
|
||||
## Project state
|
||||
|
||||
- Phase 2 follow-up review items addressed: encoder rate control is now
|
||||
bitrate-only (removed CRF), encoder/decoder cache their SwsContext, configs
|
||||
own their strings, bitstream helper is internal to the encoder with a NAL
|
||||
length guard, FFmpeg open errors are reported, and the round-trip test
|
||||
verifies Annex-B prefix and keyframes.
|
||||
- `.clang-format` added at repo root and clang-format enforcement added to
|
||||
`AGENTS.md` and `cpp-meson-build/SKILL.md`.
|
||||
- Phase 3 capture now has a linkable stub (`src/capture/pipewire_capture.cpp`)
|
||||
so the API can be consumed without an unresolved symbol.
|
||||
- Remaining implementation: capture, transport, rendering, and CLI/pipeline glue.
|
||||
- Phase 2 codec implementation reviewed with valgrind; all confirmed defects
|
||||
fixed on top of the capture-stub commit:
|
||||
- decoder leaked every packet payload (`av_malloc` + direct `packet->data`
|
||||
assignment bypassed the packet's owning `AVBufferRef`); payloads are now
|
||||
allocated with `av_new_packet`.
|
||||
- decoder extradata lacked `AV_INPUT_BUFFER_PADDING_SIZE`; FFmpeg's
|
||||
extradata parser over-read the buffer (valgrind invalid reads). Now
|
||||
allocated padded and zeroed.
|
||||
- oversized encoded frames are rejected before the int cast.
|
||||
- EAGAIN-retry loops in encoder/decoder now handle unexpected EOF instead
|
||||
of retrying forever.
|
||||
- fixed "RTP packet" → "AVPacket" error message in the encoder.
|
||||
- `CaptureFactory::create()` now returns
|
||||
`CaptureResult<std::unique_ptr<CaptureSession>>` (variant with
|
||||
`CaptureError`) instead of a nullable unique_ptr; the stub reports
|
||||
"PipeWire capture is not implemented yet" as an error. New pattern lives in
|
||||
`include/screencast/capture/error.h`, mirroring `codec/error.h`.
|
||||
- Validation: `meson test` 2/2 OK; valgrind on `test_codec_roundtrip` is now
|
||||
clean (0 definite losses, 0 invalid reads); clang-format clean. See
|
||||
`docs/RUNBOOK.md` for the reusable checks.
|
||||
- Phase 3 capture stub remains in place; remaining implementation: capture,
|
||||
transport, rendering, and CLI/pipeline glue.
|
||||
|
||||
## Decisions
|
||||
|
||||
@@ -28,6 +40,8 @@ Last updated: initial scaffold.
|
||||
- Discovery: mDNS/Avahi.
|
||||
- Rendering: SDL2 or SDL3 + OpenGL.
|
||||
- Namespace: `sc`.
|
||||
- Module error results use per-module `std::variant<T, XError>` types
|
||||
(`CodecResult`, `CaptureResult`) since C++20 has no `std::expected`.
|
||||
|
||||
## Active blockers
|
||||
|
||||
@@ -38,3 +52,17 @@ None.
|
||||
- GUI framework (Qt6 vs. none / CLI only) — deferred to later phase.
|
||||
- Hardware acceleration strategy (VAAPI / Vulkan Video / NVENC) — evaluate after
|
||||
software encode path works.
|
||||
|
||||
## Forward-looking review notes (for later phases)
|
||||
|
||||
- Encoder rejects non-packed strides in `make_input_frame`; PipeWire/portal
|
||||
frames usually have alignment-padded strides — Phase 3 must pass the real
|
||||
stride through to swscale instead of rejecting it.
|
||||
- `AV_CODEC_FLAG_GLOBAL_HEADER` suppresses in-band SPS/PPS; a receiver cannot
|
||||
join mid-stream or recover after PLI without parameter sets. Phase 5 must
|
||||
prepend SPS/PPS to keyframes or negotiate them in signaling.
|
||||
- Encoder sets no VBV (`maxrate`/`buffer_size`) — ABR only; add for smoother
|
||||
UDP streaming in Phase 7.
|
||||
- No negative-path tests yet (bad config, bad stride, undersized buffer).
|
||||
- `to_annex_b_h264` sniffs AVCC vs Annex-B by content; if an AVCC-emitting
|
||||
encoder is ever added, prefer an explicit config flag over the heuristic.
|
||||
@@ -1,5 +1,7 @@
|
||||
#pragma once
|
||||
|
||||
#include "screencast/capture/error.h"
|
||||
|
||||
#include <cstdint>
|
||||
#include <memory>
|
||||
#include <optional>
|
||||
@@ -52,7 +54,7 @@ class CaptureSession {
|
||||
// Factory for the PipeWire / xdg-desktop-portal capture backend.
|
||||
class CaptureFactory {
|
||||
public:
|
||||
static std::unique_ptr<CaptureSession> create(CaptureTarget target);
|
||||
static CaptureResult<std::unique_ptr<CaptureSession>> create(CaptureTarget target);
|
||||
};
|
||||
|
||||
} // namespace sc
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
#pragma once
|
||||
|
||||
#include <string>
|
||||
#include <variant>
|
||||
|
||||
namespace sc {
|
||||
|
||||
struct CaptureError {
|
||||
std::string message;
|
||||
};
|
||||
|
||||
// C++20 does not provide std::expected. Use a variant-based result type so
|
||||
// fallible capture operations do not rely on exceptions.
|
||||
template <typename T> using CaptureResult = std::variant<T, CaptureError>;
|
||||
|
||||
template <typename T> constexpr bool is_capture_error(const CaptureResult<T>& result) noexcept {
|
||||
return std::holds_alternative<CaptureError>(result);
|
||||
}
|
||||
|
||||
template <typename T> T& capture_value(CaptureResult<T>& result) {
|
||||
return std::get<T>(result);
|
||||
}
|
||||
|
||||
template <typename T> const T& capture_value(const CaptureResult<T>& result) {
|
||||
return std::get<T>(result);
|
||||
}
|
||||
|
||||
template <typename T> CaptureError& capture_error(CaptureResult<T>& result) {
|
||||
return std::get<CaptureError>(result);
|
||||
}
|
||||
|
||||
template <typename T> const CaptureError& capture_error(const CaptureResult<T>& result) {
|
||||
return std::get<CaptureError>(result);
|
||||
}
|
||||
|
||||
} // namespace sc
|
||||
@@ -4,10 +4,10 @@
|
||||
|
||||
namespace sc {
|
||||
|
||||
std::unique_ptr<CaptureSession> CaptureFactory::create(CaptureTarget /*target*/) {
|
||||
CaptureResult<std::unique_ptr<CaptureSession>> CaptureFactory::create(CaptureTarget /*target*/) {
|
||||
// Phase 3 placeholder. The PipeWire / xdg-desktop-portal capture backend
|
||||
// will be implemented here once the codec round-trip is solidified.
|
||||
return nullptr;
|
||||
// will be implemented here once the portal integration lands.
|
||||
return CaptureError{"PipeWire capture is not implemented yet"};
|
||||
}
|
||||
|
||||
} // namespace sc
|
||||
|
||||
Reference in New Issue
Block a user