ymartin59 commented 3 years ago

I propose to collaboratively edit this issue description (with comments for discussions about points) so that to prepare how framework has to evolve.

Objectives:

if possible, package should run seamlessly on both DSM 6 and DSM 7
- this is not possible, packages for DSM 6 and DSM 7 are not compatible
- packages for DSM 7 need at least os_min_ver="7.0-40000"
reduce as much as possible package maintenance: implement support in framework make files and generated/embedded scripts

Design proposals:

:heavy_check_mark: At upgrade "var" content is restored as "home" in both DSM 6 and DSM 7
Migrate to the new resource/permission file/specification which help implement common uses cases that otherwise required root Privileges (may not be required see comment, but is something we are working towards)
- :heavy_check_mark: SPK_COMMANDS and SPK_LINKS have to generate resource file for links
- :heavy_check_mark: installer has to generate database resource file from wizard for concerned application
- :x: Use privilege file to add package to a group e.g. sc-media
- :heavy_check_mark: Use privilege file to add create shared folder

To investigate

Privileges: what should be done?
:x: Verify that adding package to a group e.g. sc-media still works from the install script.

Minimal requirements for DSM7

os_min_ver= must be present and the value at least "7.0-40000"
A conf/privilege file must exist (and have "defaults": { "run-as": "package" } [other values not validated yet])

publicarray commented 3 years ago

I've played around a little in my own fork, just to see what breaks and I think I got the minimal requirements done so packages can be "installed" but probably won't work. And binary packages can now be linked to /usr/local using Synology's resource file.

th0ma7 commented 3 years ago

OPTIONAL: There are a few changes I'd like to complete on the framework before jumping on the dsm7 bang-wagon:

Review the DSM vs SRM management by using the new TC_TYPE variable now in place in the toolchain
Review toolchains naming to be singular and use long naming for mk/spksrc.tc*.mk for mk/spksrc.toolchain*.mk
Review the ARM7 exception used for some packages such as with go dependencies in order to have a a generic approach such as ARCH_NO_DUPE or FORCE_ARCHS (help on naming appreciated) pointing towards default available arches allowing to remove dupes for other arches as well as needed and also allow forcing specific sub-arches. This would also allow eliminating newly added syno-apollolake-6.2 in the all-supported builds.
Last but not least, a little more far-stretched, review PLIST management not following regular cookie building makefile layout.

And forgot to mention meson/ninja build support as many packages cannot be further updated anymore...

hgy59 commented 3 years ago

synology has published a beta release of the Development documentation DSM_Developer_Guide_7_0_Beta.pdf. It is newer than the preview version.

One interesting difference I found is for 3rdparty developers:

1. Force lower privilege for package

All packages should provide conf/privilege with package in run-as explicitly. Any privileged operation should be accomplished via resource worker. But if you are 3rdparty developer, you can still use root privilege.

publicarray commented 3 years ago

@ymartin59 Thanks, but why not link to the file? https://global.download.synology.com/download/Document/Software/DeveloperGuide/Firmware/DSM/7.0/enu/DSM_Developer_Guide_7_0_Beta.pdf I don't know but with the current build, when I make a package that asks for root "run-as": "root" I get this:

Failed to install. The package should run with a lower privilege level. Please contact the package developer to modify the privilege settings.

hgy59 commented 3 years ago

@publicarray thanks for the link (comment updated). I think that this will work with DSM 7 beta (we are still on DSM 7 preview).

publicarray commented 3 years ago

That may be true. but I think sooner or later permissions will be more restricted over time, maybe at DSM version 24 ;) Still think it's worth to migrate some of the shell scripts to their resource file equivalent. What do you think?

hgy59 commented 3 years ago

Yes, agree. We should use the official kind of resource scripts whereever provided.

EDIT: the following is proposed already

We should use resource scripts for creating links too. As I introduced SPK_COMMANDS and SPK_LINKS for commandline tools, it would be worth to create the resource scripts based on these (or slightly adjusted) variables.

publicarray commented 3 years ago

resource scripts for creating links too.

Yeah I've worked on that in my dsm7 branch. You can see the result in the synocli-disk GitHub release here. https://github.com/publicarray/spksrc/releases

Agreed. Currently I assume SPK_ COMMANDS are binaries so for other resource types new variables need to be made. Using SPK_COMMANDS would seamlessly work on existing scripts.

