From 4079adf42487021cb6ca95e46db8d9ececcb7194 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 16 Sep 2026 21:32:36 -0600 Subject: [PATCH 1/6] sceVideocodec/sceMpegbase: fix the Media Engine frame descriptor mpeg.prx reads the eight frame buffer addresses straight off the front of the structure sceVideocodec publishes - `lw` at 0x00..0x1C, the same in Daxter's disc copy (1.3, at 08805698) and in flash0:/kd/mpeg.prx (1.8, at 08805898) - and takes the dimensions from its own context. We were writing the dimensions at 0x00/0x04 and the buffers at 0x10, so slots 0 and 1 received 17 and 30 and the four chroma addresses never arrived at all. That survived in Daxter only by cancelling out: mpeg.prx hands the same words back in the descriptor it builds for the colour conversion, which read them with the same skew. It bites as soon as they are used as real addresses. So also: - sceMpegBaseYCrCbCopy moves the frame, rather than copying 48 bytes of descriptor over the caller's table of destination pointers. mpegbase.prx builds a DMA list over the eight buffers (080010f8 in mpegbase_260.prx): flags bit 0 takes 0,1,4,5 and bit 1 takes 2,3,6,7. mpeg.prx always passes 3. - The chroma buffers are paired like the luma ones, left/right then even/odd rows, which is what that flag split assumes. Ours grouped them by half, so the per-buffer sizes disagreed with the caller's. - The conversion reads the descriptor as mpegbase.prx does, and takes the buffers from wherever they are: still in the Media Engine, or already copied into the game's memory. Co-Authored-By: Claude Opus 5 (1M context) --- Core/HLE/sceMpegbase.cpp | 115 +++++++++++++++++++++++++------------ Core/HLE/sceVideocodec.cpp | 29 +++++++--- Core/HLE/sceVideocodec.h | 8 +-- 3 files changed, 102 insertions(+), 50 deletions(-) diff --git a/Core/HLE/sceMpegbase.cpp b/Core/HLE/sceMpegbase.cpp index 0598a74345..c0fdb1893b 100644 --- a/Core/HLE/sceMpegbase.cpp +++ b/Core/HLE/sceMpegbase.cpp @@ -132,24 +132,31 @@ std::vector MpegBaseTakePESPacket(u32 dest) { // mpeg.prx hands the ME's decoded output to these to be converted to RGB. The descriptor it // passes is 48 bytes, which matches the range mpegbase.prx bounds-checks before using it. // -// The dimensions appear twice. Death Jr. has the plain macroblock counts in both pairs, but -// Thrillville's psmfplayer path has them shifted up by 8 in the first pair and plain in the -// second, so the second is the one to trust. The four luma buffers are at 0x20; the chroma ones -// aren't in here at all (see the recovery in MpegBaseCscRange). +// mpeg.prx builds this on its own stack right before each call (1.3 at 08805698, 1.8 at 08805898 - +// the two are the same shape), from the eight buffer addresses sceVideocodec published plus the +// dimensions out of its own context. mpegbase.prx reads exactly these fields: the dimensions at +// 0x00/0x04 and all eight buffers at 0x10..0x2c. +// +// The buffers can be in either place: the Media Engine's memory for a frame the decoder just +// produced, or the game's, once sceMpegBaseYCrCbCopy has moved one out. struct SceMp4AvcCscStruct { - s32_le scaledHeight; // 0x00 height << 8 on some paths, macroblocks on others - unused - s32_le scaledWidth; // 0x04 - s32_le mode0; // 0x08 - s32_le mode1; // 0x0c - s32_le height; // 0x10 in macroblocks - s32_le width; // 0x14 in macroblocks - s32_le unk18; // 0x18 - s32_le unk1c; // 0x1c - u32_le buffer[4]; // 0x20 luma only + s32_le height; // 0x00 in macroblocks + s32_le width; // 0x04 + s32_le unk08; // 0x08 + s32_le unk0c; // 0x0c + u32_le buffer[8]; // 0x10 four luma, then four chroma }; static_assert(sizeof(SceMp4AvcCscStruct) == 0x30); +// A frame buffer is either in the Media Engine's memory or, after a copy, in the game's. +static const u8 *MpegBaseFramePointer(u32 addr, int size) { + if (const u8 *me = VideocodecMEPointer(addr, size)) { + return me; + } + return Memory::GetTypedPointerRange(addr, size); +} + // The ME doesn't write plain planar YCbCr. The layout below was established by analysing // sceMpegBaseYCrCbCopy output on a real PSP (documented in JPCSP's sceVideocodec), and it uses // all eight buffers in the descriptor: @@ -163,8 +170,8 @@ static_assert(sizeof(SceMp4AvcCscStruct) == 0x30); // // Chroma is one (Cb,Cr) byte pair per 2x2 pixel square, so in chroma coordinates the bands are // 16 wide with 8-pixel halves, and the same even/odd split applies: -// buffer4: left half, even chroma rows buffer5: left half, odd chroma rows -// buffer6: right half, even chroma rows buffer7: right half, odd chroma rows +// buffer4: left half, even chroma rows buffer5: right half, even chroma rows +// buffer6: left half, odd chroma rows buffer7: right half, odd chroma rows // // Untangling it into plain planes costs one pass per frame, which keeps the conversion below // readable and is not where the time goes. @@ -178,20 +185,17 @@ static bool ReadTiledYCbCr(const u32 *buffers, int width, int height, const int *ySize = sizes; const int *cSize = sizes + 4; - // These are addresses in the Media Engine's memory, not in PSP RAM, so they resolve through - // sceVideocodec rather than through Memory::. A descriptor that was never filled in holds - // small integers instead, and those simply aren't in the ME's range. const u8 *y[4] = {}; const u8 *c[4] = {}; for (int i = 0; i < 4; i++) { if (ySize[i] > 0) { - y[i] = VideocodecMEPointer(buffers[i], ySize[i]); + y[i] = MpegBaseFramePointer(buffers[i], ySize[i]); if (!y[i]) { return false; } } if (cSize[i] > 0) { - c[i] = VideocodecMEPointer(buffers[4 + i], cSize[i]); + c[i] = MpegBaseFramePointer(buffers[4 + i], cSize[i]); if (!c[i]) { return false; } @@ -226,8 +230,8 @@ static bool ReadTiledYCbCr(const u32 *buffers, int width, int height, if (!c[b]) { continue; } - const int xOffset = (b >> 1) ? 8 : 0; - const int yStart = (b & 1) ? 1 : 0; + const int xOffset = (b & 1) ? 8 : 0; + const int yStart = (b >> 1) ? 1 : 0; int j = 0; for (int bandX = xOffset; bandX < width2; bandX += 16) { for (int row = yStart; row < height2; row += 2) { @@ -278,6 +282,10 @@ static int MpegBaseCscRange(u32 bufferRGB, u32 cscAddr, int bufferWidth, bufferWidth = g_mpegBaseBufferWidth; } + u32 buffers[8]{}; + for (int i = 0; i < 8; i++) { + buffers[i] = csc->buffer[i]; + } const int width = csc->width << 4; const int height = csc->height << 4; if (width <= 0 || height <= 0 || width > 1024 || height > 1024) { @@ -296,14 +304,6 @@ static int MpegBaseCscRange(u32 bufferRGB, u32 cscAddr, int bufferWidth, return hleLogError(Log::Mpeg, -1, "range outside the frame"); } - // The descriptor only carries the four luma buffers. Recover the full set from the allocation - // sceVideocodec handed out - both ends of that are ours. - u32 buffers[8]{}; - for (int i = 0; i < 4; i++) { - buffers[i] = csc->buffer[i]; - } - VideocodecGetFrameBuffers(csc->buffer[0], buffers); - std::vector luma, cb, cr; if (!ReadTiledYCbCr(buffers, width, height, luma, cb, cr)) { return hleLogError(Log::Mpeg, -1, "YCbCr buffers not readable"); @@ -372,7 +372,9 @@ static int sceMpegBaseCscAvc(u32 bufferRGB, u32 unknown, int bufferWidth, u32 cs if (!csc.IsValid()) { return hleLogError(Log::Mpeg, -1, "bad csc struct pointer"); } - return MpegBaseCscRange(bufferRGB, cscAddr, bufferWidth, 0, 0, csc->width << 4, csc->height << 4); + // The whole frame. MpegBaseCscRange clamps the range to the real frame size, which it gets + // from the allocation rather than from the descriptor, so ask for more than any frame can be. + return MpegBaseCscRange(bufferRGB, cscAddr, bufferWidth, 0, 0, 1024, 1024); } static u32 sceMpegBaseCscAvcRange(u32 bufferRGB, u32 unknown, u32 rangeAddr, int bufferWidth, u32 cscAddr) { @@ -387,16 +389,53 @@ static u32 sceMpegBaseCscAvcRange(u32 bufferRGB, u32 unknown, u32 rangeAddr, int return MpegBaseCscRange(bufferRGB, cscAddr, bufferWidth, rangeX, rangeY, rangeWidth, rangeHeight); } -// Copies one 48-byte YCrCb descriptor over another. The hardware copies the buffers themselves -// when told to; games seen so far only use it to move the descriptor around, so that is all we -// do until something needs more. +// Moves a decoded frame between two sets of buffers - both arguments are descriptors, and what +// gets copied is the pixels they point at, not the descriptors themselves. +// +// mpegbase.prx builds a DMA list over the eight buffers (080010f8 in mpegbase_260.prx): bit 0 of +// the flags selects buffers 0, 1, 4 and 5, bit 1 selects 2, 3, 6 and 7, and the per-buffer sizes +// are the same ones sceVideocodec lays its frame out with. mpeg.prx always passes 3, i.e. all +// eight. The source is the ME's own frame; the destination is wherever the caller wants it, which +// for psmfplayer is a slot in its output pool. static int sceMpegBaseYCrCbCopy(u32 dstAddr, u32 srcAddr, int flags) { - if (!Memory::IsValidRange(dstAddr, sizeof(SceMp4AvcCscStruct)) || - !Memory::IsValidRange(srcAddr, sizeof(SceMp4AvcCscStruct))) { + auto dst = PSPPointer::Create(dstAddr); + auto src = PSPPointer::Create(srcAddr); + if (!dst.IsValid() || !src.IsValid()) { return hleLogError(Log::Mpeg, -1, "bad descriptor pointer"); } - Memory::Memcpy(dstAddr, srcAddr, (u32)sizeof(SceMp4AvcCscStruct), "MpegBaseYCrCbCopy"); - return hleLogDebug(Log::Mpeg, 0, "flags %08x - descriptor copied, buffers left alone", flags); + + // Same story as the colour conversion: the frame size comes from the allocation the source + // buffers belong to, not from the descriptor's own dimension fields. + // mpeg.prx fills the destination's dimensions in macroblocks, the same way the colour + // conversion gets them; the source descriptor it builds on its stack states them in pixels. + const int width = dst->width << 4; + const int height = dst->height << 4; + if (width <= 0 || height <= 0 || width > 1024 || height > 1024) { + return hleLogError(Log::Mpeg, -1, "unreasonable frame size %dx%d", width, height); + } + int sizes[8]; + VideocodecFrameBufferLayout(width, height, sizes, nullptr); + + int copied = 0; + for (int i = 0; i < 8; i++) { + // Buffers 0,1,4,5 go with bit 0 and 2,3,6,7 with bit 1 - the even/odd row halves of luma + // and of chroma respectively. + const int bit = ((i & 3) < 2) ? 1 : 2; + if (!(flags & bit) || sizes[i] <= 0) { + continue; + } + const u8 *from = MpegBaseFramePointer(src->buffer[i], sizes[i]); + if (!from) { + return hleLogError(Log::Mpeg, -1, "source buffer %d not readable", i); + } + if (!Memory::IsValidRange(dst->buffer[i], sizes[i])) { + return hleLogError(Log::Mpeg, -1, "destination buffer %d (%08x, %d bytes) not writable", + i, (u32)dst->buffer[i], sizes[i]); + } + Memory::MemcpyUnchecked(dst->buffer[i], from, sizes[i]); + copied += sizes[i]; + } + return hleLogDebug(Log::Mpeg, 0, "flags %d, %dx%d, %d bytes", flags, width, height, copied); } const HLEFunction sceMpegbase[] = diff --git a/Core/HLE/sceVideocodec.cpp b/Core/HLE/sceVideocodec.cpp index 877b8c1635..bc403918a5 100644 --- a/Core/HLE/sceVideocodec.cpp +++ b/Core/HLE/sceVideocodec.cpp @@ -227,9 +227,13 @@ u32 VideocodecFrameBufferLayout(int width, int height, int sizes[8], u32 offsets // buffer0/2 take the odd band out when the width isn't a multiple of 32. const int lumaLeft = ((width + 16) >> 5) * (height >> 1) * 16; const int lumaRight = (width >> 5) * (height >> 1) * 16; + // Chroma is paired the same way luma is - left/right of a band, then even/odd rows - which is + // what sceMpegBaseYCrCbCopy's flags assume: bit 0 selects buffers 0,1,4,5 (the even rows) and + // bit 1 selects 2,3,6,7. Sizes have to line up with that, or a copy writes the wrong count + // into a buffer someone else sized. const int local[8] = { lumaLeft, lumaRight, lumaLeft, lumaRight, - lumaLeft >> 1, lumaLeft >> 1, lumaRight >> 1, lumaRight >> 1, + lumaLeft >> 1, lumaRight >> 1, lumaLeft >> 1, lumaRight >> 1, }; u32 total = 0; for (int i = 0; i < 8; i++) { @@ -278,13 +282,16 @@ static bool PublishFrameBuffers(VideocodecCtx &vctx, u32 structAddr, int width, buffers[i] = vctx.frameBuffers + offsets[i]; } - if (!Memory::IsValidRange(structAddr, 48)) { + // Eight addresses and nothing else. mpeg.prx reads them straight off the front of this + // structure - `lw` at 0x00..0x1C, verified in both 1.3 (Daxter's disc copy, at 08805698) and + // 1.8 (flash0:/kd/mpeg.prx, at 08805898) - and takes the frame's dimensions from its own + // context rather than from here. Writing anything else at the front lands in buffer slots 0 + // and 1, which sceMpegBaseYCrCbCopy then DMAs to. + if (!Memory::IsValidRange(structAddr, 8 * 4)) { return false; } - Memory::WriteUnchecked_U32(height >> 4, structAddr + 0); // macroblocks - Memory::WriteUnchecked_U32(width >> 4, structAddr + 4); for (int i = 0; i < 8; i++) { - Memory::WriteUnchecked_U32(buffers[i], structAddr + 16 + i * 4); + Memory::WriteUnchecked_U32(buffers[i], structAddr + i * 4); } return true; } @@ -308,7 +315,7 @@ void VideocodecGetCtxInfo(std::vector *infos) { } } -bool VideocodecGetFrameBuffers(u32 firstBuffer, u32 buffers[8]) { +bool VideocodecGetFrameBuffers(u32 firstBuffer, u32 buffers[8], int *width, int *height) { // sceMpegbase only has the first of the eight addresses, so find whose allocation it is. const VideocodecCtx *found = nullptr; for (const auto &[addr, ctx] : g_videocodecCtxs) { @@ -322,6 +329,12 @@ bool VideocodecGetFrameBuffers(u32 firstBuffer, u32 buffers[8]) { } u32 offsets[8]; VideocodecFrameBufferLayout(found->frameBufferWidth, found->frameBufferHeight, nullptr, offsets); + if (width) { + *width = found->frameBufferWidth; + } + if (height) { + *height = found->frameBufferHeight; + } for (int i = 0; i < 8; i++) { buffers[i] = found->frameBuffers + offsets[i]; } @@ -381,8 +394,8 @@ static void WriteTiledYCbCr(const u32 *buffers, const AvcDecoder &dec, int width if (!dst) { continue; } - const int xOffset = (b >> 1) ? 8 : 0; - const int yStart = (b & 1) ? 1 : 0; + const int xOffset = (b & 1) ? 8 : 0; + const int yStart = (b >> 1) ? 1 : 0; int j = 0; for (int bandX = xOffset; bandX < width2; bandX += 16) { for (int row = yStart; row < height2; row += 2) { diff --git a/Core/HLE/sceVideocodec.h b/Core/HLE/sceVideocodec.h index 0bb19e3216..d6cbb1f603 100644 --- a/Core/HLE/sceVideocodec.h +++ b/Core/HLE/sceVideocodec.h @@ -52,10 +52,10 @@ void VideocodecGetCtxInfo(std::vector *infos); // than through Memory:: - the ME's memory is not part of PSP RAM. u8 *VideocodecMEPointer(u32 addr, u32 size); -// mpeg.prx copies only the four luma buffers into the descriptor it hands sceMpegBaseCscAvc. -// Both ends of that are ours, so the conversion can recover the other four from the allocation -// they came from. Returns false if `firstBuffer` isn't one we handed out. -bool VideocodecGetFrameBuffers(u32 firstBuffer, u32 buffers[8]); +// The eight buffers of the frame starting at `firstBuffer`, and its size. Returns false if that +// isn't the start of an allocation we handed out - which is the normal answer once +// sceMpegBaseYCrCbCopy has moved a frame into the game's own memory. +bool VideocodecGetFrameBuffers(u32 firstBuffer, u32 buffers[8], int *width = nullptr, int *height = nullptr); // How the eight buffers a frame is delivered in are sized and laid out, in the order the // descriptor lists them: four luma (left/right half of a 32-pixel band, even/odd rows) then four From e784f8bccb7ee0806d1768c2795e0fd8ac4d4812 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 17 Sep 2026 09:53:03 -0600 Subject: [PATCH 2/6] sceMpegbase: delay the colour conversion, as the hardware does sceMpegBaseCscAvc/CscAvcRange run on the DMACPLUS and take real time. A psmfplayer game re-blits the current video frame every render frame while it waits for the next, so an instant return here is a tight loop that never yields and starves the audio thread the playback clock is paced by - the whole A/V pipeline then deadlocks a few frames into the movie. SOCOM: Tactical Strike hung exactly this way running the real mpeg.prx; with the delay it plays. Same value and reason as our sceMpeg HLE's sceMpegAvcCsc. Co-Authored-By: Claude Opus 5 (1M context) --- Core/HLE/sceMpegbase.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Core/HLE/sceMpegbase.cpp b/Core/HLE/sceMpegbase.cpp index c0fdb1893b..49b29c99b9 100644 --- a/Core/HLE/sceMpegbase.cpp +++ b/Core/HLE/sceMpegbase.cpp @@ -340,8 +340,14 @@ static int MpegBaseCscRange(u32 bufferRGB, u32 cscAddr, int bufferWidth, // last frame the GE drew - the same notification our sceMpegAvcCsc HLE does. The pixel mode // numbering matches GEBufferFormat, as it does there. gpu->PerformWriteFormattedFromMemory(bufferRGB, destSize, bufferWidth, (GEBufferFormat)g_mpegBasePixelMode); - return hleLogDebug(Log::Mpeg, 0, "%dx%d at %d,%d -> %08x stride %d", - rangeWidth, rangeHeight, rangeX, rangeY, bufferRGB, bufferWidth); + // This runs on the DMACPLUS hardware and takes real time, and a caller running the real + // mpeg.prx leans on that. A psmfplayer game blits the current video frame every render frame + // while it waits for the next one to be ready, so returning instantly turns that into a tight + // loop that never yields and starves the audio thread - which is what paces playback - so the + // whole A/V pipeline deadlocks a few frames in. (SOCOM: Tactical Strike hangs exactly here.) + // Our sceMpeg HLE delays the equivalent sceMpegAvcCsc by the same amount for the same reason. + return hleDelayResult(hleLogDebug(Log::Mpeg, 0, "%dx%d at %d,%d -> %08x stride %d", + rangeWidth, rangeHeight, rangeX, rangeY, bufferRGB, bufferWidth), "mpegbase csc", 4000); } static int sceMpegBaseCscInit(int bufferWidth) { From 45727ef2beccbb25e681c4ad55db1ddda7f4410c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 17 Sep 2026 10:09:48 -0600 Subject: [PATCH 3/6] Tighten the comments on the mpeg PRX changes Cut restatement and asides that only made sense against earlier, wrong versions of the code, and prefer parentheses over paired dashes. Also fix two comments left stale by the descriptor rework, and record the rule in AGENTS.md. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 11 +++++++ Core/HLE/sceMpegbase.cpp | 57 ++++++++++++++++-------------------- Core/HLE/sceVideocodec.cpp | 22 +++++++------- Core/HLE/sceVideocodec.h | 4 +-- Core/Util/BlockAllocator.cpp | 5 ++-- 5 files changed, 51 insertions(+), 48 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 59aa1b1804..51a5f65db1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -252,6 +252,17 @@ private: But generally follow the surrounding style. Braces are preferred on the same line. Braces are always used even when they could be omitted due the inner part being just a single line. +### Comments + +Keep comments tight. Say the thing once; don't restate what the code already shows, and don't +allude to a previous, now-corrected version ("eight addresses and nothing else" - the "and nothing +else" only makes sense against the old wrong layout, which is history). Cut filler like "the two +are the same shape" down to "(the same shape)". + +For a parenthetical aside, prefer parentheses over a pair of spaced dashes: write "the audio thread +that paces playback", or "the audio thread (which paces playback)", not "the audio thread - which +paces playback -". A single trailing dash to tack on an example is fine. + We've been inconsistent with copyright notices, but for new files, have the year at 2012, and add the "This program is free software..." as in other files. `// Copyright (c) 2012- PPSSPP Project.` diff --git a/Core/HLE/sceMpegbase.cpp b/Core/HLE/sceMpegbase.cpp index 49b29c99b9..d07d3b0ab1 100644 --- a/Core/HLE/sceMpegbase.cpp +++ b/Core/HLE/sceMpegbase.cpp @@ -132,13 +132,12 @@ std::vector MpegBaseTakePESPacket(u32 dest) { // mpeg.prx hands the ME's decoded output to these to be converted to RGB. The descriptor it // passes is 48 bytes, which matches the range mpegbase.prx bounds-checks before using it. // -// mpeg.prx builds this on its own stack right before each call (1.3 at 08805698, 1.8 at 08805898 - -// the two are the same shape), from the eight buffer addresses sceVideocodec published plus the -// dimensions out of its own context. mpegbase.prx reads exactly these fields: the dimensions at -// 0x00/0x04 and all eight buffers at 0x10..0x2c. +// mpeg.prx builds this on its own stack before each call (1.3 at 08805698, 1.8 at 08805898, the +// same shape) from the eight buffer addresses sceVideocodec published and the dimensions from its +// own context. mpegbase.prx reads the dimensions at 0x00/0x04 and the eight buffers at 0x10..0x2c. // -// The buffers can be in either place: the Media Engine's memory for a frame the decoder just -// produced, or the game's, once sceMpegBaseYCrCbCopy has moved one out. +// The buffers live either in Media Engine memory (a freshly decoded frame) or in the game's +// memory (once sceMpegBaseYCrCbCopy has moved one out). struct SceMp4AvcCscStruct { s32_le height; // 0x00 in macroblocks s32_le width; // 0x04 @@ -335,17 +334,16 @@ static int MpegBaseCscRange(u32 bufferRGB, u32 cscAddr, int bufferWidth, } } NotifyMemInfo(MemBlockFlags::WRITE, bufferRGB, destSize, "MpegBaseCsc"); - // The CPU just wrote a video frame straight into what is usually a display buffer. The hardware - // backends don't see that on their own, so without telling them, the screen keeps showing the - // last frame the GE drew - the same notification our sceMpegAvcCsc HLE does. The pixel mode - // numbering matches GEBufferFormat, as it does there. + // The CPU just wrote a video frame into what is usually a display buffer. The hardware backends + // don't see that on their own, so without telling them the screen keeps showing the last frame + // the GE drew (the same notification our sceMpegAvcCsc HLE does). The pixel mode numbering + // matches GEBufferFormat, as it does there. gpu->PerformWriteFormattedFromMemory(bufferRGB, destSize, bufferWidth, (GEBufferFormat)g_mpegBasePixelMode); - // This runs on the DMACPLUS hardware and takes real time, and a caller running the real - // mpeg.prx leans on that. A psmfplayer game blits the current video frame every render frame - // while it waits for the next one to be ready, so returning instantly turns that into a tight - // loop that never yields and starves the audio thread - which is what paces playback - so the - // whole A/V pipeline deadlocks a few frames in. (SOCOM: Tactical Strike hangs exactly here.) - // Our sceMpeg HLE delays the equivalent sceMpegAvcCsc by the same amount for the same reason. + // This runs on the DMACPLUS and takes real time. A psmfplayer game blits the current video + // frame every render frame while it waits for the next, so an instant return is a tight loop + // that never yields and starves the audio thread that paces playback, and the A/V pipeline + // deadlocks a few frames in (SOCOM: Tactical Strike hangs exactly here). Our sceMpeg HLE delays + // sceMpegAvcCsc the same way. return hleDelayResult(hleLogDebug(Log::Mpeg, 0, "%dx%d at %d,%d -> %08x stride %d", rangeWidth, rangeHeight, rangeX, rangeY, bufferRGB, bufferWidth), "mpegbase csc", 4000); } @@ -378,8 +376,8 @@ static int sceMpegBaseCscAvc(u32 bufferRGB, u32 unknown, int bufferWidth, u32 cs if (!csc.IsValid()) { return hleLogError(Log::Mpeg, -1, "bad csc struct pointer"); } - // The whole frame. MpegBaseCscRange clamps the range to the real frame size, which it gets - // from the allocation rather than from the descriptor, so ask for more than any frame can be. + // The whole frame. MpegBaseCscRange clamps to the real frame size (from the allocation, not the + // descriptor), so pass more than any frame can be. return MpegBaseCscRange(bufferRGB, cscAddr, bufferWidth, 0, 0, 1024, 1024); } @@ -395,14 +393,13 @@ static u32 sceMpegBaseCscAvcRange(u32 bufferRGB, u32 unknown, u32 rangeAddr, int return MpegBaseCscRange(bufferRGB, cscAddr, bufferWidth, rangeX, rangeY, rangeWidth, rangeHeight); } -// Moves a decoded frame between two sets of buffers - both arguments are descriptors, and what -// gets copied is the pixels they point at, not the descriptors themselves. +// Moves a decoded frame between two sets of buffers. Both arguments are descriptors; what gets +// copied is the pixels they point at, not the descriptors. // -// mpegbase.prx builds a DMA list over the eight buffers (080010f8 in mpegbase_260.prx): bit 0 of -// the flags selects buffers 0, 1, 4 and 5, bit 1 selects 2, 3, 6 and 7, and the per-buffer sizes -// are the same ones sceVideocodec lays its frame out with. mpeg.prx always passes 3, i.e. all -// eight. The source is the ME's own frame; the destination is wherever the caller wants it, which -// for psmfplayer is a slot in its output pool. +// mpegbase.prx builds a DMA list over the eight buffers (080010f8 in mpegbase_260.prx): flag bit 0 +// selects buffers 0,1,4,5 and bit 1 selects 2,3,6,7, with the per-buffer sizes sceVideocodec lays +// its frame out with. mpeg.prx always passes 3 (all eight). The source is the ME's frame; the +// destination is wherever the caller wants it (for psmfplayer, a slot in its output pool). static int sceMpegBaseYCrCbCopy(u32 dstAddr, u32 srcAddr, int flags) { auto dst = PSPPointer::Create(dstAddr); auto src = PSPPointer::Create(srcAddr); @@ -410,10 +407,8 @@ static int sceMpegBaseYCrCbCopy(u32 dstAddr, u32 srcAddr, int flags) { return hleLogError(Log::Mpeg, -1, "bad descriptor pointer"); } - // Same story as the colour conversion: the frame size comes from the allocation the source - // buffers belong to, not from the descriptor's own dimension fields. - // mpeg.prx fills the destination's dimensions in macroblocks, the same way the colour - // conversion gets them; the source descriptor it builds on its stack states them in pixels. + // The destination descriptor carries the dimensions in macroblocks (as the colour conversion's + // does); the stack-built source descriptor states them in pixels, so read the destination. const int width = dst->width << 4; const int height = dst->height << 4; if (width <= 0 || height <= 0 || width > 1024 || height > 1024) { @@ -424,8 +419,8 @@ static int sceMpegBaseYCrCbCopy(u32 dstAddr, u32 srcAddr, int flags) { int copied = 0; for (int i = 0; i < 8; i++) { - // Buffers 0,1,4,5 go with bit 0 and 2,3,6,7 with bit 1 - the even/odd row halves of luma - // and of chroma respectively. + // Buffers 0,1,4,5 go with bit 0 and 2,3,6,7 with bit 1 (the even and odd row halves of luma + // and chroma). const int bit = ((i & 3) < 2) ? 1 : 2; if (!(flags & bit) || sizes[i] <= 0) { continue; diff --git a/Core/HLE/sceVideocodec.cpp b/Core/HLE/sceVideocodec.cpp index bc403918a5..faf1e70661 100644 --- a/Core/HLE/sceVideocodec.cpp +++ b/Core/HLE/sceVideocodec.cpp @@ -227,10 +227,10 @@ u32 VideocodecFrameBufferLayout(int width, int height, int sizes[8], u32 offsets // buffer0/2 take the odd band out when the width isn't a multiple of 32. const int lumaLeft = ((width + 16) >> 5) * (height >> 1) * 16; const int lumaRight = (width >> 5) * (height >> 1) * 16; - // Chroma is paired the same way luma is - left/right of a band, then even/odd rows - which is - // what sceMpegBaseYCrCbCopy's flags assume: bit 0 selects buffers 0,1,4,5 (the even rows) and - // bit 1 selects 2,3,6,7. Sizes have to line up with that, or a copy writes the wrong count - // into a buffer someone else sized. + // Chroma is paired like luma (left/right of a band, then even/odd rows), which is what + // sceMpegBaseYCrCbCopy's flags assume: bit 0 selects buffers 0,1,4,5 and bit 1 selects 2,3,6,7. + // The sizes have to match that, or a copy writes the wrong count into a buffer someone else + // sized. const int local[8] = { lumaLeft, lumaRight, lumaLeft, lumaRight, lumaLeft >> 1, lumaRight >> 1, lumaLeft >> 1, lumaRight >> 1, @@ -248,9 +248,8 @@ u32 VideocodecFrameBufferLayout(int width, int height, int sizes[8], u32 offsets return total; } -// The descriptor mpeg.prx passes in is empty: on hardware the ME owns the frame buffers, and -// reports where it put them. So allocate them here and fill the descriptor in the shape -// sceMpegBaseCscAvc expects - dimensions in macroblocks, then the eight buffer addresses. +// The descriptor mpeg.prx passes in is empty: on hardware the ME owns the frame buffers and +// reports where it put them. So allocate them here and fill in the eight buffer addresses. static bool PublishFrameBuffers(VideocodecCtx &vctx, u32 structAddr, int width, int height, u32 buffers[8]) { u32 offsets[8]; const u32 total = VideocodecFrameBufferLayout(width, height, nullptr, offsets); @@ -282,11 +281,10 @@ static bool PublishFrameBuffers(VideocodecCtx &vctx, u32 structAddr, int width, buffers[i] = vctx.frameBuffers + offsets[i]; } - // Eight addresses and nothing else. mpeg.prx reads them straight off the front of this - // structure - `lw` at 0x00..0x1C, verified in both 1.3 (Daxter's disc copy, at 08805698) and - // 1.8 (flash0:/kd/mpeg.prx, at 08805898) - and takes the frame's dimensions from its own - // context rather than from here. Writing anything else at the front lands in buffer slots 0 - // and 1, which sceMpegBaseYCrCbCopy then DMAs to. + // mpeg.prx reads eight buffer addresses off the front of this structure (`lw` at 0x00..0x1C, + // verified in 1.3 at 08805698 and 1.8 at 08805898) and takes the frame dimensions from its own + // context. So write only the addresses; anything else at the front lands in slots 0 and 1, + // which sceMpegBaseYCrCbCopy then DMAs to. if (!Memory::IsValidRange(structAddr, 8 * 4)) { return false; } diff --git a/Core/HLE/sceVideocodec.h b/Core/HLE/sceVideocodec.h index d6cbb1f603..d46b1bcce1 100644 --- a/Core/HLE/sceVideocodec.h +++ b/Core/HLE/sceVideocodec.h @@ -53,8 +53,8 @@ void VideocodecGetCtxInfo(std::vector *infos); u8 *VideocodecMEPointer(u32 addr, u32 size); // The eight buffers of the frame starting at `firstBuffer`, and its size. Returns false if that -// isn't the start of an allocation we handed out - which is the normal answer once -// sceMpegBaseYCrCbCopy has moved a frame into the game's own memory. +// isn't the start of an allocation we handed out (the normal answer once sceMpegBaseYCrCbCopy has +// moved a frame into the game's memory). bool VideocodecGetFrameBuffers(u32 firstBuffer, u32 buffers[8], int *width = nullptr, int *height = nullptr); // How the eight buffers a frame is delivered in are sized and laid out, in the order the diff --git a/Core/Util/BlockAllocator.cpp b/Core/Util/BlockAllocator.cpp index f0aa16bf80..e96d138315 100644 --- a/Core/Util/BlockAllocator.cpp +++ b/Core/Util/BlockAllocator.cpp @@ -451,9 +451,8 @@ void BlockAllocator::DoState(PointerWrap &p) const bool compact = s >= 2; int count = 0; - // An allocator that was never Init'd (or has been Shutdown) has no blocks at all. That's a - // perfectly good state to save - it's what one that nothing has asked for memory from yet - // looks like - so zero blocks is a normal count here, not a corrupt one. + // An allocator that was never Init'd (or has been Shutdown) has no blocks, which is a valid + // state to save, so a zero block count here is normal rather than corrupt. if (p.mode == p.MODE_READ) { Shutdown(); From b561de3ff1031814f702f5bd9a02f859c75eb8d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 17 Sep 2026 10:25:57 -0600 Subject: [PATCH 4/6] sceAudiocodec: size the AAC input frame like the firmware The AAC decode path read the frame size from srcBytesRead, which is an output field the decoder writes - it's 0 on the first call, so the decoder was handed 0 readable bytes and audio never started. avcodec.prx's decodeUtility sizes the AAC input from the byte at 0x2c instead: 0x609 when nonzero, 0x600 when zero. Investigated after report by sum2012 in #16238. Not actually tested yet. Co-Authored-By: Claude Opus 5 (1M context) --- Core/HLE/sceAudiocodec.cpp | 14 ++++++++------ Core/HLE/sceAudiocodec.h | 9 +++++---- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/Core/HLE/sceAudiocodec.cpp b/Core/HLE/sceAudiocodec.cpp index 5306340605..1e7809f4e3 100644 --- a/Core/HLE/sceAudiocodec.cpp +++ b/Core/HLE/sceAudiocodec.cpp @@ -96,11 +96,9 @@ static_assert(offsetof(SceAudiocodecCodec, allocMem) == 0x68); // AAC (0x1003) // ------------------------------------------------ -// Sample rate is at offset 0x28. -// srcBytesConsumed can be very small the first frames. -// 0x1000 is always the frame size. -// The firmware sizes AAC from the bytes at 0x2c and 0x2d rather than from 0x28: input is -// 0x600 or 0x609, output 0x1000 or 0x2000, depending on those two. Consistent with the above. +// Sample rate is at offset 0x28. The input frame size is 0x609 when the byte at 0x2c is nonzero +// and 0x600 when it is zero (avcodec.prx decodeUtility, case 0x1003); the output size comes from +// the byte at 0x2d and is 0x1000 or 0x2000. // MP3 (0x1002) // ------------------------------------------------ @@ -342,7 +340,11 @@ static int sceAudiocodecDecode(u32 ctxPtr, int codec) { sampleRate = Mp3SampleRateFromContext(ctx); break; case PSP_CODEC_AAC: - bytesPerFrame = ctx->srcBytesRead; + // avcodec.prx sizes the AAC input frame from the byte at 0x2c: 0x609 when it is nonzero, + // 0x600 when it is zero (decodeUtility, case 0x1003). srcBytesRead, which this used to read, + // is an output field and is 0 on the first call, so the decoder got nothing to read and + // audio never started (seen in Meururu no Atelier Plus running the real mpeg.prx). + bytesPerFrame = ctx->fmt.aac.unk2c ? 0x609 : 0x600; sampleRate = ctx->fmt.aac.sampleRate; break; case PSP_CODEC_AT3: diff --git a/Core/HLE/sceAudiocodec.h b/Core/HLE/sceAudiocodec.h index 1da7067f31..e298cba0b7 100644 --- a/Core/HLE/sceAudiocodec.h +++ b/Core/HLE/sceAudiocodec.h @@ -100,12 +100,13 @@ struct SceAudiocodecCodec { s32 unk64; // 0x64 } mp3; - // AAC. The hardware sizes its input from the byte at 0x2c and its output from the byte - // at 0x2d; the sample rate at 0x28 is our own observation from games. + // AAC. The input frame size is 0x609 when unk2c is nonzero and 0x600 when it is zero, and + // the output size comes from unk2d (avcodec.prx decodeUtility); the sample rate at 0x28 is + // our own observation from games. struct { u32 sampleRate; // 0x28 - u8 unk2c; // 0x2c - u8 unk2d; // 0x2d + u8 unk2c; // 0x2c selects the input frame size (see above) + u8 unk2d; // 0x2d selects the output size u8 unk2e; // 0x2e u8 unk2f; // 0x2f s32 unk30[14]; // 0x30..0x67 From ba7bf86e73bd33b60ff306db2a84ceae31d87d65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 17 Sep 2026 10:45:16 -0600 Subject: [PATCH 5/6] sceAudiocodec: don't return a zero Atrac3+ frame size for odd bitrates CalculateInputBytesAndChannelsAt3Plus only set the frame size for the four bitrates it had a table entry for and left it at 0 otherwise, which fails the decode - the same shape of bug the AAC path just had. formatByte2 * 8 + 8 is the size for all four known bitrates, so use it for the rest too; the firmware never returns 0 here. The common PSMF path is unaffected: it takes the size from the frame's own header before reaching this. Co-Authored-By: Claude Opus 5 (1M context) --- Core/HLE/sceAudiocodec.cpp | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/Core/HLE/sceAudiocodec.cpp b/Core/HLE/sceAudiocodec.cpp index 1e7809f4e3..1950e09821 100644 --- a/Core/HLE/sceAudiocodec.cpp +++ b/Core/HLE/sceAudiocodec.cpp @@ -139,24 +139,12 @@ void CalculateInputBytesAndChannelsAt3Plus(const SceAudiocodecCodec *ctx, int *i return; } - int size = formatByte2 * 8 + 8; - // No idea if this is accurate, this is just a guess... - if (formatByte1 & 8) { - *channels = 2; - } else { - *channels = 1; - } - switch (size) { - case 0x118: - case 0x178: - case 0x230: - case 0x2E8: - // These have been seen before, let's return it. - *inputBytes = size; - return; - default: - break; - } + // bit 3 of the first byte is the channel count. This is a guess, but it fits the data we have. + *channels = (formatByte1 & 8) ? 2 : 1; + // formatByte2 * 8 + 8 gives the frame size for every bitrate we have data for (0x118, 0x178, + // 0x230, 0x2E8), so use it for any other value as well rather than leaving inputBytes at 0, + // which the firmware never does and which would fail the decode outright. + *inputBytes = formatByte2 * 8 + 8; } // Atrac3 (0x1001). Unlike Atrac3+, the context doesn't carry a frame size - libatrac3plus.prx From c596bdf9a488eb24acbffcef31f2051ddf1c710b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 17 Sep 2026 11:01:09 -0600 Subject: [PATCH 6/6] sceAudiocodec/sceVideocodec: fill an AAC gap and name two ME functions sceAudiocodecCheckNeedMem set neededMem for every codec except AAC, where it was left at whatever was in the context. Set it to 0x18f20 like the others; our faked ME memory meant this never blocked, but a game that validates it could. Also name the two hash-named sceVideocodec exports from their firmware behaviour: 0x893B32B1 configures the codec during sceMpegCreate in mode 1 (SetMode), and 0xD95C24D5 copies a decoded YCbCr frame between buffers via the ME (CopyYCbCr), the videocodec-level counterpart of sceMpegBaseYCrCbCopy. Both are still stubs - only mpeg.prx calls them, on paths nothing we run reaches - but they are now documented for whoever implements them. Co-Authored-By: Claude Opus 5 (1M context) --- Core/HLE/sceAudiocodec.cpp | 1 + Core/HLE/sceVideocodec.cpp | 15 +++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/Core/HLE/sceAudiocodec.cpp b/Core/HLE/sceAudiocodec.cpp index 1950e09821..29b0449b34 100644 --- a/Core/HLE/sceAudiocodec.cpp +++ b/Core/HLE/sceAudiocodec.cpp @@ -460,6 +460,7 @@ static int sceAudiocodecCheckNeedMem(u32 ctxPtr, int codec) { break; case 0x1003: // Kosmodrones uses sceAudiocodec directly (no intermediate library). + ctx->neededMem = 0x18f20; INFO_LOG(Log::ME, "CheckNeedMem for codec %04x: format %02x %02x", codec, ctx->fmt.at3.formatByte1, ctx->fmt.at3.formatByte2); break; } diff --git a/Core/HLE/sceVideocodec.cpp b/Core/HLE/sceVideocodec.cpp index faf1e70661..b9489ce120 100644 --- a/Core/HLE/sceVideocodec.cpp +++ b/Core/HLE/sceVideocodec.cpp @@ -635,11 +635,18 @@ static int sceVideocodecSetMemory(u32 ctxAddr, int type) { return hleLogDebug(Log::ME, 0); } -static int sceVideocodec_893B32B1(u32 ctxAddr, int type) { +// 0x893B32B1. mpeg.prx runs this from sceMpegCreate, only in mode 1 (the path where the game reads +// raw YCbCr out rather than letting sceMpegbase convert to RGB). On hardware it writes back the +// 0x28-byte output descriptor at ctx+0x10 and issues ME video op 0x6D68B223 to configure the codec. +// Nothing we run reaches it, so it stays a stub - implement it if a mode-1 game needs it. +static int sceVideocodecSetMode(u32 ctxAddr, int type) { return hleLogWarning(Log::ME, 0, "UNIMPL"); } -static int sceVideocodec_D95C24D5(u32 ctxAddr, int type) { +// 0xD95C24D5. Copies a decoded YCbCr frame between two sets of buffers through the ME (op +// 0x21521BE5) - the videocodec-level counterpart of sceMpegBaseYCrCbCopy. mpeg.prx calls it on the +// sceMpegAvcCopyYCbCr path. Nothing we run reaches it, so it stays a stub for now. +static int sceVideocodecCopyYCbCr(u32 ctxAddr, int type) { return hleLogWarning(Log::ME, 0, "UNIMPL"); } @@ -656,8 +663,8 @@ const HLEFunction sceVideocodec[] = { {0X17CF7D2C, &WrapI_UI, "sceVideocodecGetFrameCrop", 'i', "xi"}, {0X26927D19, &WrapI_UI, "sceVideocodecGetVersion", 'i', "xi"}, {0X627B7D42, &WrapI_UI, "sceVideocodecGetSEI", 'i', "xi"}, - {0X893B32B1, &WrapI_UI, "sceVideocodec_893B32B1", 'i', "xi"}, - {0XD95C24D5, &WrapI_UI, "sceVideocodec_D95C24D5", 'i', "xi"}, + {0X893B32B1, &WrapI_UI, "sceVideocodecSetMode", 'i', "xi"}, + {0XD95C24D5, &WrapI_UI, "sceVideocodecCopyYCbCr", 'i', "xi"}, }; void Register_sceVideocodec() {