Skip to content

Re-split #25 into atomic per-issue commits - #38

Open
aa03146 wants to merge 6 commits into
mainfrom
resplit/25-from-main
Open

aa03146 wants to merge 6 commits into
mainfrom
resplit/25-from-main

Conversation

@aa03146

@aa03146 aa03146 commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

修复内容(每个 commit 对应一个独立问题)

  1. fix(rtsp): build SETUP URI from SDP a=control attribute — 依据 SDP a=control 正确拼接 SETUP URI。
  2. fix(rtp): inject SDP SPS/PPS ahead of key frames when absent in-band — 带内缺失 SPS/PPS 时用 SDP 中的参数集在关键帧前注入。
  3. feat(demux): add DEMUX_ProbeStreamInfo for pre-start codec detection — 启动前探测码流信息(编码类型)。
  4. fix(vcodec): propagate buffer-allocation failure in linlonv5v7 codec — linlonv5v7 缓冲分配失败时正确向上传播错误。
  5. fix(vcodec): harden DMA-BUF mmap lifecycle against leaks and double-free — 修复 DMA-BUF mmap 生命周期泄漏与重复释放。
  6. fix(vdec,venc): tear down partially-initialised channel on bring-up failure — 通道初始化失败时清理已建资源(vdec plugin_open 清理、venc pthread 初始化回滚、EnableChn 错误路径关闭 plugin)。

验证(开发板 riscv64)

  • 构建:cmake .. && make -j4 100% 通过,无错误。
  • Lint:lint_cpp.sh 对本 PR 改动的 12 个文件 0 error 0 warning。
  • 回归:sample_file_transcode test_video.mp4 out.h264 --codec h264 --frames 250 → ret=0,输出 2670575 字节,ffprobe 帧数 = 250。

weirongmin added 6 commits July 24, 2026 17:15
The DEMUX RTSP client hardcoded the SETUP request URI as
"rtsp://host:port/<path>/track<N>", which does not match servers
that advertise a different control string. mediamtx advertises
"a=control:trackID=0" with a Content-Base of ".../<path>/", so the
hardcoded form drew a 400 Bad Request and the pull failed.

Capture the raw "a=control:" value per media section in SdpInfo and
build the SETUP URI from it:
  - absolute "rtsp://" control  -> use verbatim
  - relative token/path          -> append to the aggregate URL with a
                                    single '/' separator
  - missing/"*"                   -> fall back to the legacy trackID form
Some RTSP servers (e.g. mediamtx) advertise SPS/PPS only in the SDP
fmtp sprop-parameter-sets and never repeat them in-band before each
IDR. The hardware VDEC then cannot initialise and produces zero
decoded frames.

Before emitting a key frame, splice the stored SPS/PPS
([startcode+SPS][startcode+PPS]) ahead of the frame NALs. Injection is
skipped when the frame already begins with an SPS NAL (in-band
parameter sets present) to avoid duplication, and when there is not
enough room in the frame buffer.
Add a synchronous probe API so callers can learn the real codec type and
resolution (RTSP from SDP; MP4/TS/FLV from the container header) BEFORE
creating a decoder and building the DEMUX->VDEC bind. The live connection
opened by the probe is retained and reused by the worker thread via a new
bPreConnected flag, so the source is never connected twice. If the channel
is already running, the probe degrades to returning the currently published
stream info.
On CMA exhaustion, allocateBuffers() may grant fewer buffers than
requested (or zero). Previously allocateCodecBuffers() returned void and
stream() ignored the result, so a codec could come up with too few or no
buffers, silently drop every frame, and still report init success -
leaving the pipeline stalled with no error.

- allocateCodecBuffers() now returns S32 and treats a partial grant
  (nBufNum != requested) as MPP_INIT_FAILED.
- stream() propagates the failure and calls freeCodecBuffers() to roll
  back before returning.
- al_dec_init()/al_enc_init() check stream()'s return, tear down the
  codec, close nVideoFd and fail instead of returning a dead channel.
- Init/destroy paths initialise nVideoFd to -1 and guard close()/
  destoryCodec() with >=0 / non-NULL checks so partial init cleans up
  safely and destroy is idempotent.
- getBuffer() bounds-checks the index; allocateBuffers() rolls back on a
  partial REQBUFS grant; freeBuffers() is NULL-safe and idempotent.
