Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port FFmpeg 7.0 to ffmpegdirect Piers #304

Open
wants to merge 1 commit into
base: Piers
Choose a base branch
from

Conversation

basilgello
Copy link
Contributor

From xbmc/xbmc#24972

Signed-off-by: Vasyl Gello [email protected]

@basilgello
Copy link
Contributor Author

@phunkyfish I tested that on Omega in Debian - works so far!

@phunkyfish
Copy link
Collaborator

We can’t do this on omega, we don’t change ffmpeg for released versions. It needs to be on the Piers branch.

@phunkyfish
Copy link
Collaborator

But should apply the same on Piers.

And nice work!

@basilgello
Copy link
Contributor Author

It id targeting Piers branch, despite the commit name!

@phunkyfish phunkyfish changed the title Port FFmpeg 7.0 to ffmpegdirect Omega Port FFmpeg 7.0 to ffmpegdirect Piers Aug 12, 2024
@phunkyfish
Copy link
Collaborator

Updated!

@phunkyfish
Copy link
Collaborator

phunkyfish commented Aug 19, 2024

Can you rebase and update from the parent PR? That’s on 7.0.2 now and the Android build issues are fixed on Piers. So it should build fully on Jenkins.

Also, please check that any patches apply cleanly for ffmpeg. There are 5 altogether I think.

@basilgello
Copy link
Contributor Author

Roger that! :)

@basilgello
Copy link
Contributor Author

@phunkyfish the rebased original PR does not bring anything meaningful for FFmpegDirect:

git diff f89f51b8a67623f4882c455cd0f6615618096ebd..1d844951fd61d4a892cfbb5e22983a21c721bba7 -- $(git diff --name-only 031877fd4b833028b25a7d3f59f269907e9b4403..1d844951fd61d4a892cfbb5e22983a21c721bba7)

But I will update to 7.0.2 and ensure patches make sense

@basilgello
Copy link
Contributor Author

diff --git a/addons/resource.language.en_gb/resources/strings.po b/addons/resource.language.en_gb/resources/strings.po
index 0a67421cae..9ea4294d1f 100644
--- a/addons/resource.language.en_gb/resources/strings.po
+++ b/addons/resource.language.en_gb/resources/strings.po
@@ -11351,27 +11351,23 @@ msgstr ""
 
 #. Label for a select option or list item, representing an icon graphic (like a TV channel icon)
 #: xbmc/pvr/dialogs/GUIDialogPVRChannelManager.cpp
-#: xbmc/pvr/guilib/PVRGUIActionsEPG.cpp
 msgctxt "#19282"
 msgid "Current icon"
 msgstr ""
 
 #. Label for a select option or list item, representing an icon graphic (like a TV channel icon)
 #: xbmc/pvr/dialogs/GUIDialogPVRChannelManager.cpp
-#: xbmc/pvr/guilib/PVRGUIActionsEPG.cpp
 msgctxt "#19283"
 msgid "No icon"
 msgstr ""
 
-#. used by several skins and PVR
-#: xbmc/pvr/PVRContextMenus.cpp
+#. used by several skins
 msgctxt "#19284"
 msgid "Choose icon"
 msgstr ""
 
 #. Label for a select/menu option to select an icon graphic
 #: xbmc/pvr/dialogs/GUIDialogPVRChannelManager.cpp
-#: xbmc/pvr/guilib/PVRGUIActionsEPG.cpp
 msgctxt "#19285"
 msgid "Browse for icon"
 msgstr ""
@@ -17915,39 +17911,8 @@ msgctxt "#34113"
 msgid "To keep certain AVRs powered we send an inaudible random noise signal. You can disable this setting if you are using headphone or analog output."
 msgstr ""
 
-#. Indicates if Audio Engine should include lfe when downmixing
-#: system/settings/settings.xml
-msgctxt "#34114"
-msgid "Include LFE when downmixing"
-msgstr ""
-
-#. Description of setting with label #34114 "Include LFE when downmixing"
-#: system/settings/settings.xml
-msgctxt "#34115"
-msgid "If enabled, this setting will include lfe channel into the mixing when there is no dedicated LFE output channel available. This only makes sense for full range speakers."
-msgstr ""
-
-#. Description of setting with label #34114 "Include LFE when downmixing"
-#: system/settings/settings.xml
-msgctxt "#34116"
-msgid "Off"
-msgstr ""
-
-#. Description of setting with label #34114 "Include LFE when downmixing"
-#: system/settings/settings.xml
-msgctxt "#34117"
-msgid "50%"
-msgstr ""
-
-#. Description of setting with label #34114 "Include LFE when downmixing"
-#: system/settings/settings.xml
-msgctxt "#34118"
-msgid "100%"
-msgstr ""
-
-
 #empty strings from id 34114 to 34119