I think I'll create PRs once Synology releases the beta and once I'm more confident that it works correctly.

publicarray commented 3 years ago

To maniuplate json I would like to jq as a dependecy to the docker image and spksrc. Why is sed not enough? Once arrays are used it makes code hard to read and error prone, jq ensures that we generate valid json.

e.g. To add a package to the sc-media and sc-example group in the privilege file:

$ jq --arg packagename sc-media '."join-pkg-groupnames" += [{$packagename}]' privilege | jq --arg packagename sc-example '."join-pkg-groupnames" += [{$packagename}]'
{
  "defaults": {
    "run-as": "package"
  },
  "join-pkg-groupnames": [
    {
      "packagename": "sc-media"
    },
    {
      "packagename": "sc-example"
    }
  ]
}

Edit: there are other ways to do this (see below) but the above is easy to use in a loop.

$ jq --arg pkg1 sc-media --arg pkg2 sc-example '."join-pkg-groupnames"+=[{packagename: $pkg1},{packagename: $pkg2}]' privilege
{
  "defaults": {
    "run-as": "package"
  },
  "join-pkg-groupnames": [
    {
      "packagename": "sc-media"
    },
    {
      "packagename": "sc-example"
    }
  ]
}

Edit2: Create the minium needed for a privilege file

$ echo '{}' | jq --arg run_as package '.defaults = {"run-as": $run_as}'
# or
$ echo '{}' | jq --arg key run-as --arg value package '.defaults[$key] = $value'

{
  "defaults": {
    "run-as": "package"
  }
}

I'm happy to work with alterntives too :) Just please don't make use sed for json manipulation 😆

hgy59 commented 3 years ago

@publicarray may you please rebase the dsm7 branch to current head@master? I would like to build some current packages (like ntopng) for my DS218+ that has installed DSM7 preview.

publicarray commented 3 years ago

@hgy59

Yup. It's Done 👍

publicarray commented 3 years ago

DSM7 is now in Beta https://www.synology.com/en-global/beta/DSM70Beta

kalatos86 commented 3 years ago

Is there any information when the community packages might have update to be compatible with DSM 7?

publicarray commented 3 years ago

@kalatos86 Since this is an open source project where people donate their time to contribute I can't give you an ETA. But you are more than welcome to build packages / contribute on the dsm7 branch

publicarray commented 3 years ago

DSM7 and Logs

I propose to change logging in DSM7, if we do nothing some logs will be lost between, package upgrades and install because the target/var folder is not permanent storage. (not a huge issue) Using var aka ${SYNOPKG_PKGVAR}/${SYNOPKG_PKGNAME}.log" fixes the above problem but turns out that the $SYNOPKG_PKGVAR variable does not exist in every stage of the installation cycle thus attempting to write the log file in /${SYNOPKG_PKGNAME}.log which fails in those stages. I think writing to stderr fixes these issues because now Synology is responsible for logging and log rotation (which it does on package reinstall) It also frees up some code in our scripts. Please let me know what you think. @ymartin59 @hgy59 @th0ma7

# /var/package/{package}/target/var/{package.log}.log (Current, main branch)
LOGFILE="${SYNOPKG_PKGDEST}/var/${SYNOPKG_PKGNAME}.log"

# /var/package/{package}/var/{package.log}.log (permanent storage, is in dsm7 branch)
LOGFILE="${SYNOPKG_PKGVAR}/${SYNOPKG_PKGNAME}.log"

# /tmp/synopkgmgr.log-{package} # I no idea how/where to retrieve it, since the file disappears immediately after install.
LOGFILE= ${SYNOPKG_TEMP_LOGFILE}

# /var/log/packages/{package}.log (I think that this is the best option)
echo "$1" 1>&2 #  stderr

# for completeness: show the message to the user
echo "$1" # (during install only, /var/log/packages/{package}.log when service is running)

 # /var/log/messages
logger -p 3 "$1"

th0ma7 commented 3 years ago

Sounds like a good idea but I haven't looked much on that part of the code, neither on the dsm7 update yet. @ymartin59 and @hgy59 might have more to say on this.

publicarray commented 3 years ago

Thanks @th0ma7

