hrydgard / ppsspp

A PSP emulator for Android, Windows, Mac and Linux, written in C++. Want to contribute? Join us on Discord at https://discord.gg/5NJB6dD or just send pull requests / issues. For discussion use the forums at forums.ppsspp.org.
https://www.ppsspp.org
Other
11.12k stars 2.16k forks source link

Zero no Kiseki and Class of Heroes BGM cut off and hangup with the updated ffmpeg (GHA phase shifting) #5286

Closed daniel229 closed 9 years ago

daniel229 commented 10 years ago

happen in a specific scene. With the old ffmpeg,this does not happen.

debug log https://gist.github.com/daniel229/8728305

Affected games:

Others reported here: http://report.ppsspp.org/logs/kind/620

daniel229 commented 10 years ago

If pass the scene before the BGM cut off,It would not hang.

unknownbrackets commented 10 years ago

Did it log this ever before the update?

18:23:438 SOUND_THREAD I[ME]: HW\MediaEngine.cpp:84 FF: GHA Phase shifting
18:23:438 SOUND_THREAD I[ME]: HW\MediaEngine.cpp:84 FF:  is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented.

This sounds very similar to the Class of Heroes problem.

-[Unknown]

daniel229 commented 10 years ago

old one does not have that message

new one 01

old one 02

unknownbrackets commented 10 years ago

https://gitorious.org/libav/libav/commit/e6f0bb65270be51ea43345a28f8e4b1a728f7d0e line 1729. I was assuming this was a glitch but maybe the audio really does have this feature.

We could try extracting the audio files from the iso and playing with a recent build of ffmpeg. If it also reports that error, then it's probably something that ultimately needs to be solved within ffmpeg. If it never hits the error, we are munging the data somehow.

Either way, maybe we should detect AVERROR_PATCHWELCOME specifically and pretend it worked instead. Even silence is better than the entire game hanging.

-[Unknown]

daniel229 commented 10 years ago

The newest ffmpeg player also has that bug,but it is playing well,It would not stop the BGM. 05

unknownbrackets commented 10 years ago

What if you try changing (Core/HLE/sceAtrac.cpp):

                    avret = avcodec_decode_audio4(atrac->pCodecCtx, atrac->pFrame, &got_frame, &packet);
                    if (avret < 0) {
                        ERROR_LOG(ME, "avcodec_decode_audio4: Error decoding audio %d", avret);
                        atrac->failedDecode = true;
                        // No need to free the packet if decode_audio4 fails.
                        // Avoid getting stuck in a loop (Virtua Tennis)
                        *SamplesNum = 0;
                        *finish = 1;
                        *remains = 0;
                        return ATRAC_ERROR_ALL_DATA_DECODED;
                    }

To:

                    avret = avcodec_decode_audio4(atrac->pCodecCtx, atrac->pFrame, &got_frame, &packet);
                    if (avret == AVERROR_PATCHWELCOME) {
                        ERROR_LOG(ME, "Unsupported feature in ATRAC audio.");
                        // Let's try the next frame.
                    } else if (avret < 0) {
                        ERROR_LOG(ME, "avcodec_decode_audio4: Error decoding audio %08x", avret);
                        atrac->failedDecode = true;
                        // No need to free the packet if decode_audio4 fails.
                        // Avoid getting stuck in a loop (Virtua Tennis)
                        *SamplesNum = 0;
                        *finish = 1;
                        *remains = 0;
                        return ATRAC_ERROR_ALL_DATA_DECODED;
                    }

-[Unknown]

daniel229 commented 10 years ago

That's good,BGM can continue.Though the BGM would stuttering when hit that error like FFMPEG player,but It would not hang.

hrydgard commented 10 years ago

I have notified Maxim that this game exhibits this unimplemented ATRAC feature.

maximumspatium commented 10 years ago

I removed GHA Phase shifting feature from the final release because I could not test it due to lack of samples. Please extract broken audio files from these games and upload them somewhere so I can investigate a fix.