-#34119 reserved for future use
+#34114-34119 reserved for future use
 
 #: system/settings/settings.xml
 msgctxt "#34120"
diff --git a/xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAEResampleFFMPEG.cpp b/xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAEResampleFFMPEG.cpp
index a4fc91554f..6fce0af981 100644
--- a/xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAEResampleFFMPEG.cpp
+++ b/xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAEResampleFFMPEG.cpp
@@ -29,15 +29,8 @@ CActiveAEResampleFFMPEG::~CActiveAEResampleFFMPEG()
   swr_free(&m_pContext);
 }
 
-bool CActiveAEResampleFFMPEG::Init(SampleConfig dstConfig,
-                                   SampleConfig srcConfig,
-                                   bool upmix,
-                                   bool normalize,
-                                   double centerMix,
-                                   CAEChannelInfo* remapLayout,
-                                   AEQuality quality,
-                                   bool force_resample,
-                                   float sublevel)
+bool CActiveAEResampleFFMPEG::Init(SampleConfig dstConfig, SampleConfig srcConfig, bool upmix, bool normalize, double centerMix,
+                                   CAEChannelInfo *remapLayout, AEQuality quality, bool force_resample)
 {
   m_dst_chan_layout = dstConfig.channel_layout;
   m_dst_channels = dstConfig.channels;
@@ -141,9 +134,6 @@ bool CActiveAEResampleFFMPEG::Init(SampleConfig dstConfig,
     return false;
   }
 
-  if (sublevel > 0.0f)
-    av_opt_set_double(m_pContext, "lfe_mix_level", static_cast<double>(sublevel), 0);
-
   if (hasMatrix)
   {
     if (swr_set_matrix(m_pContext, reinterpret_cast<const double*>(m_rematrix), AE_CH_MAX) < 0)

@basilgello
Copy link
Contributor Author

Looks like 1h is definitely not enough for the pipeline.

@phunkyfish
Copy link
Collaborator

Looks like 1h is definitely not enough for the pipeline.

We in only need to worry about Jenkins. Everything else will timeout.

@phunkyfish
Copy link
Collaborator

phunkyfish commented Aug 20, 2024

The patches do not apply cleanly. Can you fix them?

[ 87%] Performing patch step for 'ffmpeg'
patching file configure
Hunk #1 succeeded at 7060 (offset 332 lines).
patching file configure
Hunk #1 succeeded at 8188 (offset 363 lines).
patching file configure
Hunk #1 succeeded at 5784 (offset 218 lines).
Hunk #2 succeeded at 5837 (offset 220 lines).
patching file libavcodec/dxva2.c
Hunk #1 succeeded at 782 (offset 5 lines).

I don't see patch 4 being applied. Maybe it is no longer required.

@basilgello
Copy link
Contributor Author

I have removed one patch and renamed 0005 to 0004. OK I will re-export them

@phunkyfish
Copy link
Collaborator

I have removed one patch and renamed 0005 to 0004. OK I will re-export them

Perfect, thanks!

Copy link
Collaborator

@phunkyfish phunkyfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved for when Kodi PR gets merged

@zuzia-dev
Copy link

I tested under Debian 12 with KODI v22 Piers and it works great. Thanks!

@basilgello
Copy link
Contributor Author

@zuzia-dev You did build ffmpeg 7.0.2 for 12 or reverted the ffmpeg6 patch?

@zuzia-dev
Copy link

zuzia-dev commented Sep 1, 2024

You did build ffmpeg 7.0.2 for 12 or reverted the ffmpeg6 patch?

@basilgello
I compiled KODI v22 with ffmpeg 7.0.2: xbmc/xbmc#24972

@basilgello
Copy link
Contributor Author

Thanks. In Debian 12 I will need to build against ffmpeg6, just in case :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants