RobertBeckebans / RBDOOM-3-BFG

Doom 3 BFG Edition source port with updated DX12 / Vulkan renderer and modern game engine features
https://www.moddb.com/mods/rbdoom-3-bfg
GNU General Public License v3.0
1.38k stars 247 forks source link

Fails to compile against FFmpeg version 4.4+ #628

Closed EndlessEden closed 1 year ago

EndlessEden commented 2 years ago

api depreciation since FFmpeg version 3.1 have been removed starting with version 4.4.

Notably, avcodec_decode_video2() The original decoding function is disassembled into two functions avcodec_send_packet() and avcodec_receive_frame().

but also avpicture_fill() [ replaced with av_image_fill_arrays ] av_free_packet(packet); [ replaced with av_packet_unref(packet); ]


I implore you to please update neo/renderer/Cinematic.cpp so it supports modern versions of FFmpeg and versions to come.

EndlessEden commented 2 years ago

if you need more quick info, these two projects have done the same. - Here is a reference (https://www.programmerall.com/article/36691434216/)

and here are three projects whom have done this as well, whom all used these functions. https://github.com/pesintta/vdr-plugin-vaapidevice/pull/38/commits/c5cbb26f3459f9511e82f7eb1070e3ddea73cf02 https://mail.gnome.org/archives/commits-list/2016-February/msg05531.html https://github.com/dmlc/decord/pull/160/commits

SRSaunders commented 2 years ago

I am not sure if this will help your situation, but as a work-around you can build RBDOOM-3-BFG without any dependencies on FFmpeg. Just add the following cmake options to your cmake-xxx shell script: -DFFMPEG=OFF -DBINKDEC=ON. With these build options the internal binkdec decoder replaces the external ffmpeg decoder.

MadDeCoDeR commented 2 years ago

Just a minor heads up on this. The issuer have send in my repo a similar issue, as it seems the issue wasn't only with old FFMPEG but also with deprecated functions that FFMPEG 4.4 still uses. If anyone is interested to update it look for the newly released version 5.0 of the FFMPEG instead since they have completely removed the deprecated types and functions

coldtobi commented 2 years ago

FWWIW, Debian is also moving to ffmpeg 5.0 soon(tm) -- https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1004574 -- so any hints or migration guides what needs to be changed would be highly appreciated so that I can keep rbdoom3bfg in Debian...

MadDeCoDeR commented 2 years ago

You can always check my repo (BFA's code) for the necessary changes. I have already update the FFMPEG code in order to be compatible with 5.0

SRSaunders commented 2 years ago

@coldtobi are you trying to build using the cmake-linux-debug.sh script? If so that one depends on ffmpeg as you note above. Until someone contributes the migration work for ffmpeg 5, I suggest you simply build using the cmake-linux-release.sh script from the RBDoom3BFG repository, or copy its contents into your build environment. This cmake shell script enables the internal BinkDec decoder and completely disables the dependency on ffmpeg. The important cmake settings are: -DFFMPEG=OFF -DBINKDEC=ON. No functionality is lost - game cinematics play back just fine with the BinkDec decoder. Please try this out to see if it meets your needs in the short term.

FYI - for your reference, see the notes regarding ffmpeg in the Linux build section of the RBDoom3BFG README: https://github.com/RobertBeckebans/RBDOOM-3-BFG#compile_linux

coldtobi commented 2 years ago

@MadDeCoDeR thanks, I'll take a look if I can extract whats need to be changed.

@SRSaunders Thanks for you input, but unfortunately won't cut it ... This is for "the" Debian package (as in https://tracker.debian.org/pkg/rbdoom3bfg), so it will not be possible to stay with ffmpeg 4.x, as Debian is moving away from that version. (Which will not be tomorrow, as the library transition bug mentions a ton of affected packages, https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1004831) Its good to know that there is a fallback (DBINKDEC), however, frankly, I'd like to stay with ffmeg for Debian if possible. As said, no urgency yet, the ffmpeg libray will for sure take some time to complete...

SRSaunders commented 2 years ago

@coldtobi thanks for your response. Just for clarity, I was not suggesting that you stay with ffmpeg 4, but instead that you could move the official Debian package to use BinkDec as the default for RBDoom3BFG since that option is available now. This would work for RBDoom3BFG 1.3+ release packages and perhaps for the 1.2 release, but not for anything earlier. However, if this is not your preference and you want to stay with ffmpeg for RBDoom3BFG on Debian that is your choice. What is the timeframe needed for a solution?

Note that I made some changes to Cinematic.cpp and a few other places in my most recent pull request (issue #633 - currently pending) so we will have to be careful about coordination for new ffmpeg 5 changes. Fortunately my changes are mostly in the BinkDec area so the likelihood of overlap should be low. Just a heads-up.

coldtobi commented 2 years ago

timeline: I guess it will take some time until the transition is ready. (as there are so many pacakges involved), but it is hard to put into concrete numbers. Said that, Debian release cycle has just started, next freeze will be 2023, and this is the final date to get something into a Debian releases. Said that, I'd like to avoid that it drops out of testing, as that would affect several derivate distributions (like Ubuntu...); it will drop out of testing once the transition is started (Debian #1004831 will then become "release critical."

TL;DR: No urgency yet.

SRSaunders commented 2 years ago

I was curious about the effort so I just went ahead and tried it. Thanks to @MadDeCoDeR's excellent work, I have ported over the changes removing the deprecated calls and have it running on macOS with ffmpeg 4.4.1 / OpenAL and will try it on Linux with ffmpeg 5 next. Hopefully it just runs as is.

I notice a couple of things:

  1. Audio for the intro video now works - excellent!
  2. av_init_packet() is still showing as deprecated. Is this API still part of ffmpeg5 or will it disappear as well?
  3. convergence_duration is also showing as deprecated. Same question as above, but may be less of a concern since this looks like an implicit type issue that may automatically disappear with newer versions of ffmpeg.

First I will try it on Linux and then move on to Windows with XAudio2. Lastly I will see if I can eliminate the final deprecation warnings. However, in general this is a positive step.

coldtobi commented 2 years ago

Nice! If you want, put it on some branch and I give it a spin as well. (and I try compile it against ffmpeg 5)

SRSaunders commented 2 years ago

Quick update on progress:

  1. Validated with ffmpeg 5 on macOS. Re issues 2&3 above: i) av_init_packet() is still available in ffmpeg5 but marked as deprecated, and ii) as I suspected the convergence_duration issue disappeared with ffmpeg5.
  2. Fixed a few bugs in the new code: i) cinematicAudio must be an instance variable of idCinematicLocal, and ii) don't try to unqueue buffers that were never queued in the first place inside CinematicAudio_OpenAL::ShutdownAudio(). These two fixes get rid of OpenAL errors in the log. They may be useful fixes for @MadDeCoDeR and the BFA code base.
  3. Tested with both OpenGL and Vulkan builds. No issues and no Vulkan validation errors.

