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.41k stars 2.19k forks source link

ppsspp 1.14.4 does not build with FFmpeg 6.0 #15308

Open brad0 opened 2 years ago

brad0 commented 2 years ago

ppsspp 1.14.4 does not build with FFmpeg 6.0

/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.14.4/ppsspp-1.14.4/Core/AVIDump.cpp:148:10: error: assigning to 'AVCodec *' from 'const AVCodec *' discards qualifiers
        codec = avcodec_find_encoder(s_codec_context->codec_id);
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
brad0 commented 2 years ago

Looking at another project I was patching for FFmpeg I changed this line and it build Ok...

Index: Core/AVIDump.cpp
--- Core/AVIDump.cpp.orig
+++ Core/AVIDump.cpp
@@ -91,7 +91,7 @@ bool AVIDump::Start(int w, int h)

 bool AVIDump::CreateAVI() {
 #ifdef USE_FFMPEG
-       AVCodec *codec = nullptr;
+       const AVCodec *codec = nullptr;

        // Use gameID_EmulatedTimestamp for filename
        std::string discID = g_paramSFO.GetDiscID();
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:451:3: error: use of undeclared identifier 'avcodec_free_context'; did you mean 'avformat_free_context'?
                avcodec_free_context(&codecCtx_);
                ^~~~~~~~~~~~~~~~~~~~
                avformat_free_context
/usr/local/include/libavformat/avformat.h:1903:6: note: 'avformat_free_context' declared here
void avformat_free_context(AVFormatContext *s);
     ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:451:24: error: cannot initialize a parameter of type 'AVFormatContext *' with an rvalue of type 'AVCodecContext **'
                avcodec_free_context(&codecCtx_);
                                     ^~~~~~~~~~
/usr/local/include/libavformat/avformat.h:1903:45: note: passing argument to parameter 's' here
void avformat_free_context(AVFormatContext *s);
                                            ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:473:3: error: use of undeclared identifier 'avcodec_flush_buffers'
                avcodec_flush_buffers(codecCtx_);
                ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:498:4: error: use of undeclared identifier 'avcodec_flush_buffers'
                        avcodec_flush_buffers(codecCtx_);
                        ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:570:14: error: use of undeclared identifier 'avcodec_send_packet'
                        int err = avcodec_send_packet(codecCtx_, packet_);
                                  ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:578:13: error: use of undeclared identifier 'avcodec_receive_frame'; did you mean 'avcodec_profile_name'?
                int err = avcodec_receive_frame(codecCtx_, frame_);
                          ^~~~~~~~~~~~~~~~~~~~~
                          avcodec_profile_name
/usr/local/include/libavcodec/codec_id.h:615:13: note: 'avcodec_profile_name' declared here
const char *avcodec_profile_name(enum AVCodecID codec_id, int profile);
            ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:578:35: error: cannot initialize a parameter of type 'enum AVCodecID' with an lvalue of type 'AVCodecContext *'
                int err = avcodec_receive_frame(codecCtx_, frame_);
                                                ^~~~~~~~~
/usr/local/include/libavcodec/codec_id.h:615:49: note: passing argument to parameter 'codec_id' here
const char *avcodec_profile_name(enum AVCodecID codec_id, int profile);
                                                ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1810:20: error: member access into incomplete type 'AVCodecContext'
                        atrac->codecCtx_->sample_rate,
                                        ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HW/SimpleAudioDec.h:27:8: note: forward declaration of 'AVCodecContext'
struct AVCodecContext;
       ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1812:20: error: member access into incomplete type 'AVCodecContext'
                        atrac->codecCtx_->sample_fmt,
                                        ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HW/SimpleAudioDec.h:27:8: note: forward declaration of 'AVCodecContext'
struct AVCodecContext;
       ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1813:20: error: member access into incomplete type 'AVCodecContext'
                        atrac->codecCtx_->sample_rate,
                                        ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HW/SimpleAudioDec.h:27:8: note: forward declaration of 'AVCodecContext'
struct AVCodecContext;
       ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1843:21: error: use of undeclared identifier 'avcodec_alloc_context3'
        atrac->codecCtx_ = avcodec_alloc_context3(codec);
                           ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1847:19: error: member access into incomplete type 'AVCodecContext'
                atrac->codecCtx_->extradata = (uint8_t *)av_mallocz(14);
                                ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HW/SimpleAudioDec.h:27:8: note: forward declaration of 'AVCodecContext'
struct AVCodecContext;
       ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1848:19: error: member access into incomplete type 'AVCodecContext'
                atrac->codecCtx_->extradata_size = 14;
                                ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HW/SimpleAudioDec.h:27:8: note: forward declaration of 'AVCodecContext'
struct AVCodecContext;
       ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1852:19: error: member access into incomplete type 'AVCodecContext'
                atrac->codecCtx_->extradata[0] = 1;
                                ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HW/SimpleAudioDec.h:27:8: note: forward declaration of 'AVCodecContext'
struct AVCodecContext;
       ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1853:19: error: member access into incomplete type 'AVCodecContext'
                atrac->codecCtx_->extradata[3] = atrac->channels_ << 3;
                                ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HW/SimpleAudioDec.h:27:8: note: forward declaration of 'AVCodecContext'
struct AVCodecContext;
       ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1854:19: error: member access into incomplete type 'AVCodecContext'
                atrac->codecCtx_->extradata[6] = atrac->jointStereo_;
                                ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HW/SimpleAudioDec.h:27:8: note: forward declaration of 'AVCodecContext'
struct AVCodecContext;
       ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1855:19: error: member access into incomplete type 'AVCodecContext'
                atrac->codecCtx_->extradata[8] = atrac->jointStereo_;
                                ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HW/SimpleAudioDec.h:27:8: note: forward declaration of 'AVCodecContext'
struct AVCodecContext;
       ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1856:19: error: member access into incomplete type 'AVCodecContext'
                atrac->codecCtx_->extradata[10] = 1
                                ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HW/SimpleAudioDec.h:27:8: note: forward declaration of 'AVCodecContext'
struct AVCodecContext;
       ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HLE/sceAtrac.cpp:1861:19: error: member access into incomplete type 'AVCodecContext'
                atrac->codecCtx_->channels = 1;
                                ^
/home/brad/tmp/ffmpeg-ports/ports/pobj/ppsspp-1.12.3/ppsspp-1.12.3/Core/HW/SimpleAudioDec.h:27:8: note: forward declaration of 'AVCodecContext'
struct AVCodecContext;
       ^
unknownbrackets commented 2 years ago

The const change seems fine if you want to send a pull request.

For the other errors, it seems like some avcodec header was not included?

-[Unknown]

brad0 commented 2 years ago

The const change seems fine if you want to send a pull request.

For the other errors, it seems like some avcodec header was not included?

FFmpeg 5.0 dropped a bunch of deprecated API.

LoneFox78 commented 2 years ago

Looks like there is only one instance of removed API being used, namely this: https://github.com/hrydgard/ppsspp/blob/f58d4dfcfe414f8b0d411f3dc7009141ecf37481/Core/HW/MediaEngine.cpp#L437 I don't know how to fix that. Commenting it out makes the project build, but then video playback is broken.

Rest of the errors can be fixed by adding const and #include "libavcodec/avcodec.h" to appropriate places.

unknownbrackets commented 2 years ago

Right, the basic problem is this: FFmpeg wants you to provide a video file, and it will discover the streams in it and play it.

PSP games, however, normally call an API to tell the PSP video playback routines about streams. A careful observer may notice that this means, even if the video header is incomplete, the PSP can still play the video. The information is in the "wrong" place. PSP games can also dynamically loop and reorder packets in the data they read, because the PSP video playback routines allow it.

FFmpeg would tell you that the PSP games were coded wrong. Surely their solution would be to rewrite each of the 1400+ PSP games to handle video playback in a more modern way, reading the headers the way FFmpeg believes is correct and allowing FFmpeg to discover all streams from the video file headers.

Unfortunately, that's a different project. This is an emulator, not a collection of 1,400 projects to individually port PSP games to PC and to Android. We have the same issues with graphics rendering etc., but we use APIs that allow us to simulate and handle the things PSP games did.

Here we really have a few options:

1 - Keeping using the older FFmpeg, which still allows us to add streams in the way PSP games were coded, indefinitely.

2 - Use the new FFmpeg, delete the code for stream addition, and find some hacky way to make 70-85% of games work correctly, with game-specific hacks for a few popular games. This would lower overall compatibility and accuracy, but the people who want to use system FFmpeg would all celebrate.

3 - Write our own, from scratch, video parsing, decoding, etc. routines to more accurately mimic the PSP. In fact, there are issues with FFmpeg, and it's definitely not bit-accurate to what the PSP decoders produced. This would result, after many years of work probably, in greater accuracy and compatibility.

4 - Take the old FFmpeg (or perhaps the new, and reverse the deprecations) and rename it to "PPmpeg". Then proceed with modifying it to more accurately mimic the PSP. This is probably functionally identical to the previous, but would take significantly less time - although might still be years of work. But I suppose people who want to use system FFmpeg would be more angry and insulted that we didn't spend years rewriting FFmpeg from scratch.

Options 3 and 4 represent quite a lot of work (especially 3), and option 2 looks very disappointing. That's why we've stuck with option 1.

-[Unknown]

Johnnynator commented 1 year ago

The need_parsing is only moved to the internal api, so there are still two ways of using it, directly accessing the struct field that has it or using the avpriv_stream_set_need_parsing function. While both ways are imo kinda doable things when statically linking against ffmpeg, I dislike doing it this way for system libraries.

Following patch did work just fine in a quick test (MHFU).

diff --git a/Core/HW/MediaEngine.cpp b/Core/HW/MediaEngine.cpp
index 8e6870f0b..d602f5fac 100644
--- a/Core/HW/MediaEngine.cpp
+++ b/Core/HW/MediaEngine.cpp
@@ -37,7 +37,10 @@ extern "C" {
 #include "libavformat/avformat.h"
 #include "libavutil/imgutils.h"
 #include "libswscale/swscale.h"
-
+#if LIBAVFORMAT_VERSION_INT >= AV_VERSION_INT(59, 23, 100)
+       // private libavformat api (see demux.h in ffmpeg src tree)
+       void avpriv_stream_set_need_parsing(AVStream *st, enum AVStreamParseType type);
+#endif
 }
 #endif // USE_FFMPEG

@@ -434,7 +437,11 @@ bool MediaEngine::addVideoStream(int streamNum, int streamId) {
 #else
                        stream->request_probe = 0;
 #endif
+#if LIBAVFORMAT_VERSION_INT >= AV_VERSION_INT(59, 23, 100)
+                       avpriv_stream_set_need_parsing(stream, AVSTREAM_PARSE_FULL);
+#else
                        stream->need_parsing = AVSTREAM_PARSE_FULL;
+#endif
                        // We could set the width here, but we don't need to.
                        if (streamNum >= m_expectedVideoStreams) {
                                ++m_expectedVideoStreams;
v-fox commented 1 year ago

So, I've kludged together a patch with all mentioned workarounds and ppsspp finally compiled with ffmpeg-6:

ppsspp-support_ffmpeg5.patch ```patch diff --git a/Core/AVIDump.cpp b/Core/AVIDump.cpp index 99c74af..a7d7207 100644 --- a/Core/AVIDump.cpp +++ b/Core/AVIDump.cpp @@ -91,7 +91,7 @@ bool AVIDump::Start(int w, int h) bool AVIDump::CreateAVI() { #ifdef USE_FFMPEG - AVCodec *codec = nullptr; + const AVCodec *codec = nullptr; // Use gameID_EmulatedTimestamp for filename std::string discID = g_paramSFO.GetDiscID(); diff --git a/Core/HLE/sceAtrac.cpp b/Core/HLE/sceAtrac.cpp index bf16281..4b194b2 100644 --- a/Core/HLE/sceAtrac.cpp +++ b/Core/HLE/sceAtrac.cpp @@ -123,6 +123,7 @@ static const int atracDecodeDelay = 2300; #ifdef USE_FFMPEG extern "C" { +#include "libavcodec/avcodec.h" #include "libavformat/avformat.h" #include "libswresample/swresample.h" #include "libavutil/samplefmt.h" diff --git a/Core/HLE/sceMpeg.cpp b/Core/HLE/sceMpeg.cpp index 2f8cb86..15b074c 100644 --- a/Core/HLE/sceMpeg.cpp +++ b/Core/HLE/sceMpeg.cpp @@ -108,6 +108,7 @@ static bool useRingbufferPutCallbackMulti = true; #ifdef USE_FFMPEG extern "C" { +#include "libavcodec/avcodec.h" #include "libavformat/avformat.h" #include "libavutil/imgutils.h" #include "libswscale/swscale.h" @@ -801,7 +802,7 @@ static bool InitPmp(MpegContext * ctx){ pmp_want_pix_fmt = AV_PIX_FMT_RGBA; // Create H264 video codec - AVCodec * pmp_Codec = avcodec_find_decoder(AV_CODEC_ID_H264); + const AVCodec * pmp_Codec = avcodec_find_decoder(AV_CODEC_ID_H264); if (pmp_Codec == NULL){ ERROR_LOG(ME, "Can not find H264 codec, please update ffmpeg"); return false; diff --git a/Core/HW/MediaEngine.cpp b/Core/HW/MediaEngine.cpp index fa4b819..40356bb 100644 --- a/Core/HW/MediaEngine.cpp +++ b/Core/HW/MediaEngine.cpp @@ -38,6 +38,10 @@ extern "C" { #include "libavutil/imgutils.h" #include "libswscale/swscale.h" +#if LIBAVFORMAT_VERSION_INT >= AV_VERSION_INT(59, 23, 100) + // private libavformat api (see demux.h in ffmpeg src tree) + void avpriv_stream_set_need_parsing(AVStream *st, enum AVStreamParseType type); +#endif } #endif // USE_FFMPEG @@ -410,13 +414,19 @@ bool MediaEngine::addVideoStream(int streamNum, int streamId) { #else stream->request_probe = 0; #endif +#if LIBAVFORMAT_VERSION_INT >= AV_VERSION_INT(59, 23, 100) + avpriv_stream_set_need_parsing(stream, AVSTREAM_PARSE_FULL); +#else stream->need_parsing = AVSTREAM_PARSE_FULL; +#endif // We could set the width here, but we don't need to. if (streamNum >= m_expectedVideoStreams) { ++m_expectedVideoStreams; } +#if LIBAVFORMAT_VERSION_INT < AV_VERSION_INT(57, 33, 100) m_codecsToClose.push_back(stream->codec); +#endif return true; } } @@ -499,7 +509,7 @@ bool MediaEngine::setVideoStream(int streamNum, bool force) { AVStream *stream = m_pFormatCtx->streams[streamNum]; #if LIBAVFORMAT_VERSION_INT >= AV_VERSION_INT(57, 33, 100) - AVCodec *pCodec = avcodec_find_decoder(stream->codecpar->codec_id); + const AVCodec *pCodec = avcodec_find_decoder(stream->codecpar->codec_id); if (!pCodec) { WARN_LOG_REPORT(ME, "Could not find decoder for %d", (int)stream->codecpar->codec_id); return false; diff --git a/Core/HW/SimpleAudioDec.cpp b/Core/HW/SimpleAudioDec.cpp index 8f250ab..dd8fa14 100644 --- a/Core/HW/SimpleAudioDec.cpp +++ b/Core/HW/SimpleAudioDec.cpp @@ -28,6 +28,7 @@ #ifdef USE_FFMPEG extern "C" { +#include "libavcodec/avcodec.h" #include "libavformat/avformat.h" #include "libswresample/swresample.h" #include "libavutil/samplefmt.h" diff --git a/Core/HW/SimpleAudioDec.h b/Core/HW/SimpleAudioDec.h index ec792c9..c96715b 100644 --- a/Core/HW/SimpleAudioDec.h +++ b/Core/HW/SimpleAudioDec.h @@ -78,7 +78,7 @@ private: int wanted_resample_freq; // wanted resampling rate/frequency AVFrame *frame_; - AVCodec *codec_; + const AVCodec *codec_; AVCodecContext *codecCtx_; SwrContext *swrCtx_; ```

I wish some of you with grand plans would expand on

Rest of the errors can be fixed by adding const and #include "libavcodec/avcodec.h" to appropriate places.

statement because putting those const was a real headache due to misleading compiler error on mismatch between .cpp and .h files.

jbeich commented 1 year ago

94bab4506f2e needs another fix due to AVStream.codec removal in favor of AVStream.codecpar:

Core/HW/MediaEngine.cpp:427:38: error: no member named 'codec' in 'AVStream'
                        m_codecsToClose.push_back(stream->codec);
                                                  ~~~~~~  ^
hrydgard commented 1 year ago

PRs accepted. As mentioned, the officially supported way to build is to use the included ffmpeg, not the system ffmpeg.

jbeich commented 1 year ago

the officially supported way to build is to use the included ffmpeg, not the system ffmpeg.

Until #15969 the included FFmpeg is not usable on BSDs or any other platform without prebuilt binaries. Providing binaries for just one target tuple requires care due to OS ABI changes across versions.

unknownbrackets commented 1 year ago

Until then, you can basically just run linux_x86-64.sh in that directory (even for BSD, it should be basically the same.) It's an extra step, but the binaries aren't really needed - it's just that compiling FFmpeg on Windows is a bit of a pain, and we want PPSSPP to be very easy to just download the source and be able to compile.

A rebuilt and trimmed version of FFmpeg named PPmpeg, though, would probably be easier to integrate into build systems. Still waiting for someone to come along and say they're tired of having free weekends for the next several months and always dreamed of rewriting FFmpeg, though.

-[Unknown]

v-fox commented 1 year ago

PRs accepted. As mentioned, the officially supported way to build is to use the included ffmpeg, not the system ffmpeg.

This is horrible development practice. Using ancient obsolete code, especially when few small fixes here and there worked just fine despite claims about "full rewrite needed", is a rabbit hole that can lead to bitrotting if you start making hard dependencies on other dead code willy-nilly. What else next, compiler and static glibc from 2015?

Ffmpeg-4 is a fat-ass library size of which is multiple of entirety of the emulator which is its sole user on an entire system. A whole bunch of dead code like that is horrible for distro maintenance.

But now with 1.15 even "my" patch is broken due to:

94bab45 needs another fix due to AVStream.codec removal in favor of AVStream.codecpar:

There is even recommendations for that since 2016: https://stackoverflow.com/questions/71565636/replacing-deprecated-avstream-codec-parameter-in-libav https://ffmpeg.org/pipermail/libav-user/2016-October/009801.html

A rebuilt and trimmed version of FFmpeg named PPmpeg, though, would probably be easier to integrate into build systems. Still waiting for someone to come along and say they're tired of having free weekends for the next several months and always dreamed of rewriting FFmpeg, though.

Removal of the offending line makes it compile without rewriting whole ffmpeg for months:

+#if LIBAVFORMAT_VERSION_INT < AV_VERSION_INT(57, 33, 100)
            m_codecsToClose.push_back(stream->codec);
+#endif

Is it really something out of your reach? It's sure out mine to make it proper and safe at runtime but I'm sure any one of you can manage better if you actually tried to follow the recommendation of your dependencies instead of going against them and giving up at first inconvenience as if its the whole world who is wrong and needs being recreated but by someone else.

unknownbrackets commented 1 year ago

This is horrible development practice. Using ancient obsolete code, especially when few small fixes here and there worked just fine despite claims about "full rewrite needed", is a rabbit hole that can lead to bitrotting if you start making hard dependencies on other dead code willy-nilly. What else next, compiler and static glibc from 2015?

You're right. Please email all the developers of each released PSP game and tell them they were stupid to write their games to play videos in a way that is no longer popular in 2023 and that they should be ashamed of themselves unless they port their all the games they released for PSP to modern consoles, for free, or at least redo the video playback in their games (again, for free.)

I'm so glad you've volunteered to do this. With 1400+ games made for the PSP, just contacting an arguing with all the developers of these games about their horrible development practices is a lot of work. You seem quite good at it, so I'm glad you've taken up the mantle. Looking forward to great results of your hard work, and thank you.

-[Unknown]

v-fox commented 1 year ago

You're right. Please email all the developers of each released PSP game and tell them they were stupid to write their games to play videos in a way that is no longer popular in 2023 and that they should be ashamed of themselves unless they port their all the games they released for PSP to modern consoles, for free, or at least redo the video playback in their games (again, for free.)

I'm so glad you've volunteered to do this. With 1400+ games made for the PSP, just contacting an arguing with all the developers of these games about their horrible development practices is a lot of work. You seem quite good at it, so I'm glad you've taken up the mantle. Looking forward to great results of your hard work, and thank you.

-[Unknown]

I did not volunteer to entertained your delusions. But I did volunteer to make a patch and post it here while you only tell tales and sarcastic personal attacks: https://github.com/hrydgard/ppsspp/issues/15308#issuecomment-1469358665

How about you tell me what exactly prevents you from taking and applying it ? Because, miraculously, ppsspp compiles against system ffmpeg6 and launches games with videos while you spin your dramatizations and weird excuses. I really wonder what the hell is this "big problem" while I somehow managed to bruteforce it by cluelessly throwing shit at a wall. Did you like the "great results of my hard work" ?

My patch is complete guess-work so I give no guarantees that things did not break at runtime. But it also just a collection of few simple one-liners. Since you fancy yourself such a specialist, it shouldn't be problematic for you to sort out few corner cases and follow the proper practices.

hrydgard commented 1 year ago

The patch looks okay as such, and I'm willing to merge it. It's still actually a fact though that due to differences in how ffmpeg works internally, for whatever reason, game compatibility with new ffmpeg versions is actually lower than with the stripped-down version of ffmpeg that we currently use and prefer.

And investigating why that is doesn't seem like a very good use of anybody's time. All video we're ever gonna play with this was encoded between 2005 and 2012-ish (as that's the period of time when PSP games were made), so it doesn't seem wrong to use an old decoder.

v-fox commented 1 year ago

And investigating why that is doesn't seem like a very good use of anybody's time. All video we're ever gonna play with this was encoded between 2005 and 2012-ish (as that's the period of time when PSP games were made), so it doesn't seem wrong to use an old decoder.

That's the thing with all emulators. PS1 video are even older. wine has the same issue of dealing with ancient games and it uses gstreamer->ffmpeg to achieve this. But it's not excuse to use unmaintained code from that era and leave 99% of it as unused dead weight. It wouldn't be an issue if it was some small static lib but ffmpeg is gigantic. If everyone starts doing that then Linux will turn from 5-10 GB full-featured system into 50 GB dll-hell of broken dependencies and duplicates. Distro maintainers were combating the same thing happening with LLVM for a while now. Our side mostly won so far but this is a place I did not expect such dogmatic pushback from.

I get that figuring out such complex framework takes skill and patience. But claiming that you "have to" rewrite the whole thing is… strange, to say the least. This is something to say after you've completed basic maintenance loop: read official ffmpeg documentation on porting from old API, tried it, failed completely, appealed to its devs to give you needed API, failed again and then gave up. Stopping at one-liners is just embarrassing unless one could claim to be as incompetent at this as I am.

hrydgard commented 1 year ago

Well the goal of PPSSPP has never been to be a dependency-optimized module in a Linux system. It's an app with the goal to run PSP games as well as possible. There's then a compromise necessary between spending the time on cleaning dependencies (which is VERY labor-intensive given that we run on so many non-linux platforms), and actually working on emulating games.

I'm not in any way against taking contributions that make us work better as a part of a linux system as long as it doesn't make our other work harder in any way. But I hope you understand that dependency-optimization, while nice, is never top priority for us.

And the technical part of porting to a new ffmpeg version is no big deal, but again, it is a fact that there are games that are not functioning properly with newer versions, unknown why and likely extremely time consuming to investigate properly, while we already have a solution that works for us.

unknownbrackets commented 1 year ago

Well, some of why is known - videos in PSP games don't always have entirely correct headers, but it's OK because the PSP firmware doesn't use them. FFmpeg does, but there were ways to force the correct data in before - ways that are gone in newer FFmpeg versions.

There are other problems anyway, such as how FFmpeg always wants to "read ahead" on the stream which sometimes causes temporary corruption (I believe you can still see this in Valkyie Profile for example.) Eventually PPSSPP probably does need to be divorced from FFmpeg's parsing for various reasons, as I said. The newer FFmpeg versions are only making it farther.

Just being proud of it compiling isn't a big deal. I was the one who put the #ifs in to make it use broken legacy paths in newer FFmpeg versions already, I just haven't wasted my time since FFmpeg 3 or 4 or something to make it pointlessly compile with reduced compatibility. But yes, I'm sure it can be made to compile with newer FFmpeg. And I realize that is the only thing package maintainers generally care about - once it compiles, they indeed intend for someone else to do the actual work of making it work properly. But at that point it's just bug reports that don't go to the package maintainers, so their job is done.

-[Unknown]

v-fox commented 1 year ago

Well the goal of PPSSPP has never been to be a dependency-optimized module in a Linux system. It's an app with the goal to run PSP games as well as possible. There's then a compromise necessary between spending the time on cleaning dependencies (which is VERY labor-intensive given that we run on so many non-linux platforms), and actually working on emulating games.

I'm not in any way against taking contributions that make us work better as a part of a linux system as long as it doesn't make our other work harder in any way. But I hope you understand that dependency-optimization, while nice, is never top priority for us.

Having basic student-grade code maintenance is far from extreme of going for perfection. I understand where you coming from BUT I have almost 2 decades of experience of distro management and packaging that shows this as a very common bad habit learned and reinforced by poor maintenance practices of Windows & MacOS development. Like any thing, if you do it long enough then anything become a norm. After which one can't even perceive how outrageous it may be. Everyone has this tendency but it's no justification for praising bad habits.

And the technical part of porting to a new ffmpeg version is no big deal, but again, it is a fact that there are games that are not functioning properly with newer versions, unknown why and likely extremely time consuming to investigate properly, while we already have a solution that works for us.

Giant piles of dead code is not a solution and generalized statements are meaningless. If you know exactly that something has broke then correct course of action is contacting ffmpeg developers and telling them what kind of workaround you need.

Well, some of why is known - videos in PSP games don't always have entirely correct headers, but it's OK because the PSP firmware doesn't use them. FFmpeg does, but there were ways to force the correct data in before - ways that are gone in newer FFmpeg versions.

Then go to ffmpeg devs and appeal to return them as "totally necessary". If they as stubborn as you are then at least you will have facts to back up your ridiculous statements. So far it's a whole lot of "we tried nothing and we're all out of ideas".

Just being proud of it compiling isn't a big deal. I was the one who put the #ifs in to make it use broken legacy paths in newer FFmpeg versions already, I just haven't wasted my time since FFmpeg 3 or 4 or something to make it pointlessly compile with reduced compatibility. But yes, I'm sure it can be made to compile with newer FFmpeg. And I realize that is the only thing package maintainers generally care about - once it compiles, they indeed intend for someone else to do the actual work of making it work properly. But at that point it's just bug reports that don't go to the package maintainers, so their job is done.

Yes, how DARE package maintainers care about maintenance practices and user experience while trying to manage thousands of packages in a constantly moving target. Let's instead all engage in endless sophistry and write entire novels of facetious excuses, throw shit at anyone who tries to do anything "badly" while not contributing instead of anything of value like reading the newer manuals, asking the right people to fix it in right places and backing up your stream of consciousness with correct by-the-book work.

xgqt commented 1 year ago

But at that point it's just bug reports that don't go to the package maintainers, so their job is done.

@unknownbrackets

Most "major" Linux distributions and BSD systems have bug trackers with automated package testing and bug reports indeed do go straight to the package maintainers and then it is their job to take care of how a given package interacts with the other packaged software. For example see: https://bugs.gentoo.org/886177 where we decided that PPSSPP compilation option overwriting was acceptable. (Also, for the record - I do use PPSSPP myself :D)

Some software is very complicated and it might be better to report the bug upstream to get a satisfiable/correct approach to solving the given problem - speaking from experience with misc Lisp/Scheme software. But that does not mean that I (or I think any other pkg maint) say "go fix it" - I just want to let the upstream maintainer know that such a problem exists. Afterwards both parties should concentrate on the problem.

Tatsh commented 9 months ago

This was fixed in #18670 @hrydgard

v-fox commented 9 months ago

This was fixed in #18670

Is this actually tested to work? Because this seems to be very short change for such long talk.

hrydgard commented 9 months ago

Well, the issue is about building, not working :) Besides, this is not really an officially supported configuration, just something that some people feel is useful anyway somehow. We only really support using our copy of ffmpeg.

v-fox commented 9 months ago

Well, the issue is about building, not working :) Besides, this is not really an officially supported configuration, just something that some people feel is useful anyway somehow. We only really support using our copy of ffmpeg.

Knowingly closing something unusable as 'fixed' seems like a case of what they call 'malicious compliance'. Which would be especially bad if it hangs and not just gives out corrupted rendering.

"We only really support using our copy of X" is kind of meaningless in case of Linux and emulators as almost all of them bundle almost every single dependency in giant hodgepodge pile for their Windows/MacOS builds and we can't mimic that. But I'm sure you're well-versed in this too.

hrydgard commented 9 months ago

I can reopen it, sure, but I have zero plans to do any further work on it. It's just not feasible to support all possible combinations here.

Instead, what we really should do is to do our own demuxing and reduce our reliance on ffmpeg, and ideally call the codecs directly bypassing it altogether, in as many cases as possible. The thing is, the PSP has some quirks in its demuxer that doesn't really match what ffmpeg is doing. The old version we ship is closer than the newer version of ffmpeg, and for that reason we just can't maintain the same compatibility across versions of ffmpeg the way things work currently. This has been explained many times, and I'm not really sure what you are implying the alternative is.

v-fox commented 9 months ago

I can reopen it, sure, but I have zero plans to do any further work on it. It's just not feasible to support all possible combinations here.

Good. At least it may serve as warning for those who try building like that.

Instead, what we really should do is to do our own demuxing and reduce our reliance on ffmpeg, and ideally call the codecs directly bypassing it altogether, in as many cases as possible. The thing is, the PSP has some quirks in its demuxer that doesn't really match what ffmpeg is doing. The old version we ship is closer than the newer version of ffmpeg, and for that reason we just can't maintain the same compatibility across versions of ffmpeg the way things work currently. This has been explained many times, and I'm not really sure what you are implying the alternative is.

Whatever issue may be, abandoned code is abandoned code with all its baggage. I switched to use bundled ffmpeg for my packages and see that it's cut down enough that build time and size are reasonable. As long as they are then it's fine by me. For now, maybe at least big warning at configuration phase is in order, to communicate that it's not just 'not recommended' (all non-default configs are and most of them suck for packaging) but that 'successful' build is broken and should not be used. Same for Qt versus SDL build.

But so many projects go as far as to fork or get 'stuck' on old versions of fatties like entire LLVM (rpcs3 is a big offender in that but recently it gets satisfied with a current version). So I'm more afraid of a potential slippery-slope maintenance-nightmare scenario when more dependencies would get stuck like that.