While I was looking through some code I found a few packages uses busybox to delete users/groups from what I assume a migration from around DSM5 to DSM6? (can someone confirm that that is the intent?) Since I expect it already hard to migrate settings from DSM6 I want to drop busybox completely in DSM7 and not support package migration from DSM5 to DSM7, (IMHO It doesn't make sense to do so anyway)

publicarray commented 3 years ago

@ymartin59 @hgy59 What arches should we build for DSM7.0? I think this should cover everything:

make -j`nproc` arch-x64-7.0 arch-armv7-7.0 arch-aarch64-7.0 arch-evansport-7.0

hgy59 commented 3 years ago

@ymartin59 @hgy59 What arches should we build for DSM7.0? I think this should cover everything:
make -j`nproc` arch-x64-7.0 arch-armv7-7.0 arch-aarch64-7.0 arch-evansport-7.0

Yes, only these four generic archs are supported by current models with DSM 7 support

Detailed List	Arch	Generic Arch
alpine	armv7
apollolake	x64
armada370	armv7
armada375	armv7
armada37xx	aarch64
armada38x	armv7
armadaxp	armv7
avoton	x64
braswell	x64
broadwell	x64
broadwellnk	x64
broadwellntbap
bromlow	x64
cedarview	x64
comcerto2k	armv7
denverton	x64
evansport	i686
geminilake	x64
grantley	x64
monaco	armv7
purley	x64
rtd1296	aarch64
v1000	x64

publicarray commented 3 years ago

Thanks! @hgy59 For testing I've build some packages and uploaded them here: https://seby.io/download/synology-7.0 Let me know what you think. Some install wizards still need an updated permission/migration message.

NOTE the files are of alpha quality. Use them at your own risk.

yasars commented 3 years ago

@publicarray publicarray

my friend can you make an test from ffmpeg for apollolake (ds218+) ?? i woult test it :)

tianchaomaker commented 3 years ago

403: Forbidden

Unauthorized IP Address.

Either disable the IP address whitelist or add your address to it.

If you're editing settings.json, see the 'rpc-whitelist' and 'rpc-whitelist-enabled' entries.

If you're still using ACLs, use a whitelist instead. See the transmission-daemon manpage for details.

