Skip to content

Commit 3d57d3b

Browse files
wysaidCopilot
andcommitted
fix: restructure video writer to fix Windows build and address review comments
Major changes: - Replace ccap_writer.mm with ccap_writer.cpp (pure C++ dispatch layer) using factory function pattern instead of #include of .mm/.cpp files - Compile platform implementations as separate source files (Apple-only .mm, Windows-only .cpp) matching the existing file reader pattern - Rewrite Windows implementation to use MFCreateSinkWriterFromURL (correct API) instead of non-existent MFCreateMediaSinkForURL - Guard #pragma comment(lib) behind _MSC_VER for MinGW compatibility Review comment fixes: - Respect WriterConfig::codec preference (try requested codec first) - Replace usleep() with std::this_thread::sleep_for (portability) - Use high-precision timescale (600000) for CMTime instead of truncating to integer fps - Add reportError() calls throughout (not just CCAP_LOG_E) - Validate frame dimensions match configured output in writeFrame() - Require even dimensions in open() for NV12 encoding - Check m_mfInitialized in Windows open() before calling MF APIs - Fix C API timestamp: pass resolved timestamp to writeFrame() - Share NV12 conversion code via inline helpers in ccap_writer_imp.h - Support I420/BGRA32 conversion on Windows (was BGR24/NV12 only) - Guard playback validation in tests behind CCAP_ENABLE_FILE_PLAYBACK - Zero-initialize VideoFrame structs in tests - Use std::error_code overloads in test TearDown() for robustness - Update frameRate docs to reflect actual behavior (default 30fps) - Set sample duration on Windows for better seek/playback behavior - Run format_all.sh Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c841c1f commit 3d57d3b

12 files changed

Lines changed: 296 additions & 329 deletions

CMakeLists.txt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,20 @@ endif ()
135135
# Video writer sources (Windows and macOS only)
136136
option(CCAP_ENABLE_VIDEO_WRITER "Enable video file writing support (Windows/macOS)" ON)
137137
if (CCAP_ENABLE_VIDEO_WRITER AND (APPLE OR WIN32))
138-
# Exclude writer sources from main glob to avoid double-compilation (platform impl included via #include)
138+
# Exclude writer sources from main glob to avoid double-compilation
139139
list(FILTER LIB_SOURCE EXCLUDE REGEX ".*ccap_writer_apple.*")
140140
list(FILTER LIB_SOURCE EXCLUDE REGEX ".*ccap_writer_windows.*")
141141
list(FILTER LIB_SOURCE EXCLUDE REGEX ".*ccap_writer_c\..*$")
142142
list(FILTER LIB_SOURCE EXCLUDE REGEX ".*ccap_writer\..*$")
143143
list(APPEND LIB_SOURCE
144-
${CMAKE_CURRENT_SOURCE_DIR}/src/ccap_writer.mm
144+
${CMAKE_CURRENT_SOURCE_DIR}/src/ccap_writer.cpp
145145
${CMAKE_CURRENT_SOURCE_DIR}/src/ccap_writer_c.cpp
146146
)
147+
if (APPLE)
148+
list(APPEND LIB_SOURCE ${CMAKE_CURRENT_SOURCE_DIR}/src/ccap_writer_apple.mm)
149+
elseif (WIN32)
150+
list(APPEND LIB_SOURCE ${CMAKE_CURRENT_SOURCE_DIR}/src/ccap_writer_windows.cpp)
151+
endif ()
147152
message(STATUS "ccap: Video file writing support enabled")
148153
else ()
149154
message(STATUS "ccap: Video file writing support disabled (unsupported platform or disabled)")
@@ -152,7 +157,6 @@ endif ()
152157
if (APPLE)
153158
file(GLOB LIB_SOURCE_MAC ${CMAKE_CURRENT_SOURCE_DIR}/src/*.mm)
154159
list(FILTER LIB_SOURCE_MAC EXCLUDE REGEX ".*ccap_writer_apple.*")
155-
list(FILTER LIB_SOURCE_MAC EXCLUDE REGEX ".*ccap_writer\.mm$")
156160
message(STATUS "ccap: Using Objective-C++ for macOS: ${LIB_SOURCE_MAC}")
157161
list(APPEND LIB_SOURCE ${LIB_SOURCE_MAC})
158162
endif ()

include/ccap_writer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ struct WriterConfig {
4747
VideoFormat container = VideoFormat::MP4;
4848
uint32_t width = 0; ///< Frame width in pixels
4949
uint32_t height = 0; ///< Frame height in pixels
50-
double frameRate = 30.0; ///< Target frame rate; 0 = variable rate
50+
double frameRate = 30.0; ///< Target frame rate (default 30fps; used for timestamp generation when timestampNs is 0)
5151
uint64_t bitRate = 5'000'000; ///< Target bit rate in bits/s; 0 = auto
5252
};
5353

include/ccap_writer_c.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ typedef struct {
4545
CcapVideoFormat container; ///< Container format
4646
uint32_t width; ///< Frame width
4747
uint32_t height; ///< Frame height
48-
double frameRate; ///< Target frame rate (0 = variable)
48+
double frameRate; ///< Target frame rate (default 30fps)
4949
uint64_t bitRate; ///< Target bit rate in bits/s (0 = auto)
5050
} CcapWriterConfig;
5151

src/ccap_imp_linux.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,12 @@ class ProviderV4L2 : public ProviderImp {
101101
bool m_isStreaming = false;
102102

103103
// V4L2 device capabilities
104-
struct v4l2_capability m_caps {};
104+
struct v4l2_capability m_caps{};
105105
std::vector<V4L2Format> m_supportedFormats;
106106
std::vector<DeviceInfo::Resolution> m_supportedResolutions;
107107

108108
// Current format
109-
struct v4l2_format m_currentFormat {};
109+
struct v4l2_format m_currentFormat{};
110110

111111
// Buffer management
112112
std::vector<V4L2Buffer> m_buffers;

src/ccap_imp_windows.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,7 +1003,7 @@ HRESULT STDMETHODCALLTYPE ProviderDirectShow::BufferCB(double SampleTime, BYTE*
10031003
return S_OK;
10041004
}
10051005

1006-
HRESULT STDMETHODCALLTYPE ProviderDirectShow::QueryInterface(REFIID riid, _COM_Outptr_ void __RPC_FAR* __RPC_FAR* ppvObject) {
1006+
HRESULT STDMETHODCALLTYPE ProviderDirectShow::QueryInterface(REFIID riid, _COM_Outptr_ void __RPC_FAR * __RPC_FAR * ppvObject) {
10071007
static constexpr const IID IID_ISampleGrabberCB = { 0x0579154A, 0x2B53, 0x4994, { 0xB0, 0xD0, 0xE7, 0x73, 0x14, 0x8E, 0xFF, 0x85 } };
10081008

10091009
if (riid == IID_IUnknown) {
@@ -1168,7 +1168,7 @@ void ProviderDirectShow::close() {
11681168
bool ProviderDirectShow::start() {
11691169
if (!m_isOpened) return false;
11701170

1171-
// File mode
1171+
// File mode
11721172
#ifdef CCAP_ENABLE_FILE_PLAYBACK
11731173
if (m_isFileMode && m_fileReader) {
11741174
return m_fileReader->start();

src/ccap_imp_windows.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class ProviderDirectShow : public ProviderImp, public ISampleGrabberCB {
9393
inline FrameOrientation frameOrientation() const { return m_frameOrientation; }
9494

9595
private:
96-
HRESULT STDMETHODCALLTYPE QueryInterface(REFIID riid, _COM_Outptr_ void __RPC_FAR* __RPC_FAR* ppvObject) override;
96+
HRESULT STDMETHODCALLTYPE QueryInterface(REFIID riid, _COM_Outptr_ void __RPC_FAR * __RPC_FAR * ppvObject) override;
9797
ULONG STDMETHODCALLTYPE AddRef(void) override;
9898
ULONG STDMETHODCALLTYPE Release(void) override;
9999

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,22 @@
11
/**
2-
* @file ccap_writer.mm
2+
* @file ccap_writer.cpp
33
* @author wysaid (this@wysaid.org)
4-
* @brief Video writer platform dispatch layer.
4+
* @brief Video writer platform dispatch layer (pure C++).
55
* @date 2025-05
66
*/
77

88
#include "ccap_writer.h"
9-
#include "ccap_writer_imp.h"
10-
#include "ccap_imp.h"
11-
12-
#include "ccap_convert.h"
13-
#include "ccap_utils.h"
149

15-
#include <cstring>
16-
#include <memory>
17-
#include <vector>
10+
#include "ccap_writer_imp.h"
1811

1912
#ifdef CCAP_ENABLE_VIDEO_WRITER
2013

21-
#if __APPLE__
22-
#include "ccap_writer_apple.mm"
23-
#elif defined(_WIN32) || defined(_MSC_VER)
24-
#include "ccap_writer_windows.cpp"
25-
#endif
26-
2714
namespace ccap {
2815

29-
// ---- Platform dispatch ----
30-
3116
static VideoWriter::Impl* impl(void* p) { return reinterpret_cast<VideoWriter::Impl*>(p); }
3217
static const VideoWriter::Impl* impl(const void* p) { return reinterpret_cast<const VideoWriter::Impl*>(p); }
3318

34-
VideoWriter::VideoWriter() : m_impl(nullptr) {
35-
#if __APPLE__
36-
m_impl = new WriterApple();
37-
#elif defined(_WIN32) || defined(_MSC_VER)
38-
m_impl = new WriterWindows();
39-
#endif
40-
}
19+
VideoWriter::VideoWriter() : m_impl(createVideoWriterImpl()) {}
4120

4221
VideoWriter::~VideoWriter() {
4322
delete impl(m_impl);

0 commit comments

Comments
 (0)