Repository navigation
RDKEMW-28267: Memory/efficiency - AES in-place, static CC colors, JsonObject move, scheduler guard - #335
Open
jaihere4u wants to merge 1 commit into
Open
RDKEMW-28267: Memory/efficiency - AES in-place, static CC colors, JsonObject move, scheduler guard#335jaihere4u wants to merge 1 commit into
jaihere4u wants to merge 1 commit into
Conversation
…guard - Aes::Decrypt: in-place EVP decrypt instead of per-segment malloc+memcpy - gstsubtecsink render: remove dead std::string + per-byte push_back; use data()+size() directly - TtmlSubtecParser: parse mdat straight into the output vector instead of malloc->copy->free per TTML fragment - Gate gst_structure_to_string metadata serialization behind MW_LOG_LEVEL debug checks (2 sites: InterfacePlayerRDK, Mp4Demux) - PlayerCCManager getColor: replace one-shot malloc'd color array (never freed, raced on flagForMalloc) with static storage + static-init fill - PacketSender: release mPktMutex before blocking socket write/reconnect so SendPacket producers cannot stall behind a stalled subtec client - PlayerJsonObject/DrmJsonObject: move ctor now clears the source so a moved-from owning object cannot double cJSON_Delete the tree - PlayerIsoBmffBuffer::parseBuffer: release boxes from a prior parse so a reused instance does not accumulate stale objects - PlayerScheduler: guard task execution - an escaping exception would kill the worker thread and silently drop all queued player work Co-Authored-By: Devin <devin@cognition.ai>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical issues append trailing NUL bytes to TTML payloads and allow an unbounded packet queue to grow during stalled sends, risking memory exhaustion.
4 open issues
What changed in this PR
This PR addresses memory use, efficiency, and reliability in the middleware-player-interface’s decryption, subtitle, JSON, and scheduling code. Performance gains remain unmeasured; unit tests and sanitizer validation are still required.
Changes:
- Reduce temporary allocations, buffer copies, and unnecessary metadata serialization.
- Correct JSON move ownership and release objects retained between buffer parses.
- Catch scheduler task exceptions and release the packet queue mutex during socket writes.
| File | Description |
|---|---|
subtec/subtecparser/TtmlSubtecParser.cpp |
Parse subtitle payloads directly into a vector. |
subtec/libsubtec/PacketSender.cpp |
Release the queue mutex during packet transmission. |
PlayerScheduler.cpp |
Catch task exceptions so queued work can continue. |
playerJsonObject/PlayerJsonObject.h |
Transfer JSON ownership safely during moves. |
playerisobmff/playerisobmffbuffer.cpp |
Delete previous box objects before reparsing. |
mp4demux/mp4demux.hpp |
Serialize metadata only when verbose logging is enabled. |
InterfacePlayerRDK.cpp |
Guard DRM metadata serialization by debug log level. |
gst-plugins/gst_subtec/gstsubtecsink.cpp |
Replace per-byte copying with vector assignment. |
drm/DrmJsonObject.h |
Transfer DRM JSON ownership safely during moves. |
drm/aes/Aes.cpp |
Decrypt in place and zero the trailing buffer bytes on success. |
closedcaptions/PlayerCCManager.cpp |
Replace heap-allocated color storage with statically initialized storage. |
Review effort: Balanced
Comment on lines
+118
to
+119
| lock.unlock(); | ||
| sendPacket(std::move(pkt)); |
| // temporary malloc + copy per fragment. | ||
| std::vector<uint8_t> data(mdatLen); | ||
| data.assign(mdat, mdat+mdatLen); | ||
| isobuf.parseMdatBox(data.data(), mdatLen); |
| { | ||
| obj.mTask(obj.mData); | ||
| } | ||
| catch (const std::exception &e) |
| // EVP supports in-place decryption (out == in): decrypt directly | ||
| // into the input buffer, avoiding a per-segment temp allocation | ||
| // and a full-size copy-back. | ||
| if (!EVP_DecryptUpdate(OPEN_SSL_CONTEXT, (unsigned char*)encryptedDataPtr, &decLen, (const unsigned char*)encryptedDataPtr, (int)encryptedDataLen)) |
jaihere4u
changed the title
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
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.
Jira: RDKEMW-28267 - Middleware PI memory & reliability audit.
Findings covered: HEAP-01..04, MEM-04, REL-01, LAT-01/02, EXE-01 (see ticket + attached MPI_Memory_Audit.html for file:line evidence).
Static-analysis findings; no runtime metrics yet - unit tests + sanitizer runs required before merge.