Thanks in advance Maxim

daniel229 commented 10 years ago

@maximumspatium I have sent you a link to the file,please check PM on the PPSSPP forum.

maximumspatium commented 10 years ago

@daniel229 Thanks! I'll look into it tomorrow...

unknownbrackets commented 10 years ago

@maximumspatium do you need more samples? We could probably collect logs and build a list of games that use these features. I noticed Unchained Blades also uses GHA phase shifting.

-[Unknown]

maximumspatium commented 10 years ago

@unknownbrackets yes, a couple of additional samples would help a lot! I've updated my ffmpeg branch to support the sample I've got from @daniel229. Because only few frames of this sample require GHA phase shifting feature, I would like to do exhaustive testing of my patch in order to ensure that it doesn't randomly crash before committing it to the upstream project.

Thanks in advance! Maxim

unknownbrackets commented 10 years ago

Okay, I've just now added reporting for that feature: http://report.ppsspp.org/logs/kind/620

Anything else we should report? Sometimes we get garbage data or errors that seem okay, so reporting every message from ffmpeg doesn't make a lot of sense.

-[Unknown]

daniel229 commented 10 years ago

I remenber 5 or 6 games using this feature in my games.

maximumspatium commented 10 years ago

@unknownbrackets @daniel229 I would very appreciate it if you could extract and upload somewhere broken ATRAC3+ media files for me to play with. I neither have time nor possibility to do this job myself.

Thank you in advance! Best regards Maxim

maximumspatium commented 10 years ago

@unknownbrackets if ffmpeg shows errors on specific ATRAC3+ media files, it's not OK to ignore them! They should be better reported to ffmpeg developers! Best regards Maxim

daniel229 commented 10 years ago

@maximumspatium I have send you a link to a file on the forum PM,some at3+ I don't know how to extract,some I have forget,so just one.

maximumspatium commented 10 years ago

@daniel229 Thank you! Best regards Maxim

unknownbrackets commented 10 years ago

if ffmpeg shows errors on specific ATRAC3+ media files, it's not OK to ignore them!

I mean errors like this:

FF: left block unavailable for requested intra4x4 mode -1 at 0 2
FF: error while decoding MB 0 2, bytestream (td)
FF: top block unavailable for requested intra4x4 mode -1 at 26 0
FF: error while decoding MB 26 0, bytestream (td)
FF: missing picture in access unit with size 2025
FF: no frame!
FF: error while decoding MB 16 13, bytestream (td)
FF: Cannot use next picture in error concealment

These aren't coming from ATRAC3+ files but I'm not sure if we can tell easily.

-[Unknown]

hrydgard commented 10 years ago

Such errors generally arise because we do some mismanagement of video buffers, causing a decode of bad data. I think a few games still show symptoms of this in the first frames of their intro movies.

daniel229 commented 10 years ago

@maximumspatium I found 7 samples,1from Mana Khemia Student Alliance,6 from World Neverland Qukria Kingdom,and these is a good sample called MIDHUS.AT3,It reports over 100 times Atrac3+: GHA phase shifting,please check forums PM.

daniel229 commented 10 years ago

Some BGM files are packed in a file.I don't know how to extract those files.

Sakimichi commented 10 years ago

@daniel229

Some BGM files are packed in a file.I don't know how to extract those files.

Shall I send the files/instructions on how to extract for Norn9 [ULJM06716]? Or shall I upload the BGM files instead?

daniel229 commented 10 years ago

You can send the files to @maximumspatium http://forums.ppsspp.org/member.php?action=profile&uid=14086 ,share instructions on how to extract for Norn9 also helps.

daniel229 commented 10 years ago

Idolmaster seems difference. it's GHA amplitude mode 0

