这是indexloc提供的服务,不要输入任何密码
Skip to content

addpkg(main/jellyfin): 10.10.7 #24184

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

addpkg(main/jellyfin): 10.10.7 #24184

wants to merge 3 commits into from

Conversation

NORALDM
Copy link
Contributor

@NORALDM NORALDM commented Apr 7, 2025

Closes #9751
Supersedes #23074
Had to go a bit overboard with the post_get_source step because multiple git urls in TERMUX_PKG_SRCURL don't seem to be supported. Most patches are taken from the termux packages repo except for skia-DEPS.patch, skia-gn-build.patch, and jellyfin-defaults.patch.

@NORALDM
Copy link
Contributor Author

NORALDM commented Apr 7, 2025

I have been able to personally test the arm64 build but both x86 builds remain untested, would be nice if someone could test those

Copy link
Member

@TomJo2000 TomJo2000 left a comment

Choose a reason for hiding this comment

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

The skiasharp part of this build should probably be split out into a separate package that jellyfin-server can then depend on.

Same for jellyfin-ffmpeg probably, although that may be better served as a subpackage of jellyfin-server.
See also:

TERMUX_SUBPKG_DESCRIPTION="Minimal FFmpeg libraries for Audacity"
TERMUX_SUBPKG_INCLUDE="
opt/audacity/lib/
share/doc/audacity-ffmpeg/
"

Thank you for taking over this PR, it's quite a lot of work.

TERMUX_PKG_DESCRIPTION="A free media system for organizing and streaming media (server)"
TERMUX_PKG_LICENSE="GPL-2.0"
TERMUX_PKG_MAINTAINER="@termux"
TERMUX_PKG_VERSION="10.10.7"
Copy link
Member

Choose a reason for hiding this comment

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

Consider using an array for the version here, this is supported by the build system.

Suggested change
TERMUX_PKG_VERSION="10.10.7"
TERMUX_PKG_VERSION=(
"10.10.7"
"7.0.2.9"
)

The - part of the version number is reserved for the package revision and should be replaced with a . here and substituted for the URL.