---Original--- From: "Sebastian Schmidt"<notifications@github.com> Date: Sat, Jan 2, 2021 23:15 PM To: "SynoCommunity/spksrc"<spksrc@noreply.github.com>; Cc: "Manual"<manual@noreply.github.com>;"天朝maker"<563716438@qq.com>; Subject: Re: [SynoCommunity/spksrc] DSM 7 support framework design (#4215)

Thanks! @hgy59 For testing I've build some packages and uploaded them here: https://seby.io/download/synology-7.0 Let me know what you think. Some install wizards still need an updated permission/migration message.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

publicarray commented 3 years ago

@yasars Sorry, I'm not taking request at the moment. I'm testing packages that are small and use different features of the framework. You are welcome to compile the packages you need by cloning the repository and building them yourself. Or wait until DSM7 support in is mature enough that we can can publish packages. I actually tried compiling ffmpeg but we are not there yet:

In file included from /spksrc/toolchain/syno-x64-7.0/work/x86_64-pc-linux-gnu/x86_64-pc-linux-gnu/include/c++/7.3.0/cmath:45:0,
                 from /spksrc/spk/ffmpeg/work-x64-7.0/libaom-gitbb35ba9148543f22ba7d8642e4fbd29ae301f5dc/third_party/libwebm/mkvparser/mkvparser.cc:18:
/spksrc/spk/ffmpeg/work-x64-7.0/libaom-gitbb35ba9148543f22ba7d8642e4fbd29ae301f5dc/third_party/libwebm/mkvparser/mkvparser.cc: In function ‘bool mkvparser::__builtin_isnan(double)’:
/spksrc/spk/ffmpeg/work-x64-7.0/libaom-gitbb35ba9148543f22ba7d8642e4fbd29ae301f5dc/third_party/libwebm/mkvparser/mkvparser.cc:35:45: error: ‘__builtin_isnan’ is not a member of ‘std’
 inline bool isnan(double val) { return std::isnan(val); }
                                             ^
/spksrc/spk/ffmpeg/work-x64-7.0/libaom-gitbb35ba9148543f22ba7d8642e4fbd29ae301f5dc/third_party/libwebm/mkvparser/mkvparser.cc:35:45: note: suggested alternatives:
<built-in>: note:   ‘__builtin_isnan’
/spksrc/spk/ffmpeg/work-x64-7.0/libaom-gitbb35ba9148543f22ba7d8642e4fbd29ae301f5dc/third_party/libwebm/mkvparser/mkvparser.cc:35:13: note:   ‘mkvparser::__builtin_isnan’
 inline bool isnan(double val) { return std::isnan(val); }
             ^
/spksrc/spk/ffmpeg/work-x64-7.0/libaom-gitbb35ba9148543f22ba7d8642e4fbd29ae301f5dc/third_party/libwebm/mkvparser/mkvparser.cc: In function ‘bool mkvparser::__builtin_isinf_sign(double)’:
/spksrc/spk/ffmpeg/work-x64-7.0/libaom-gitbb35ba9148543f22ba7d8642e4fbd29ae301f5dc/third_party/libwebm/mkvparser/mkvparser.cc:36:45: error: ‘__builtin_isinf_sign’ is not a member of ‘std’
 inline bool isinf(double val) { return std::isinf(val); }
                                             ^
/spksrc/spk/ffmpeg/work-x64-7.0/libaom-gitbb35ba9148543f22ba7d8642e4fbd29ae301f5dc/third_party/libwebm/mkvparser/mkvparser.cc:36:45: note: suggested alternatives:
<built-in>: note:   ‘__builtin_isinf_sign’
/spksrc/spk/ffmpeg/work-x64-7.0/libaom-gitbb35ba9148543f22ba7d8642e4fbd29ae301f5dc/third_party/libwebm/mkvparser/mkvparser.cc:36:13: note:   ‘mkvparser::__builtin_isinf_sign’
 inline bool isinf(double val) { return std::isinf(val); }
             ^
make[6]: *** [CMakeFiles/webm.dir/build.make:115: CMakeFiles/webm.dir/third_party/libwebm/mkvparser/mkvparser.cc.o] Error 1

@tianchaomaker Can you share your settings.json if it exists? It's in /var/packages/transmission/var/settings.json

@hgy59 For noarch packages like syno-magnet, DSM7 requires the OS_MIN_VER to be set What would be the best way to address this?

# Pure web package, make sure ARCH is not defined
override ARCH=
OS_MIN_VER = 7.0-4000

hgy59 commented 3 years ago

@hgy59 For noarch packages like syno-magnet, DSM7 requires the OS_MIN_VER to be set What would be the best way to address this?

As we will be able to create noarch packages for DSM 6.x (or 5.2) and DSM 7.x without modification of source code (=Makefile) I would prefer a solution that looks like make noarch-6.1 or make noarch-7.0

publicarray commented 3 years ago

@hgy59 I was thinking along similar lines.

publicarray commented 3 years ago

but I don't have any "good" ideas on how to implement that.

tianchaomaker commented 3 years ago

{ "download-dir": "@download_dir@", "incomplete-dir": "@incomplete_dir@", "incomplete-dir-enabled": @incomplete_dir_enabled@, "rpc-authentication-required": true, "rpc-username": "@username@", "rpc-password": "@password@", "rpc-whitelist-enabled": false, "umask": 2, "watch-dir": "@watch_dir@", "watch-dir-enabled": @watch_dir_enabled@ }

------------------ 原始邮件 ------------------ 发件人: "SynoCommunity/spksrc" <notifications@github.com>; 发送时间: 2021年1月3日(星期天) 上午7:48 收件人: "SynoCommunity/spksrc"<spksrc@noreply.github.com>; 抄送: "ゅ進@￠麗§"<563716438@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [SynoCommunity/spksrc] DSM 7 support framework design (#4215)

@yasars Sorry, I'm not taking request at the moment. I'm testing packages that are small and use different features of the framework.

@tianchaomaker Can you share your settings.json if it exists? It's in /var/packages/transmission/var/settings.json

@hgy59

For noarch packages like syno-magnet, DSM7 requires the OS_MIN_VER to be set What would be the best way to address this?

Pure web package, make sure ARCH is not defined override ARCH= OS_MIN_VER = 7.0-4000

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

publicarray commented 3 years ago

Thanks @tianchaomaker Can you download the package again. I've made some changes to the install script. It looks like the wizard settings where not applied. So a clean reinstall is required.

hgy59 commented 3 years ago

@publicarray Thank you for the noarch implementation I have built adminer for DSM7 and it works for MySQL database (DS-218+, apollolake, x64)

tianchaomaker commented 3 years ago

403: Forbidden Unauthorized IP Address.

Either disable the IP address whitelist or add your address to it.

If you're editing settings.json, see the 'rpc-whitelist' and 'rpc-whitelist-enabled' entries.

If you're still using ACLs, use a whitelist instead. See the transmission-daemon manpage for details.

---Original--- From: "Sebastian Schmidt"<notifications@github.com> Date: Mon, Jan 4, 2021 04:49 AM To: "SynoCommunity/spksrc"<spksrc@noreply.github.com>; Cc: "Mention"<mention@noreply.github.com>;"天朝maker"<563716438@qq.com>; Subject: Re: [SynoCommunity/spksrc] DSM 7 support framework design (#4215)

Thanks @tianchanomaker Can you download the package again. I've made some changes to the install script. It looks like the wizard settings where not applied. So a clean reinstall is required.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

kalatos86 commented 3 years ago

If there will be any option to test the packages you develop please let me know. I am fully available in order to test on my DSM.

Thanks,

Lukasz

W dniu 4 stycznia 2021 0:36 użytkownik 天朝maker notifications@github.com napisał:

403: Forbidden Unauthorized IP Address.

Either disable the IP address whitelist or add your address to it.

If you're editing settings.json, see the 'rpc-whitelist' and 'rpc-whitelist-enabled' entries.

If you're still using ACLs, use a whitelist instead. See the transmission-daemon manpage for details.

---Original--- From: "Sebastian Schmidt"<notifications@github.com> Date: Mon, Jan 4, 2021 04:49 AM To: "SynoCommunity/spksrc"<spksrc@noreply.github.com>; Cc: "Mention"<mention@noreply.github.com>;"天朝maker"<563716438@qq.com>; Subject: Re: [SynoCommunity/spksrc] DSM 7 support framework design (#4215)

Thanks @tianchanomaker Can you download the package again. I've made some changes to the install script. It looks like the wizard settings where not applied. So a clean reinstall is required.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

publicarray commented 3 years ago

@hgy59 When I tried Adminer the scrips did not have permissions to to write to the /web shared folder. So I used Synology's resource worker instead. Would you be willing to test that version? Thanks

svyaznoy362 commented 3 years ago

Where can I download transmission for dsm 7

kalatos86 commented 3 years ago

Same question here. Is there any build which we can test ?

Wysłane z iPhone'a

Wiadomość napisana przez svyaznoy362 notifications@github.com w dniu 08.01.2021, o godz. 15:18:

Where can I download transmission for dsm 7

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

hgy59 commented 3 years ago

@svyaznoy362 @kalatos86 see above

publicarray commented 3 years ago

I've started documenting some of what has been done into the wiki: DSM-7.0-Beta

th0ma7 commented 3 years ago

From your howto guide @publicarray (nice work btw) I noticed that chmod, chown are somewhat unavailable now. A few questions comes to mind for my use cases related to hardware acceleration with ffmpeg:

What is the permission schema on the device file now on DSM7 with recent intel CPU (kernel 4.4+)? On DSM6 it is the following:
```
$ ll /dev/dri/renderD128 
crw------- 1 root root 226, 128 Dec 28 08:35 /dev/dri/renderD128
```
Can they be ran at startup time by the application user?
Could we grant access to sc-media user account (or create a new account for that specifically)? If so, permissions on vaapi and ffmpeg could then be group=sc-media. But then again that would require set-uid-bit which in turns requires chmod access.

th0ma7 commented 3 years ago

Also, I was thinking of doing a new round of updates to ffmpeg and tvheadend but at this point I'd rather also look into migrating things over to DSM7. Should I work on the dsm7 branch only then? Or are we planning to merge the branch over to master?

publicarray commented 3 years ago

Thanks @th0ma7

$ ls -l /dev/dri/renderD128 
crw-rw---- 1 root videodriver 226, 128 Jan 10 07:31 /dev/dri/renderD128

# note that i have plex installed:

$ synouser --get PlexMediaServer
User Name   : [PlexMediaServer]
User Type   : [AUTH_LOCAL]
User uid    : [297536]
Primary gid : [297536]
Fullname    : []
User Dir    : [/var/packages/PlexMediaServer/home]
User Shell  : [/sbin/nologin]
Expired     : [false]
User Mail   : []
Alloc Size  : [130]
Member Of   : [2]
(937) videodriver
(297536) PlexMediaServer

I propose to do whatever plex is doing and add ffmpeg to the group.

Hm, I have not seen this documented anywhere:

$ cat /var/packages/PlexMediaServer/conf/resource
{"video-driver":{}}

If you want to develop for DSM6 I recommend staying on master otherwise dms7 branch is the way to go. Occasionally I rebase master to dsm6 so that it is easier to merge it in later, plus I get fixes from master. Therefore, an occasional force push is to be expected on the dsm7 branch.

I've already managed to compile ffmpeg with this change https://github.com/SynoCommunity/spksrc/commit/bdaa332916e5c54f20a997ccc3bb37be61a044b3, Now I'm testing the resource file from Plex.

Personally I would like to see the .NET framework makefiles reviewed and merged #4207 before we merge dsm7.

Or are we planning to merge the branch over to master?

Yea see https://github.com/SynoCommunity/spksrc/pull/4373#issuecomment-756927643

publicarray commented 3 years ago

@th0ma7 looks like it worked:

$ synogroup --get videodriver
Group Name: [videodriver]
Group Type: [AUTH_LOCAL]
Group ID:   [937]
Group Members: 
0:[PlexMediaServer]
1:[sc-ffmpeg]
$ synouser --get sc-ffmpeg
User Name   : [sc-ffmpeg]
User Type   : [AUTH_LOCAL]
User uid    : [110454]
Primary gid : [110454]
Fullname    : []
User Dir    : [/var/packages/ffmpeg/home]
User Shell  : [/sbin/nologin]
Expired     : [false]
User Mail   : []
Alloc Size  : [115]
Member Of   : [2]
(110454) sc-ffmpeg
(937) videodriver

th0ma7 commented 3 years ago

looks like it worked:

$ synogroup --get videodriver
Group Name: [videodriver]
Group Type: [AUTH_LOCAL]
Group ID:   [937]
Group Members: 
0:[PlexMediaServer]
1:[sc-ffmpeg]
$ synouser --get sc-ffmpeg
User Name   : [sc-ffmpeg]
User Type   : [AUTH_LOCAL]
User uid    : [110454]
Primary gid : [110454]
Fullname    : []
User Dir    : [/var/packages/ffmpeg/home]
User Shell  : [/sbin/nologin]
Expired     : [false]
User Mail   : []
Alloc Size  : [115]
Member Of   : [2]
(110454) sc-ffmpeg
(937) videodriver

Good thing that this is working but ffmpeg has no need to be included in the group. It's rather users who need to use ffmpeg that will require to be in videodriver group such as tvheadend, comskip and chromaprint. Quick question, with DSM7 do we have to enforce user and group for no-deamon tools such as ffmpeg or synocli-* or do files are kept being owned by root?

Another avenue currently included in ffmpeg is to set-gid-bit on both bin/ffmpeg and bin/vainfo. If we could manage to do that on the group only would be quite nice but this would require a:

$ chgrp videodriver ffmpeg vainfo
$ chmod g+s ffmpeg vainfo
$ ls -la ffmpeg vainfo 
-rwxr-sr-x 1 root videodriver 266760 Oct 18 09:58 ffmpeg
-rwxr-sr-x 1 root videodriver  21784 Oct 18 09:58 vainfo

publicarray commented 3 years ago

True, but I think it's good to add ffmpeg as well, good for testing and using the command line. Files are always owned by the package, Synology's default is the package name but I've added the sc- prefix for consistency and backwards compatibility. chown -hR "${package}:${package}" is run by synology during package extraction.

Nice indeed, If you like you can search for tool (optional) in the development guide. But in the meantime I don't mind setting the video-driver resource for the needed packages.

ymartin59 commented 3 years ago

@th0ma7 I do not catch the point of adding g+s flag on ffmpeg and vainfo binaries. If I remember well, this flag only allows code to invoke libc method to switch process group id to executable file group gid. So as far as there is no usage of concerned method in code, flag is useless. But I may be wrong...

th0ma7 commented 3 years ago

@ymartin59 situation is rather simple... In DSM6 the device file for video acceleration is restricted access only to root user and no one else:

$ ll /dev/dri/renderD128 
crw------- 1 root root 226, 128 Dec 28 08:35 /dev/dri/renderD128

That mean that in order to get access to it you must be root, period. To that effect currently ffmpeg and vainfo allows you to become root user for the duration of their execution through setuid:

$ ls -la ffmpeg vainfo 
-rwsr-xr-x 1 root root 266760 Oct 18 09:58 ffmpeg
-rwsr-xr-x 1 root root  21784 Oct 18 09:58 vainfo

With that in mind, in DSM7 the video acceleration device file is now owned by user root and group videodriver. This mean that any user who wishes to make usage of video acceleration through the GPU must be in the videodriver group or be root user:

$ ls -l /dev/dri/renderD128 
crw-rw---- 1 root videodriver 226, 128 Jan 10 07:31 /dev/dri/renderD128

So anyone needing to use ffmpeg and therefore use GPU video decoding acceleration needs either to:

be root
or be part of the videodriver group
or be allowed to run as... either using setuid (insecure) or setgid (much more secure)
or run as using sudo

The simplest and most secure solution covers most common use cases for background daemons by simply adding them to the videodriver group, problem solved (e.g. tvheadend)

What's left is the other type of use cases, potentially scheduled tasks for chromaprint and comskip. For the sake of simplicity, if your local user (lets pretend admin) is not part of the videodriver group, you will have to run $ sudo ... ffmpeg -option abc to get GPU acceleration running OR the ffmpeg binary will require to provide setgid access (chmod g+s) so you user "gain" access to the videodriver group when executing the binary in question. The nice thing about the setgid is that it solves all cases without needing to play with groups at all, to the detriment of lowering the security a bit (note that setuid and become root is far less secure).

Long story short:

Adding "daemon" type users needing to access to ffmpeg to the videodriver group should work out really well and cover most use-cases.
Considering the inability to use chmod in DSM7 and thus to provide setgid, any other manually called or scheduled task accessing ffmpeg will have to be run from a user that is into the videodriver group.

Lastly, creating a user sc-ffmpeg included into the videodriver group is sort of useless. The only use-case is if you intend to make usage of that user to run manual jobs or scheduled tasks which is probably nil.

EDIT: And this is only needed currently for x86_64 intel platform (although AMD V1000 might have something of that matter as well, to be confirmed).

th0ma7 commented 3 years ago

Question is: should we add a new flag such as VIDEODRIVER = true (or VIDEOACCEL or whatever make sense) so the created sc-[something] user gets automagically added to the videodriver group?

publicarray commented 3 years ago

@th0ma7 thanks for the detailed explanation!

Lastly, creating a user sc-ffmpeg included into the videodriver group is sort of useless.

Another usecase as a developer is to verify ffmpeg commands, e.g from Jellyfin log files. So I disagree that's useless. But agree that most people probably won't use it. After all most laptops have much more powerful CPUs and GPUs.

Agree we could add another variable (maybe VIDEO_HW_ACCELERATION or VAAPI?), but you're only saving 1 line.

th0ma7 commented 3 years ago

@publicarray indeed you got a good point, for developers and testing. On the one-liner aspect, this ensure we always use the same method. As for the naming... we can always change it afterwards.

Further thinking about this, we could create a VIDEO_HW_ACCELERATION_ARCHS in order to include only archs that do have hardware acceleration. That variable can later be used to build (or not) the Intel video driver (and potentially AMD eventually) and be used as a condition prior to adding the user in the videodriver group.

hgy59 commented 3 years ago

@publicarray just reworked the noarch support according to #4392 and improved the build with different work dirs.

to build previous versions (as before) just call make or with any TCVERSION < 7.0 make TCVERSION=6.0

To build for DSM7 call make TCVERSION=7.0

publicarray commented 3 years ago

Thanks, I've updated the wiki

th0ma7 commented 3 years ago

@publicarray you did a nice job with the wiki. For the noarch, shouldn't the TCVERSION=6.1 instead of 6.0 ?

SynoCommunity / spksrc

DSM 7 support framework design #4215

1. Force lower privilege for package

DSM7 and Logs

Pure web package, make sure ARCH is not defined override ARCH= OS_MIN_VER = 7.0-4000