22:43:865 MOV_DECODE_T I[ME]: HW\MediaEngine.cpp:85 FF: GHA amplitude mode 0
22:43:865 MOV_DECODE_T I[ME]: HW\MediaEngine.cpp:85 FF:  is not implemented. Update your FFmpeg version to the newest one from Git. If the problem sti
ll occurs, it means that your file has a feature which has not been implemented.
22:43:865 MOV_DECODE_T E[ME]: HW\SimpleAudioDec.cpp:201 Error decoding Audio frame (744 bytes): -1163346256 (baa8beb0)
22:43:865 MOV_DECODE_T E[ME]: HW\MediaEngine.cpp:809 Audio (AT3+) decode failed during video playback
daniel229 commented 10 years ago

GHA phase shifting causes Tenchu 4 random freeze.

daniel229 commented 10 years ago

Try to find all the problematic BGMs in the report list, but most of them are not near the new game beginning,so I can't find them.

problematic BGMs of these are extracd and sent to @maximumspatium by the PPSSPP forums PM.

BlazBlue Continuum Shift II
Chaos Head Love ChuChu!
conception trial
D.C.III.PLUS
Elminage Gothic Ulm Zakir to Yami no Gishiki
Glass Heart Princess Platinum
Gungnir
Harry Potter And The Half Blood Prince
Harry Potter and the Order of the_Phoenix
Jyuzaengi Engetsu Sangokuden 2
Kiyoka no Ori Hiiro no Kakera 4
Lego Star Wars III The Clone Wars
Memories Off Yubikiri no Kioku
Moeru Mahjong Moejong
norn9
Puzzle Quest Challenge of the Warlords
Sarugetchu SaruSaru Quest
Second Novel Kanojo no Natsu 15 fun no Kioku
snowy
Starry  Sky In Spring Portable
Suto Mani Strobe Mania
tenchu4
The King of Fighters Collection  The Orochi Saga
Weiss Schwarz White
Yummy Yummy Cooking Jam

these games happens during the intro video or logo,I don't know how to extra them.

Transformers 2
Final Armada
NHRA_Drag_Racing_Countdown_To_The_Championship
Despicable Me The Game
Samuraidou 2 Portable
LEGO Batman
Bounty Hound
Summon Night 3
Senjou no Valkyria 2
Marvel Ultimate Alliance 2
Split/Second and demo
NARUTO Shippuden: Ultimate Ninja Heroes 3 and demo

Puyo Puyo Fever reports GHA phase shifting though, but not report in ffplay.exe,So the error may be caused by media engine.

daniel229 commented 10 years ago

The BGMs that in the videoes have been extracted.

Transformers 2
Final Armada
Despicable Me The Game
Samuraidou 2 Portable
LEGO Batman
Summon Night 3
Senjou no Valkyria 2
Marvel Ultimate Alliance 2
Split/Second and demo
NARUTO Shippuden: Ultimate Ninja Heroes 3 and demo

NHRA Drag Racing Countdown To The Championship and Bounty Hound did not report errors in ffplay.exe. NHRA Drag Racing Countdown To The Championship report these errors in ppsspp https://gist.github.com/daniel229/a6db37de52a428522858

unknownbrackets commented 10 years ago

Yeah, those look like garbage data.

-[Unknown]

maximumspatium commented 9 years ago

Below a short update:

Old GHA Shifting code has been reenabled and tested on supplied samples. Thanks a lot!

The old code has been disabled due to decoder crashes during fuzzy tests. I found out that these crashes are being triggered by wrong logic in the GHA windowing. I use several shortcuts in order to avoid useless computations like multiplying with zeroes, calculating sine waves that will be thrown away later (reference decoder always calculates GHA windows 256 samples long and then use only a half of it). These techniques let me save a lot of computation power... Therefore, I need to fix the shortcut logic first. I'll come up with a final solution next week so you don't need your hacks (proposed somewhere in this forum) anymore...

Someone has mentioned earlier, that some samples report missing "Amplitude mode 0". IIRC, it's Idolmaster. Could someone extract them and PM me? I would like to fix it as well, so we can close these Atrac3+ story at last.

