fix(encoder): 动态调整输出缓冲区大小，解决编码包超限时静默丢帧问题#478
Merged
deepin-bot[bot] merged 1 commit intoMay 29, 2026
Merged
Conversation
Reviewer's GuideRefactors video encoder output buffer handling to dynamically resize the buffer when encoded packet size exceeds the current capacity, with a capped growth strategy and explicit frame dropping and logging when limits or allocations fail, and reuses this logic for both software and VAAPI paths. Sequence diagram for dynamic encoder output buffer resizingsequenceDiagram
participant Encoder as encoder_encode_video
participant EncCtx as encoder_context_t
participant VidCtx as encoder_video_context_t
participant Libs as LoadLibsInstance
Encoder->>Encoder: libav_get_encode
Encoder->>VidCtx: set pts, flags, duration from pkt
Encoder->>VidCtx: resize_outbuf_if_needed
alt pkt size <= outbuf_size
VidCtx->>VidCtx: memcpy(outbuf, pkt->data, pkt->size)
VidCtx-->>Encoder: return 1
else pkt size > outbuf_size
VidCtx->>EncCtx: compute max_reasonable_size
alt pkt size > max_reasonable_size
VidCtx->>Libs: m_av_packet_unref(pkt)
VidCtx->>VidCtx: set last_video_pts
VidCtx-->>Encoder: return 0
Encoder->>Encoder: continue (drop frame)
else pkt size <= max_reasonable_size
VidCtx->>VidCtx: realloc(outbuf, alloc_size)
alt realloc success
VidCtx->>VidCtx: update outbuf and outbuf_size
VidCtx->>VidCtx: memcpy(outbuf, pkt->data, pkt->size)
VidCtx-->>Encoder: return 1
else realloc failed
VidCtx->>Libs: m_av_packet_unref(pkt)
VidCtx->>VidCtx: set last_video_pts
VidCtx-->>Encoder: return 0
Encoder->>Encoder: continue (drop frame)
end
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider using a wider unsigned type (e.g. size_t or int64_t) for
max_reasonable_sizeand thepkt->size/alloc_sizecalculations to avoid potential overflow whenvideo_width * video_heightis large. - The hardcoded
2 * 1024 * 1024limit would be clearer and easier to tune if it were extracted into a named constant that documents the rationale for the minimum buffer size. - You now unref the packet and update
last_video_ptsinsideresize_outbuf_if_needed; please double-check that the surrounding encode loops don’t also unref or otherwise assume ownership ofpkton the dropped-frame path to avoid double-unref or inconsistent PTS handling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using a wider unsigned type (e.g. size_t or int64_t) for `max_reasonable_size` and the `pkt->size`/`alloc_size` calculations to avoid potential overflow when `video_width * video_height` is large.
- The hardcoded `2 * 1024 * 1024` limit would be clearer and easier to tune if it were extracted into a named constant that documents the rationale for the minimum buffer size.
- You now unref the packet and update `last_video_pts` inside `resize_outbuf_if_needed`; please double-check that the surrounding encode loops don’t also unref or otherwise assume ownership of `pkt` on the dropped-frame path to avoid double-unref or inconsistent PTS handling.
## Individual Comments
### Comment 1
<location path="libcam/libcam_encoder/encoder.c" line_range="1593-1602" />
<code_context>
+ * Returns: 1 if data was successfully copied to outbuf,
+ * 0 if frame was dropped (caller must continue to next iteration).
+ */
+static int resize_outbuf_if_needed(encoder_video_context_t *enc_video_ctx,
+ encoder_context_t *encoder_ctx,
+ AVPacket *pkt)
+{
+ if (pkt->size <= enc_video_ctx->outbuf_size) {
+ memcpy(enc_video_ctx->outbuf, pkt->data, pkt->size);
+ return 1;
+ }
+
+ /* Buffer limit: width * height / 2, with 2MB minimum protection */
+ int max_reasonable_size = encoder_ctx->video_width * encoder_ctx->video_height / 2;
+ if (max_reasonable_size < 2 * 1024 * 1024)
+ max_reasonable_size = 2 * 1024 * 1024;
</code_context>
<issue_to_address>
**🚨 issue (security):** Avoid potential integer overflow in max_reasonable_size computation and size handling.
`encoder_ctx->video_width * encoder_ctx->video_height / 2` and `alloc_size` are both `int`. For large resolutions this multiplication can overflow before the division, so `max_reasonable_size` (and later `alloc_size`) may be derived from a corrupted value. Please compute this in a wider type (e.g. `int64_t` or `size_t`), clamp to a sane maximum, then cast down if needed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1593
to
+1602
| static int resize_outbuf_if_needed(encoder_video_context_t *enc_video_ctx, | ||
| encoder_context_t *encoder_ctx, | ||
| AVPacket *pkt) | ||
| { | ||
| if (pkt->size <= enc_video_ctx->outbuf_size) { | ||
| memcpy(enc_video_ctx->outbuf, pkt->data, pkt->size); | ||
| return 1; | ||
| } | ||
|
|
||
| /* Buffer limit: width * height / 2, with 2MB minimum protection */ |
There was a problem hiding this comment.
🚨 issue (security): Avoid potential integer overflow in max_reasonable_size computation and size handling.
encoder_ctx->video_width * encoder_ctx->video_height / 2 and alloc_size are both int. For large resolutions this multiplication can overflow before the division, so max_reasonable_size (and later alloc_size) may be derived from a corrupted value. Please compute this in a wider type (e.g. int64_t or size_t), clamp to a sane maximum, then cast down if needed.
当编码包大小超过输出缓冲区时，替换原有的静默丢弃行为为动态扩容（1.5倍 增长），最大分配上限为 max(width*height/2, 2MB)，防止内存无限增长。 Log: 修复编码器输出缓冲区溢出时静默丢帧的问题 Bug: https://pms.uniontech.com/bug-view-345769.html
deepin pr auto review你好！我是CodeGeeX。我已经仔细审查了你提供的Git Diff。这次修改的主要目的是将原来因缓冲区不足而丢弃数据包的逻辑，优化为动态调整缓冲区大小，从而减少帧丢失并提升编码器的鲁棒性。整体思路非常清晰，但在语法逻辑、代码质量、性能和安全性方面，我发现了几个需要改进的关键点。 以下是详细的审查意见和改进建议： 1. 语法与逻辑
2. 代码质量
3. 代码性能
4. 代码安全
改进后的代码综合以上意见，我为你重构了 /* Minimum reasonable buffer size: 2MB */
#define MIN_REASONABLE_OUTBUF_SIZE (2 * 1024 * 1024)
/*
* Ensure output buffer is large enough for the encoded packet.
* Dynamically resizes the buffer if needed (1.5x reserved space, capped at
* max(width*height/2, MIN_REASONABLE_OUTBUF_SIZE)). Drops the frame if it
* exceeds the reasonable upper limit or if realloc fails.
*
* Returns: 1 if data was successfully copied to outbuf,
* 0 if frame was dropped (caller must continue to next iteration).
*/
static int resize_outbuf_if_needed(encoder_video_context_t *enc_video_ctx,
encoder_context_t *encoder_ctx,
AVPacket *pkt)
{
/* Guard against invalid negative pkt->size to prevent memcpy overflow */
if (pkt->size < 0) {
fprintf(stderr, "ENCODER: invalid packet size %d, dropping frame\n", pkt->size);
return 0;
}
/* Cast to int64_t to prevent integer overflow during multiplication */
int64_t calc_size = (int64_t)encoder_ctx->video_width * encoder_ctx->video_height / 2;
int max_reasonable_size = (calc_size < MIN_REASONABLE_OUTBUF_SIZE) ?
MIN_REASONABLE_OUTBUF_SIZE : (int)calc_size;
/* If packet size exceeds the absolute reasonable limit, drop it */
if (pkt->size > max_reasonable_size) {
fprintf(stderr, "ENCODER: packet size %d exceeds reasonable limit (%d), dropping frame\n",
pkt->size, max_reasonable_size);
return 0;
}
/* If current buffer is large enough, just copy */
if (pkt->size <= enc_video_ctx->outbuf_size) {
memcpy(enc_video_ctx->outbuf, pkt->data, pkt->size);
return 1;
}
/*
* Need to resize. Calculate new size with 1.5x reserved space.
* If the required size with reservation exceeds the limit,
* fallback to exact size to avoid truncating the reservation which
* leads to frequent reallocations.
*/
int alloc_size = pkt->size + (pkt->size >> 1);
if (alloc_size > max_reasonable_size) {
alloc_size = pkt->size; /* Fallback to exact fit if reservation exceeds limit */
}
fprintf(stderr, "ENCODER: resizing output buffer (%i -> %i, reserved %i)\n",
enc_video_ctx->outbuf_size, pkt->size, alloc_size);
uint8_t *new_buf = realloc(enc_video_ctx->outbuf, alloc_size);
if (new_buf) {
enc_video_ctx->outbuf = new_buf;
enc_video_ctx->outbuf_size = alloc_size;
/* Safety check before memcpy to prevent heap overflow due to logic errors */
if (pkt->size > enc_video_ctx->outbuf_size) {
fprintf(stderr, "ENCODER: fatal logic error, pkt->size > outbuf_size\n");
return 0; /* Should never happen, but defensive programming */
}
memcpy(enc_video_ctx->outbuf, pkt->data, pkt->size);
return 1;
}
/*
* CRITICAL: If realloc fails, the original buffer is still valid.
* We MUST NOT overwrite enc_video_ctx->outbuf with NULL, otherwise
* we leak the old buffer and cause a segfault on next access.
* We just drop the current frame and keep the old buffer for future smaller frames.
*/
fprintf(stderr, "ENCODER: failed to allocate %d bytes, dropping frame (old buffer retained)\n", alloc_size);
return 0;
}调用处的修改建议在 if (!resize_outbuf_if_needed(enc_video_ctx, encoder_ctx, pkt)) {
last_video_pts = enc_video_ctx->pts;
getLoadLibsInstance()->m_av_packet_unref(pkt);
// 建议增加：清空当前帧的长度记录，防止外部读取到脏数据
// enc_video_ctx->encoded_size = 0;
continue;
}希望这些审查意见和代码改进能帮助你提升编码器的稳定性和安全性！如果有任何疑问，欢迎继续讨论。 |
max-lvs
approved these changes
May 29, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: max-lvs, Resurgamz The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
当编码包大小超过输出缓冲区时，替换原有的静默丢弃行为为动态扩容（1.5倍 增长），最大分配上限为 max(width*height/2, 2MB)，防止内存无限增长。
Log: 修复编码器输出缓冲区溢出时静默丢帧的问题
Bug: https://pms.uniontech.com/bug-view-345769.html
Summary by Sourcery
Bug Fixes: