From 48919d01e4f936d23b595f2ada1dfd2f7f53b7d3 Mon Sep 17 00:00:00 2001 From: muhzain05 Date: Mon, 12 Oct 2026 04:06:54 -0600 Subject: [PATCH] fix(videodec2): answer picture info from the decoder that last wrote the buffer sceVideodec2GetPictureInfo searched the live decoders in handle order and returned the first one that had ever written the queried frame buffer. When two decoders wrote the same address, the lower handle answered with stale timestamps, a stale AVC decoder answered for an HEVC or VP9 picture, a stale HEVC or VP9 decoder made an AVC query throw, and resetting or deleting the decoder that wrote the buffer last exposed the older decoder's picture again. Record the decoder that last published a picture into each frame buffer and answer from that decoder only. Reset and Delete drop only the decoder's own records, so another decoder's picture is never removed or brought back. Co-Authored-By: Claude Opus 5.5 --- core/libs/prx/libSceVideodec2/Export.cpp | 29 ++-- core/libs/tests/GuestVideodec2.cpp | 169 +++++++++++++++++++++++ 2 files changed, 189 insertions(+), 9 deletions(-) diff --git a/core/libs/prx/libSceVideodec2/Export.cpp b/core/libs/prx/libSceVideodec2/Export.cpp index a4d91f714..6d9f99f87 100644 --- a/core/libs/prx/libSceVideodec2/Export.cpp +++ b/core/libs/prx/libSceVideodec2/Export.cpp @@ -147,6 +147,7 @@ struct Decoder { std::mutex lock; std::map> decoders; +std::map pictureOwners; std::uint64_t nextDecoder = 1; std::uint64_t nextQueue = 1; @@ -266,7 +267,7 @@ bool WriteNv12(Decoder& decoder, const AVFrame& frame, std::uint8_t* target, std return sws_scale(decoder.scaler, frame.data, frame.linesize, 0, frame.height, planes, strides) == frame.height; } -void EmitDecoded(Decoder& decoder, FrameBuffer* frame, OutputInfo* output) { +void EmitDecoded(std::uint64_t handle, Decoder& decoder, FrameBuffer* frame, OutputInfo* output) { output->isValid = false; output->isErrorFrame = false; output->pictureCount = 0; @@ -302,6 +303,8 @@ void EmitDecoded(Decoder& decoder, FrameBuffer* frame, OutputInfo* output) { output->framePitchInBytes = pitch; } decoder.pictures[frame->frameBuffer] = timestamps; + std::lock_guard guard(lock); + if (decoders.contains(handle)) pictureOwners[frame->frameBuffer] = handle; } } @@ -358,6 +361,7 @@ int APS5_VABI sceVideodec2CreateDecoder_nid_postfix(const DecoderConfigInfo* con int APS5_VABI sceVideodec2DeleteDecoder_nid_postfix(std::uint64_t handle) { std::lock_guard guard(lock); if (decoders.erase(handle) == 0) throw std::runtime_error("Videodec2: invalid decoder handle"); + std::erase_if(pictureOwners, [handle](const auto& owner) { return owner.second == handle; }); return 0; } @@ -383,7 +387,7 @@ int APS5_VABI sceVideodec2Decode_nid_postfix(std::uint64_t handle, const InputDa packet->pts = decoder.nextKey; decoder.inputs.emplace(decoder.nextKey++, picture); Send(decoder, packet); - EmitDecoded(decoder, frame, output); + EmitDecoded(handle, decoder, frame, output); return 0; } @@ -397,7 +401,7 @@ int APS5_VABI sceVideodec2Flush_nid_postfix(std::uint64_t handle, FrameBuffer* f Send(decoder, nullptr); decoder.flushing = true; } - EmitDecoded(decoder, frame, output); + EmitDecoded(handle, decoder, frame, output); return 0; } @@ -407,6 +411,10 @@ int APS5_VABI sceVideodec2Reset_nid_postfix(std::uint64_t handle) { auto& decoder = *found; std::lock_guard guard(decoder.mutex); decoder.pictures.clear(); + { + std::lock_guard ownersGuard(lock); + std::erase_if(pictureOwners, [handle](const auto& owner) { return owner.second == handle; }); + } avcodec_flush_buffers(decoder.context); decoder.ready.clear(); decoder.inputs.clear(); @@ -417,15 +425,18 @@ int APS5_VABI sceVideodec2Reset_nid_postfix(std::uint64_t handle) { int APS5_VABI sceVideodec2GetPictureInfo_nid_postfix(const OutputInfo* output, AvcPictureInfo* first, void* second) { (void)second; if (!output || !first) throw std::runtime_error("Videodec2: invalid argument pointer"); - std::vector> all; - { + const auto owner = [output]() -> std::uint64_t { std::lock_guard guard(lock); - for (const auto& [handle, decoder] : decoders) all.push_back(decoder); - } - for (const auto& decoder : all) { + const auto found = pictureOwners.find(output->frameBuffer); + return found != pictureOwners.end() ? found->second : 0; + }; + for (auto handle = owner(); handle != 0; handle = owner()) { + const auto decoder = FindDecoder(handle); + if (!decoder) continue; std::lock_guard guard(decoder->mutex); + if (owner() != handle) continue; const auto found = decoder->pictures.find(output->frameBuffer); - if (found == decoder->pictures.end()) continue; + if (found == decoder->pictures.end()) throw std::runtime_error("Videodec2: owned picture is missing"); if (decoder->codec != CodecAvc) throw std::runtime_error("Videodec2: picture info is only implemented for H.264"); const auto& picture = found->second; first->isValid = true; diff --git a/core/libs/tests/GuestVideodec2.cpp b/core/libs/tests/GuestVideodec2.cpp index 29649d183..45c19b990 100644 --- a/core/libs/tests/GuestVideodec2.cpp +++ b/core/libs/tests/GuestVideodec2.cpp @@ -1,9 +1,11 @@ #include #include +#include #include #include #include #include +#include #include #include "prx/libc/include/General.hpp" #include "H264Fixture.hpp" @@ -444,6 +446,171 @@ void testDecode(bool lengthPrefixed) { checkThrows([&] { sceVideodec2DeleteDecoder_nid_postfix(handle); }); } +std::uint64_t createDecoder(std::uint32_t codec, std::uint32_t profile) { + const DecoderConfigInfo config{sizeof(DecoderConfigInfo), 1, codec, profile, 0, static_cast(Width), static_cast(Height), 4, 1}; + DecoderMemoryInfo memory{sizeof(DecoderMemoryInfo)}; + check(sceVideodec2QueryDecoderMemoryInfo_nid_postfix(&config, &memory) == 0, "memory query failed"); + std::uint64_t handle = 0; + check(sceVideodec2CreateDecoder_nid_postfix(&config, &memory, &handle) == 0, "decoder creation failed"); + return handle; +} + +AvcPictureInfo query(std::vector& buffer) { + OutputInfo output{sizeof(OutputInfo)}; + output.frameBuffer = buffer.data(); + AvcPictureInfo info{sizeof(AvcPictureInfo)}; + check(sceVideodec2GetAvcPictureInfo_nid_postfix(&output, &info, nullptr) == 0, "picture info query failed"); + return info; +} + +bool owned(std::vector& buffer, std::uint64_t pts) { + const auto info = query(buffer); + return info.isValid && info.ptsData == pts; +} + +std::uint64_t publishAvc(std::uint64_t handle, std::uint64_t base, std::vector& buffer) { + const auto units = accessUnits(false); + std::size_t pictures = 0; + const auto take = [&](const OutputInfo& output) { + if (!output.isValid) return false; + check(owned(buffer, base + DisplayOrderUnits[pictures]), "picture info does not follow the latest publication"); + ++pictures; + return true; + }; + for (std::size_t unit = 0; unit < units.size(); ++unit) { + const InputData input{sizeof(InputData), units[unit].data(), units[unit].size(), base + unit, unit, base + unit}; + FrameBuffer frame{sizeof(FrameBuffer), buffer.data(), buffer.size(), false}; + OutputInfo output{sizeof(OutputInfo)}; + check(sceVideodec2Decode_nid_postfix(handle, &input, &frame, &output) == 0, "decode failed"); + take(output); + } + for (;;) { + FrameBuffer frame{sizeof(FrameBuffer), buffer.data(), buffer.size(), false}; + OutputInfo output{sizeof(OutputInfo)}; + check(sceVideodec2Flush_nid_postfix(handle, &frame, &output) == 0, "flush failed"); + if (!take(output)) break; + } + check(pictures == PictureHashes.size(), "missing pictures after flush"); + return base + DisplayOrderUnits[pictures - 1]; +} + +void publishClip(std::uint64_t handle, const std::vector>& units, std::vector& buffer) { + for (std::size_t unit = 0; unit < units.size(); ++unit) { + const InputData input{sizeof(InputData), units[unit].data(), units[unit].size(), unit, unit, 0}; + FrameBuffer frame{sizeof(FrameBuffer), buffer.data(), buffer.size(), false}; + OutputInfo output{sizeof(OutputInfo)}; + check(sceVideodec2Decode_nid_postfix(handle, &input, &frame, &output) == 0, "decode failed"); + } + for (;;) { + FrameBuffer frame{sizeof(FrameBuffer), buffer.data(), buffer.size(), false}; + OutputInfo output{sizeof(OutputInfo)}; + check(sceVideodec2Flush_nid_postfix(handle, &frame, &output) == 0, "flush failed"); + if (!output.isValid) break; + } +} + +std::uint64_t frameBufferBytes() { + const DecoderConfigInfo config{sizeof(DecoderConfigInfo), 1, 1, 100, 0, static_cast(Width), static_cast(Height), 4, 1}; + DecoderMemoryInfo memory{sizeof(DecoderMemoryInfo)}; + check(sceVideodec2QueryDecoderMemoryInfo_nid_postfix(&config, &memory) == 0, "memory query failed"); + return memory.maxFrameBufferSize; +} + +void testPictureOwnership() { + std::vector shared(frameBufferBytes()), other(frameBufferBytes()); + { + const auto older = createDecoder(1, 100); + const auto newer = createDecoder(1, 100); + publishAvc(older, 1000, shared); + const auto latest = publishAvc(newer, 5000, shared); + check(owned(shared, latest), "an older decoder answered for a buffer a newer decoder wrote last"); + check(sceVideodec2Reset_nid_postfix(newer) == 0, "reset failed"); + check(!query(shared).isValid, "a stale picture survived the reset of the decoder that last wrote the buffer"); + check(sceVideodec2DeleteDecoder_nid_postfix(older) == 0 && sceVideodec2DeleteDecoder_nid_postfix(newer) == 0, "delete failed"); + } + { + const auto older = createDecoder(1, 100); + const auto newer = createDecoder(1, 100); + publishAvc(older, 1000, shared); + publishAvc(newer, 5000, shared); + check(sceVideodec2DeleteDecoder_nid_postfix(newer) == 0, "delete failed"); + check(!query(shared).isValid, "a stale picture survived the deletion of the decoder that last wrote the buffer"); + check(sceVideodec2DeleteDecoder_nid_postfix(older) == 0, "delete failed"); + } + { + const auto older = createDecoder(1, 100); + const auto newer = createDecoder(1, 100); + publishAvc(older, 1000, shared); + const auto latest = publishAvc(newer, 5000, shared); + check(sceVideodec2Reset_nid_postfix(older) == 0, "reset failed"); + check(owned(shared, latest), "resetting a decoder that no longer owns the buffer dropped its picture"); + check(sceVideodec2DeleteDecoder_nid_postfix(older) == 0, "delete failed"); + check(owned(shared, latest), "deleting a decoder that no longer owns the buffer dropped its picture"); + check(sceVideodec2DeleteDecoder_nid_postfix(newer) == 0, "delete failed"); + check(!query(shared).isValid, "a deleted decoder still owns its picture"); + } + { + const auto older = createDecoder(1, 100); + const auto newer = createDecoder(1, 100); + publishAvc(newer, 5000, shared); + const auto latest = publishAvc(older, 1000, shared); + const auto third = createDecoder(1, 100); + const auto separate = publishAvc(third, 9000, other); + check(owned(shared, latest) && owned(other, separate), "distinct buffers are not independent"); + check(sceVideodec2Reset_nid_postfix(older) == 0, "reset failed"); + check(!query(shared).isValid && owned(other, separate), "resetting one buffer's owner changed another buffer"); + check(sceVideodec2DeleteDecoder_nid_postfix(older) == 0 && sceVideodec2DeleteDecoder_nid_postfix(newer) == 0 && sceVideodec2DeleteDecoder_nid_postfix(third) == 0, "delete failed"); + } + for (const auto codec : {CodecHevc, CodecVp9}) { + const auto units = codec == CodecHevc ? hevcUnits(HevcStream.data(), HevcStream.size()) : ivfFrames(); + const auto avc = createDecoder(1, 100); + const auto foreign = createDecoder(codec, codec == CodecHevc ? 1 : 0); + publishAvc(avc, 1000, shared); + publishClip(foreign, units, shared); + checkThrows([&] { query(shared); }); + check(sceVideodec2DeleteDecoder_nid_postfix(avc) == 0, "delete failed"); + const auto newer = createDecoder(1, 100); + const auto latest = publishAvc(newer, 5000, shared); + check(owned(shared, latest), "a decoder of another codec answered for an AVC picture"); + check(sceVideodec2DeleteDecoder_nid_postfix(foreign) == 0 && sceVideodec2DeleteDecoder_nid_postfix(newer) == 0, "delete failed"); + } +} + +void testConcurrentOwnership() { + std::vector shared(frameBufferBytes()); + const auto keeper = createDecoder(1, 100); + const auto kept = publishAvc(keeper, 1000, shared); + std::atomic done = false; + std::string failure; + std::thread reader([&] { + try { + while (!done.load()) { + const auto info = query(shared); + check(!info.isValid || info.ptsData == kept || (info.ptsData >= 5000 && info.ptsData < 5000 + PictureHashes.size()), "a query returned metadata no decoder published"); + } + } catch (const std::exception& error) { + failure = error.what(); + } + }); + try { + for (int round = 0; round < 8; ++round) { + const auto writer = createDecoder(1, 100); + publishAvc(writer, 5000, shared); + if (round % 2 == 0) check(sceVideodec2Reset_nid_postfix(writer) == 0, "reset failed"); + check(sceVideodec2DeleteDecoder_nid_postfix(writer) == 0, "delete failed"); + check(!query(shared).isValid, "a removed writer left its picture behind"); + } + } catch (...) { + done = true; + reader.join(); + throw; + } + done = true; + reader.join(); + check(failure.empty(), failure); + check(sceVideodec2DeleteDecoder_nid_postfix(keeper) == 0, "delete failed"); +} + } int main() { @@ -475,6 +642,8 @@ int main() { testClip(CodecHevc, 1, hevcLengthPrefixed, HevcHashes); testClip(CodecVp9, 0, ivfFrames(), Vp9Hashes); testTenBit(); + testPictureOwnership(); + testConcurrentOwnership(); std::puts("Videodec2 tests passed"); return 0; } catch (const std::exception& error) {