Thanks! Best regards Maxim

daniel229 commented 9 years ago

Good to hear the progress.I have get the sample from Idolmaster,that sample is extracted from a pmf file.I don't know if do something wrong,only noise playback with that sample.So I sent you the pmf file and the oma file.

unknownbrackets commented 9 years ago

Super cool, thanks again for your awesome work. I can't so much share samples, but if you need any help (I could run a bunch of files through a test program and give log output, or see what the psmf extraction problem is, etc.), let me know. More than happy to help in the ways that I'm able.

Also, let me know if there's any errors you'd like me to add to report collection (like Amplitude mode 0, etc.) If there's a way to identify all atrac3p decoder errors I'd be happy to report them all, I just don't want mpeg ones right now.

I'm sure our Android users appreciate your care for optimizational shortcuts.

-[Unknown]

hrydgard commented 9 years ago

Very cool Maxim! I hope daniel's sample will help.

maximumspatium commented 9 years ago

@daniel229

Your OMA seems to be broken. Unfortunately, I cannot neither verify nor use the original media file because FFMpeg cannot decode the audio stream from PSMF. How did you convert this PMF to OMA?

unknownbrackets commented 9 years ago

The audio stream is in pmf files as a private stream (0xBD.) I think someone mentioned a custom build of ffmpeg that could extract it. We currently use some demuxing code to pull out the data that we send to ffmpeg.

https://github.com/hrydgard/ppsspp/blob/master/Core/HW/MpegDemux.cpp#L119 https://github.com/hrydgard/ppsspp/blob/master/Core/HW/MpegDemux.cpp#L148

-[Unknown]

daniel229 commented 9 years ago

I used a tool PMF2PMP to extract the audio. Drag and drop the pmf file to the PMFAudioDumper.exe.

rename jpg to rar pmf2pmp

maximumspatium commented 9 years ago

@unknownbrackets @daniel229 Thank you for pointing me to the right code. I'm afraid we need a more serious solution for this demuxing/converting problem. From several sparse pieces of information scattered across the internet I know that PSMF is practically some extension of Mpeg2 with an additional header.

Recent FFMpeg can play back and convert video streams inside of PSMF. But audio doesn't work. Maybe it would be clever to extend ffmpeg to support Atrac audio inside of mpeg? I'd ready to undertake this task...

Various internet sources assume that the Mpeg stream always starts at file offset 2048. This could be dangerous because in your sample, daniel229, the Mpeg stream starts at offset 0x1800...

What's the purpose of this first PSMF header? Is it always safe to completely discard it?

Best regards Maxim

daniel229 commented 9 years ago

Some streams may starts at offset 0x800.That idolmaster pmf file contains two video streams. 01

maximumspatium commented 9 years ago

Sorry for being probably off-topic but I don't know what's the proper place for this kind of messages. I don't see any discussion forum around...

After some investigation it's now clear to me how Atrac3+ streams are stored inside of PRIVATE_STREAM_1 packets. It generally looks like that:

0x000001BD ; PRIVATE_STREAM_1 0x07EC ; packet length PES header: usually 8-11 bytes long four padding zeroes 0x0FD0 ; indicates somehow Atrac3+ stream 0x285C; Atrac3+ configuration (frame size, channel id, sample rate). four padding zeroes Raw Atrac3+ data follows

Moreover, the Atrac3+ configuration matches exactly the OMA container. PPSSPP code is rather cryptic here.

I noticed a strange fact: one PES packet seems to carry out four Atrac3+ frames. The data of the last frame is too short though. It seems to be cut off. Did you already see something like that? What do PPSSPP do in this case?

Best regards Maxim

unknownbrackets commented 9 years ago

We have a forum over at http://forums.ppsspp.org/, but we generally discuss dev here. We can create a new issue for PSMFs, but most likely people subscribed to this issue are interested in the discussion here.

Yes, there's a value in the header that indicates its length for psmf files. It's possibly safe to discard but has some information. That demux code is more than a little cryptic indeed.

The structure of a psmf file generally:

char magic[4] = "PSMF";
char version[4] = "0015"; // Other values have been observed.
u32_be header_size;
u32_be stream_size;
u8 unknown_or_padding[64]; // zeroes

u32_be stream_info_size; // This is the size of the following structure.

struct {
  u48_be stream_start_pts; // This is always 90000.  It's unclear if it's pts per second or start.
  u48_be stream_end_pts; // Note also that these are 48-bit, not 32-bit + 0 pad.

  // Below here I'm much less sure.

  u32_be unknown1; // Some say duration in ticks.
  u32_be unknown2; // Also 90000, some say mux rate.
  u8 num_streams; // Possibly, also at 0x80 as a 16-bit value...
  u8 unknown3; // Always 1?  Not sure.  Might be # of audio streams, definitely not video stream count.

  u32_be remaining_length; // Always 26 less than stream_info_size.

  u48_be stream_start_pts2; // Repeated?
  u48_be stream_end_pts2;

  u16_be unknown4; // Always 1?

  u32_be remaining_length2; // Always 44 less than stream_info_size.

  u16_be num_streams2; // Always matches num_streams as far as I know.

  // These are the video and audio streams.
  struct {
    u8 stream_id; // E0, E1, BD, etc.
    u8 private_stream_id; // 00, 01, etc.

    u8 unknown5; // Always 0x20?
    u8 unknown6; // FB, 55, 9F, 04... not sure.

    // Filled for video.
    u32_be ep_map_offset; // Optional (e.g. 0), from start of file.
    u32_be ep_map_count; // Optional (e.g. 0.)
    u8 stream_width_div_16;
    u8 stream_height_div_16;

    // Filled for audio.
    u8 num_channels;
    u8 frequency; // Not sure this is correct.  I've seen 2, we call it frequency in the code.
  } streams[num_streams2];
} stream_info;

// Following the stream info, the ep map begins.  This is an index.
struct {
  u8 ep_index; // This is what our code and JPCSP call it, but always C0?
  u8 pic_offset; // Don't know what this offsets into or how it can be one byte.
  u32_be pts; // Note only 32 bits.
  u32_be offset; // From start of file, in units of 2048 byte blocks.
} ep_entries[];

The EP index is basically (as I understand them) key frames.