I will try to get a branch built tomorrow with these changes for testing on Linux. Then on to Windows and XAudio2.

MadDeCoDeR commented 2 years ago

@SRSaunders Thanks for the feedback

SRSaunders commented 2 years ago

@coldtobi - The ffmpeg 5 + cinematic audio changes are complete and you can try my latest development branch at https://github.com/SRSaunders/RBDOOM-3-BFG/tree/displaymode-timer-gpu

The changes have been tested on Windows 10 (both XAudio2 and OpenAL Soft), Linux Manjaro, and macOS Big Sur. Hopefully they also work for Debian. Please let me know how it goes.

Note this dev branch is both a bit behind Robert's master, and ahead by my 2 pending pull requests combined with this new work. If you determine that it builds and runs on Debian with ffmpeg 5, I will add it to my 2nd pending pull request. I tried making a new branch off of Robert's current master, but there were conflicts due to pending changes in overlapping files in my 2nd pending pull request. My 1st pending pull request (#626) is independent, but this new work depends on my 2nd pull request (#633).

Now that the cinematic audio framework is ready, I may try to hook up the BinkDec decoder just for fun.

UPDATE Feb 10/2022: I just pushed a new commit to my dev branch that implements the following:

  1. Simplifies and fixes bugs in the ffmpeg cinematic audio playback code
  2. Audio to video sync for ffmpeg cinematic playback - previously audio was ahead of ffmpeg video by ~0.5 sec
  3. Adds Bink cinematic audio as an alternative to ffmpeg, no audio/video sync issues seen with the Bink decoder
  4. Adds proper audio buffer memory management - previously audio buffers were not released after use
  5. Tested on Windows (XAudio2 & OpenAL), Linux (Manjaro), macOS Big Sur using OpenGL and Vulkan renderers
  6. Added to pull request #633
SRSaunders commented 2 years ago

@EndlessEden and @coldtobi this issue should now be solved in the master branch and the 1.4 release. Would you please verify and close if working properly for you. Otherwise please report any remaining issues. Thanks.

coldtobi commented 2 years ago

@SRSaunders sorry for the delay: Yes. the new 1.4 version looks good for ffmpeg 5.0 support. Thanks for your efforts, very much appreciated!

(I do not have bug closing powers, unfortunatly)

SRSaunders commented 2 years ago

Thanks for the feedback and glad it's working. Happy to help.

Perhaps @EndlessEden could also review and close once verified.

runlevel5 commented 1 year ago

@SRSaunders would you be able to rebase your branch and create a new PR?

SRSaunders commented 1 year ago

@runlevel5: Rebase to what branch? These changes were integrated into main quite a while ago (#633), with subsequent bug fixes as well (#636, #644). Have new issues arisen? What are you trying to solve?

Note this tracking issue should have been closed by @EndlessEden a while ago.

runlevel5 commented 1 year ago

@SRSaunders my bad. I was having impression this has not been fixed. So I guess it is good to close this.

IF @EndlessEden could not close it, @RobertBeckebans could close it now

SRSaunders commented 1 year ago

No worries. Glad there are no new issues. Hopefully Robert can close it.