_FFMPEG_VERSION="7.0.2-9"
TERMUX_PKG_SRCURL=(
"https://github.com/jellyfin/jellyfin/archive/refs/tags/v${TERMUX_PKG_VERSION}.tar.gz"
"https://github.com/jellyfin/jellyfin-ffmpeg/archive/refs/tags/v${_FFMPEG_VERSION}.tar.gz"
Copy link
Member

Choose a reason for hiding this comment

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

We can use a bit of substitution here to fix the URL for jellyfin-ffmpeg

Suggested change
"https://github.com/jellyfin/jellyfin-ffmpeg/archive/refs/tags/v${_FFMPEG_VERSION}.tar.gz"
TERMUX_PKG_SRCURL=(
"https://github.com/jellyfin/jellyfin/archive/refs/tags/v${TERMUX_PKG_VERSION[0]}.tar.gz"
"https://github.com/jellyfin/jellyfin-ffmpeg/archive/refs/tags/v${TERMUX_PKG_VERSION[1]%.*}-${TERMUX_PKG_VERSION[1]##*.}.tar.gz"
)

local _idx;
local _sha256sums=();
local _error=0;
( [ "${#_GIT_URL[@]}" -eq "${#_GIT_COMMIT[@]}" ] && [ "${#_GIT_URL[@]}" -eq "${#_GIT_BRANCH[@]}" ] ) || termux_error_exit '_GIT_URL, _GIT_COMMIT and _GIT_BRANCH must be arrays of the same length'
Copy link
Member

Choose a reason for hiding this comment

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

Our build scripts are written for Bash, and arithmetic evaluation lets this be expressed a bit more nicely.

Suggested change
( [ "${#_GIT_URL[@]}" -eq "${#_GIT_COMMIT[@]}" ] && [ "${#_GIT_URL[@]}" -eq "${#_GIT_BRANCH[@]}" ] ) || termux_error_exit '_GIT_URL, _GIT_COMMIT and _GIT_BRANCH must be arrays of the same length'
(( ${#_GIT_URL[@]} == ${#_GIT_COMMIT[@]} && ${#_GIT_URL[@]} == ${#_GIT_BRANCH[@]} )) || termux_error_exit '_GIT_URL, _GIT_COMMIT and _GIT_BRANCH must be arrays of the same length'

"git+https://github.com/ericsink/cb.git"
"git+https://github.com/ericsink/SQLitePCL.raw.git"
"git+https://github.com/mono/skia.git"
"git+https://github.com/jellyfin/jellyfin-web.git"
Copy link
Member

Choose a reason for hiding this comment

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

  • 1.) There shouldn't be .git suffixes on those URLs.
  • 2.) Consider using associcative arrays (hashmaps) if you need a secondary index, e.g. the project name.
    kitty's build script does something similar to this.
    local -A ver=(
    [libx11]="$(. "${TERMUX_SCRIPTDIR}/packages/libx11/build.sh"; echo "${TERMUX_PKG_VERSION}")"
    [libxcb]="$(. "${TERMUX_SCRIPTDIR}/packages/libxcb/build.sh"; echo "${TERMUX_PKG_VERSION}")"
    [xcb_proto]="$(. "${TERMUX_SCRIPTDIR}/packages/xcb-proto/build.sh"; echo "${TERMUX_PKG_VERSION}")"
    [libxkbcommon]="$(. "${TERMUX_SCRIPTDIR}/x11-packages/libxkbcommon/build.sh"; echo "${TERMUX_PKG_VERSION}")"
    )
    local -A srcurl=(
    [libx11]="$(. "${TERMUX_SCRIPTDIR}/packages/libx11/build.sh"; echo "${TERMUX_PKG_SRCURL}")"
    [libxcb]="$(. "${TERMUX_SCRIPTDIR}/packages/libxcb/build.sh"; echo "${TERMUX_PKG_SRCURL}")"
    [xcb_proto]="$(. "${TERMUX_SCRIPTDIR}/packages/xcb-proto/build.sh"; echo "${TERMUX_PKG_SRCURL}")"
    [libxkbcommon]="$(. "${TERMUX_SCRIPTDIR}/x11-packages/libxkbcommon/build.sh"; echo "${TERMUX_PKG_SRCURL}")"
    )
    local -A sha256=(
    [libx11]="$(. "${TERMUX_SCRIPTDIR}/packages/libx11/build.sh"; echo "${TERMUX_PKG_SHA256}")"
    [libxcb]="$(. "${TERMUX_SCRIPTDIR}/packages/libxcb/build.sh"; echo "${TERMUX_PKG_SHA256}")"
    [xcb_proto]="$(. "${TERMUX_SCRIPTDIR}/packages/xcb-proto/build.sh"; echo "${TERMUX_PKG_SHA256}")"
    [libxkbcommon]="$(. "${TERMUX_SCRIPTDIR}/x11-packages/libxkbcommon/build.sh"; echo "${TERMUX_PKG_SHA256}")"
    )

git fetch --depth 1 origin "${_GIT_COMMIT[_idx]}"
git -c advice.detachedHead=false checkout "${_GIT_COMMIT[_idx]}"
} >/dev/null
# we need a deterministic checksum, logs get in the way
Copy link
Member

Choose a reason for hiding this comment

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

This seems convoluted.
What's the goal here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The git fetch and checkout are specifically for doing a shallow clone of a specific commit. The >/dev/null and braces are an artifact from the update-git-checksums.sh script though, I'll remove that.

elif [ "$TERMUX_ARCH" = "x86_64" ]; then
_target_cpu="x64"
elif [ "$TERMUX_ARCH" = "i686" ]; then
_target_cpu="x86"
Copy link
Member

Choose a reason for hiding this comment

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

Consider making this a case statement instead.

# if [[ -f "debian/patches/series" ]]; then
# quilt push -a
# fi
for i in $(cat debian/patches/series); do git apply --whitespace=nowarn debian/patches/$i; done
Copy link
Member

Choose a reason for hiding this comment

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

There's no need to oneliner this.

Suggested change
for i in $(cat debian/patches/series); do git apply --whitespace=nowarn debian/patches/$i; done
for i in $(< debian/patches/series); do
git apply --whitespace=nowarn "debian/patches/$i"
done

I'd also suggest using $(<, it's equivalent to $(cat1.

Footnotes

  1. The Bash manual has a bad habit of mentioning useful features offhandedly exactly once.
    https://man.archlinux.org/man/bash.1#Command_Substitution

@NORALDM
Copy link
Contributor Author

NORALDM commented Apr 7, 2025

Not sure why this closed on force pushing, might have been my git alias which resets to master to trigger the CI on my repo 🫤
Most of the suggestions have been implemented, but bundling skiasharp in a different package would require setting rpath to $PREFIX/lib for libcoreclr.so in the dotnet packages. Also, running
find -name '*.so' -exec sh -c 'readelf -d {} | grep -q "RUNPATH.*\[/usr/local/lib\]" && basename {}' \; | sort | uniq on the contents of the freebsd package for dotnet8 reveals 5 libraries, not sure if that's relevant or not (libclrjit and libmscordaccore don't seem to be distributed in Termux's dotnet package)

libSystem.Native.so
libSystem.Net.Security.Native.so
libclrjit.so
libcoreclr.so
libmscordaccore.so

@Asendaushoe
Copy link

Any process?

@truboxl
Copy link
Contributor

truboxl commented May 26, 2025

I didnt review fully, but I suggest you split libe_sqlite, skiasharp, sqlitepcl.raw to its own package for other .NET packages to reuse

Also do not depend on ffmpeg as you will be pulling 2 copies of ffmpeg, depend on ffmpeg's dependencies instead

Or I can do it manually zzz

@truboxl truboxl self-requested a review May 26, 2025 08:47
@NORALDM
Copy link
Contributor Author

NORALDM commented May 26, 2025

Splitting the libraries into separate packages was part of my plan but it would require the rpath issue: #24271 to be resolved and I don't have much of an idea of how one could do that other than maintaining a list of CMakeLists that contain targets that need rpath to be set and using sed -i to insert add_linker_options directives after the required target()s, which is somewhat of a messy solution, which is why I was waiting for suggestions on how #24271 could be handled cleanly.

To clarify, the part of #24271 relevant to this PR is that rpath for libcoreclr needs to be set to whatever location contains libraries that dotnet applications can DLLImport ($TERMUX_PREFIX/lib if libSkiaSharp.so, libe_sqlite.so are to be installed globally) because DLLImport is just dlopen internally on Linux.

@truboxl
Copy link
Contributor

truboxl commented May 26, 2025

I felt the urge to diss a long explanation there but ended up not to since I doubt it will be helpful. You can wait infinitely or make do with adjustment here and move on.

@NORALDM
Copy link
Contributor Author

NORALDM commented May 26, 2025

Alright, I'll update the PR with a commit to add rpath to libcoreclr along with commits adding separate packages for skiasharp and sqlitepcl (libe_sqlite is part of sqlitepcl)
But re:

Also do not depend on ffmpeg as you will be pulling 2 copies of ffmpeg,

That's mostly intentional because jellyfin needs a copy of ffmpeg to work properly, and it can use system ffmpeg though jellyfin's ffmpeg has additional features, and putting "jellyfin-ffmpeg | ffmpeg" in TERMUX_PKG_DEPENDS generates a CI error with test-buildorder-random since it's a subpackage.

Copy link
Contributor

@truboxl truboxl left a comment

Choose a reason for hiding this comment

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

Since upstream feels so strongly about using its own jellyfin-ffmpeg instead of system ffmpeg then just use jellyfin-ffmpeg. Preferably prioritise disk space usage than pulling unneccessary packages.

You can workaround the issue like so: (I havent check the dependency list yet)

)
TERMUX_PKG_SUGGESTS="jellyfin-ffmpeg"
TERMUX_PKG_BUILD_DEPENDS="aspnetcore-targeting-pack-8.0, dotnet-targeting-pack-8.0, libcairo, pango, libjpeg-turbo, giflib, librsvg"
TERMUX_PKG_DEPENDS="libc++, fontconfig, aspnetcore-runtime-8.0, dotnet-host, dotnet-runtime-8.0, sqlite, libexpat, libpng, libwebp, freetype, ffmpeg"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
TERMUX_PKG_DEPENDS="libc++, fontconfig, aspnetcore-runtime-8.0, dotnet-host, dotnet-runtime-8.0, sqlite, libexpat, libpng, libwebp, freetype, ffmpeg"
TERMUX_PKG_DEPENDS="libc++, fontconfig, aspnetcore-runtime-8.0, dotnet-host, dotnet-runtime-8.0, sqlite, libexpat, libpng, libwebp, freetype"

@@ -0,0 +1,4 @@
TERMUX_SUBPKG_DESCRIPTION="FFmpeg for Jellyfin with custom extensions and enhancements"
TERMUX_SUBPKG_DEPENDS="chromaprint, libdrm, zstd"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
TERMUX_SUBPKG_DEPENDS="chromaprint, libdrm, zstd"
TERMUX_SUBPKG_DEPENDS="chromaprint, fontconfig, freetype, fribidi, game-music-emu, harfbuzz, libaom, libandroid-glob, libandroid-stub, libass, libbluray, libbz2, libdav1d, libdrm, libgnutls, libiconv, liblzma, libmp3lame, libopencore-amr, libopenmpt, libopus, librav1e, libsoxr, libsrt, libssh, libtheora, libv4l, libvidstab, libvmaf, libvo-amrwbenc, libvorbis, libvpx, libwebp, libx264, libx265, libxml2, libzimg, libzmq, littlecms, ocl-icd, rubberband, svt-av1, xvidcore, zlib, zstd"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add the deps to the subpackage but chromaprint pulls in ffmpeg anyways for FFT

popd
}
termux_step_pre_configure() {
termux_setup_dotnet; termux_setup_gn; termux_setup_nodejs
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
termux_setup_dotnet; termux_setup_gn; termux_setup_nodejs
TERMUX_PKG_DEPENDS+=", jellyfin-ffmpeg"
termux_setup_dotnet; termux_setup_gn; termux_setup_nodejs

@NORALDM
Copy link
Contributor Author

NORALDM commented Jun 4, 2025

Probably out of the scope of this PR, but (aping how Alpine Linux does it) chromaprint could be split into chromaprint and libchromaprint by linking with fftw instead of ffmpeg for the FFT, which would make ffmpeg a dependency of the main chromaprint package only (for fpcalc), and jellyfin-ffmpeg could dep on libchromaprint to avoid pulling in ffmpeg. I'd prefer to do this after this package is merged in.

@NORALDM
Copy link
Contributor Author

NORALDM commented Jun 21, 2025

Woops, didn't get the failed re-rerun in my notifs. I'm going to rebase if that's ok with you @truboxl

@truboxl
Copy link
Contributor

truboxl commented Jun 21, 2025

Please do

@truboxl
Copy link
Contributor

truboxl commented Jul 1, 2025

I will take a look splitting chromaprint

It is still unacceptable large to pull 2 copies of ffmpeg

@NORALDM
Copy link
Contributor Author

NORALDM commented Jul 1, 2025

I will take a look splitting chromaprint

I have done that + split two other packages in https://github.com/NORALDM/termux-packages/tree/split-packages. If you want I can cherry pick the chromaprint split + revbumps into this PR but I feel like it would be suitable for another PR.

@truboxl
Copy link
Contributor

truboxl commented Jul 1, 2025

Ok, you can open a new PR

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.

[Package]: Jellyfin
4 participants