I have to look at how we handle frames (it's cryptic to me too, most of the mpeg handling code was thrown together in a very large and very popular pull request that made videos work for the first time by hacking it together and referencing JPCSP, etc.). I thought we just concatenated the private stream data and then sent it to ffmpeg. It might be the next mpeg frame contains the other portion of that last atrac3+ frame?

-[Unknown]

maximumspatium commented 9 years ago

@unknownbrackets Thank you very much for the explanation!

Yes, after even more investigation I found out that Atrac packets seem to be always 2048 bytes long but Atrac frames are not always an integral of this size. For example, the size of one frame of the supplied sample is 744 bytes. The packet size is 2048 bytes, so only up to 2,75 Atrac frames can be transmitted inside of one packet. Therefore, the last frame will be split and transmitted as two parts in two different packets. The demuxer has to stitch fragmented frames together...

Fortunately, the bitstream already contains hints for stitching:

0x000001BD ; PRIVATE_STREAM_1 0x07EC ; packet length PES header: usually 8-11 bytes long -------------------- ATRAC data starts here ------------------------ 0x0X: audioStreamID, where "X" identifies one of the 16 Atrac streams 0x00; padding? 0xXXYY; numRemainingBytes, 0 - indicates full frame, anything other indicates number of bytes to be added to previous packet (partial frame) 0x0FD0 ; indicates somehow Atrac3+ stream 0x285C; Atrac3+ configuration (frame size, channel id, sample rate). 0x00000000; padding zeroes Raw Atrac3+ data follows

So as you can see the two fields - audioStreamID and numRemainingBytes - represent the hint for stitching frame data together. If a packet starts with a full frame, numRemainingBytes will contain zero. Otherwise, numRemainingBytes will contain number of bytes to be added to previous packet with the same audioStreamID. Packet data won't have the audio header starting with 0x0FD0 in this case but raw Atrac data will follow instead.

It looks like PPSSPP code just ignore the numRemainingBytes value: https://github.com/hrydgard/ppsspp/blob/master/Core/HW/MpegDemux.cpp#L133 This, surely, need to be fixed.

Moreover, a bunch of the code in MpegDemux.cpp looks like a duplication of FFmpeg/libavformat/mpeg.c, so I wonder if it would be a proper solution to completely delegate this demuxing task to FFMpeg?

Best regards Maxim

unknownbrackets commented 9 years ago

Nice investigation. Indeed looks like we should be handling that hint.

Hmm, well, there's some pros/cons to the demuxing. I've been thinking it's worth redoing that, which kinda loops into this discussion.

The way the PSP's sceMpeg api works (primary way of playing videos) is like this (C++-ized and simplified):

MpegRingbuffer *buf = new Ringbuffer();
Mpeg *mpeg = new Mpeg(buf);

int videoStreamNum = 0;
int audioStreamNum = 0;

// There's a ton of ways games time video.
// Sometimes by packet, sometimes by out of data, sometimes hardcoded.
int totalFrames = 1230;
int playedFrames = 0;
while (playedFrames < totalFrames) {
   int packetsToRead = 32;
   int packetsActuallyRead = mpeg->processPackets(packetsToRead, [&] {
      // It actually does use a callback for this.
      userPushPacketsOntoRingbuffer(buf);
   });

   if (mpeg->readVideo(videoStreamNum, &videoBuffer)) {
      playedFrames++;
      userDisplayVideoFrame(&videoBuffer);
   }
   if (mpeg->readAudio(audioStreamNum, &audioBuffer)) {
      userOutputAudioFrame(&audioBuffer);
   }
}
delete mpeg;
delete buf;

However, some games loop the video or do anything they want by providing frames into the ringbuffer in any order they like. We mostly emulate this, but it's a bit messy. I think we've mostly handled the discrepancies (it's not represented above, but some games can and do read multiple video streams from the file at once.)

Anyway, I definitely would not mind our code not doing this stuff, no need to reinvent complexity.

-[Unknown]

maximumspatium commented 9 years ago

I'm proud to announce a working PSMF audio support in FFMpeg, see this thread: https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2014-December/166169.html

Attached patch works for all PSMF files I own. Now you can extract and convert PSMF ATRAC3+ streams in anything you want :) No need for unreliable tools or hacks anymore. It will be surely the case once my patch has been merged by FFMpeg project...

Best regards Maxim

unknownbrackets commented 9 years ago

Cool, sounds great. Thanks for posting the update here, looking forward to seeing it merged.

-[Unknown]

daniel229 commented 9 years ago

Try to build the ffmpeg project,still fail,ffplay.exe and postpro-53.dll are not built.

maximumspatium commented 9 years ago

@daniel229 IIRC, ffplay requires SDL to be present. You can download it here: https://www.libsdl.org/ and compile it yourself. Maybe it's possible to find a precompiled library somewhere...

Best regards Maxim

daniel229 commented 9 years ago

Thanks, @maximumspatium ,i got ffplay compiled,still something wrong,it only play audio,crash on audio,I think i'd better to wait an offcial build.

maximumspatium commented 9 years ago

@daniel229 Sounds like something's missing - a library or a tool. Drop a mail to maximumspatium at googlemail dot com and describe how did you build ffmpeg. Maybe I'll be able to help...

Best regards Maxim

daniel229 commented 9 years ago

@maximumspatium Thank,I will redo it agains and send you the infomation about how I build ffmpeg tomorrow.