- Reduce ENCODER_INPUT_BUF_NUM 12->8 to lower CMA pressure.
The internal DMA-BUF map/unmap paths tracked the mapping through
pUserPtr[0]/nTotalLength, which were mutated elsewhere, so a failed or
repeated unmap could munmap the wrong length, leak the fd, or double-free.
Early returns on the first munmap/close error also skipped the remaining
cleanup, leaking the rest of the resources.

- Track the internal mapping explicitly via pInternalDmaMap /
  nInternalDmaMapLength so unmap always uses the exact base/length.
- Guard every munmap with a MAP_FAILED/NULL check and NULL the pointer
  afterwards, making memoryUnmap() idempotent and safe to call twice.
- Accumulate errors into a single ret instead of returning early, so a
  failure on one resource no longer leaks the others.
- createBuffer() fails cleanly if createDmaBufWrapper() returns NULL and
  unmaps on memoryMap() failure; destoryBuffer() is NULL-safe.
- allocDmaBuf() fd<0 checks in memoryMap() surface allocation failures.
- dmabufwrapper: initialise fds to -1, treat fd 0 as valid, guard
  MAP_FAILED, drop early returns in freeDmaBuf(), and call freeDmaBuf()
  from destoryDmaBufWrapper() to avoid leaking the buffer fd.
…ailure

When al_dec_init/al_enc_init failed part-way (e.g. CMA exhausted) or a
thread primitive could not be created, the channel was left holding a
half-built AL context, a loaded plugin handle, and initialised sync
primitives. A later retry then re-opened on top of the leaked state.

- vdec_plugin_open(): on al_dec_init failure destroy the AL context,
  unload the plugin and zero stOps so the channel is fully clean and a
  later Enable retry re-opens from scratch.
- venc_start_threads(): check the return of every pthread_mutex_init /
  pthread_cond_init / pthread_create and roll back the primitives already
  created before returning ERR_VENC_NOMEM, instead of leaking them.
- VENC_EnableChn(): call venc_plugin_close() on the error paths that leave
  the channel IDLE so the plugin/AL context is released, not leaked.
@spacemit-robot-ci

spacemit-robot-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

Robot CI AI Review

结论:发现 3 个需要处理的问题。

发现:

  1. [中等] al/vcodec/linlonv5v7/linlonv5v7_port.c:428-434(allocateBuffers)驱动多分配缓冲区时直接失败
    原代码明确注释"the driver may grant more buffers than requested",并通过截断到 MAX_BUF_NUM 优雅处理;新代码改为 reqbuf.count != (U32)count 时一律释放并返回 MPP_INIT_FAILED,取消了对该已知驱动行为的兼容。如果目标硬件驱动确实会多分配缓冲区(正如原注释所述),会导致编解码器初始化整体失败,属于功能回归。
    建议:区分“少分配”(应失败,避免零缓冲区)和“多分配”(可保留截断处理逻辑)两种情况,而不是一刀切按不等即失败处理。

  2. [中等] mpi/demux/protocol/rtsp/rtsp_client.c:961-987(rtsp_send_setup)无 control 属性时回退格式由 /trackN 改为 /trackID=N
    原代码注释明确说明使用 /track%d(而非 trackID=%d)是为了匹配某些服务器格式;本次改动将无 control 属性时的兜底格式改为 trackID=%d,与该历史修复的意图相反,可能对依赖旧 /trackN 格式且不下发 a=control 的服务器造成新的兼容性问题。
    建议:确认目标服务器场景,若仍需兼容原先针对的服务器,保留 /trackN 作为无 control 属性时的兜底格式。

  3. [轻微] al/vcodec/linlonv5v7/linlonv5v7_buffer.c:465-468,511-514(memoryMap)分配失败时直接返回原始 fd 值而非 MPP 错误码
    allocDmaBuf 失败时返回的负数 fd 被直接作为函数返回值传给调用方(函数签名要求返回 MPP_* 系列错误码),与该模块其余分支(返回 MPP_MMAP_FAILED 等定义好的常量)不一致,可能干扰依赖具体错误码分类处理的调用方/日志。
    建议:统一返回预定义的 MPP_* 错误码(如 MPP_ALLOC_FAILED),而非透传底层 fd/errno 值。

备注:worker-cache review 失败,已回退 baseline review:worker-cache review failed: snode5: busy; hp-probook: sync worker helper failed: ssh: connect to host 10.0.91.193 port 22: No route to host

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant