windows上ccap工具保存jpg、png图片bug处理 - #53
Conversation
ccap保存jpg和png图片反转的bug处理
WalkthroughPNG/JPG scanline orientation logic is inverted to correct directional handling, while outdated orientation documentation is removed. The JPG color conversion path for BGR input is simplified by consolidating duplicate branches into a single consistent loop that converts to RGB before encoding. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@Auggie review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cli/ccap_cli_utils.cpp (1)
850-882: Optional: deduplicate the BGR→RGB conversion and skip alpha for JPG.The JPG and PNG branches now contain near-identical BGR→RGB loops. They can be hoisted into a single helper, and for JPG the alpha copy is unnecessary because
stbi_write_jpgignores the 4th channel (it always encodes RGB regardless ofcomp). Not a correctness issue — purely cleanup.♻️ Sketch of a deduplicated helper
auto convertBgrToRgb = [&](bool keepAlpha) { std::vector<uint8_t> rgbBuffer(static_cast<size_t>(width) * height * comp); const int total = width * height; for (int i = 0; i < total; ++i) { rgbBuffer[i * comp + 0] = imageData[i * comp + 2]; rgbBuffer[i * comp + 1] = imageData[i * comp + 1]; rgbBuffer[i * comp + 2] = imageData[i * comp + 0]; if (keepAlpha && comp == 4) { rgbBuffer[i * comp + 3] = imageData[i * comp + 3]; } } return rgbBuffer; }; if (imageFormat == ImageFormat::JPG) { if (isBGR && comp >= 3) { auto rgb = convertBgrToRgb(/*keepAlpha=*/false); result = stbi_write_jpg(filePath.c_str(), width, height, comp, rgb.data(), jpegQuality); } else { result = stbi_write_jpg(filePath.c_str(), width, height, comp, imageData, jpegQuality); } } else { // PNG if (isBGR && comp >= 3) { auto rgb = convertBgrToRgb(/*keepAlpha=*/true); result = stbi_write_png(filePath.c_str(), width, height, comp, rgb.data(), width * comp); } else { result = stbi_write_png(filePath.c_str(), width, height, comp, imageData, width * comp); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/ccap_cli_utils.cpp` around lines 850 - 882, The BGR→RGB conversion is duplicated in the ImageFormat::JPG and PNG branches; extract it into a single helper (e.g., convertBgrToRgb or a lambda) that takes a keepAlpha flag and returns a std::vector<uint8_t> with channels swapped, ensure you only copy alpha when keepAlpha is true and comp == 4, then call that helper when isBGR && comp >= 3; for JPG call it with keepAlpha=false (since stbi_write_jpg ignores the 4th channel) and for PNG call it with keepAlpha=true, and replace the duplicated loops with calls to this helper before invoking stbi_write_jpg/stbi_write_png.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cli/ccap_cli_utils.cpp`:
- Around line 850-882: The BGR→RGB conversion is duplicated in the
ImageFormat::JPG and PNG branches; extract it into a single helper (e.g.,
convertBgrToRgb or a lambda) that takes a keepAlpha flag and returns a
std::vector<uint8_t> with channels swapped, ensure you only copy alpha when
keepAlpha is true and comp == 4, then call that helper when isBGR && comp >= 3;
for JPG call it with keepAlpha=false (since stbi_write_jpg ignores the 4th
channel) and for PNG call it with keepAlpha=true, and replace the duplicated
loops with calls to this helper before invoking stbi_write_jpg/stbi_write_png.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ff3a2c1-de78-445c-9f65-d7695d3de0c3
📒 Files selected for processing (1)
cli/ccap_cli_utils.cpp
There was a problem hiding this comment.
Pull request overview
该 PR 修复了 ccap CLI 在 Windows 上导出 JPG/PNG 图片上下颠倒的问题，使保存逻辑与 FrameOrientation（TopToBottom / BottomToTop）的语义保持一致，并顺带移除了一段重复/无效的颜色通道转换分支。
Changes:
- 修正 PNG/JPG 保存时的垂直翻转条件：仅在
BottomToTop时进行 flip，以满足 stb 对“首行在内存起始处”的要求 - 删除 JPG 的 BGR→RGB 转换中重复的
tempBuffer.empty()分支逻辑，简化实现
|
感谢修复。 不只是 Windows，其他平台的这条路径也会受影响。已合入改动。 |
目前ccap工具在windows上保存jpg、png图片是反的，可以看一下例图。修改了一下代码，并删除了无用的处理逻辑。
Summary by CodeRabbit