-
Notifications
You must be signed in to change notification settings - Fork 8
MSYS2 build of SVT JPEG XS codec #8
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
base: main
Are you sure you want to change the base?
Conversation
jxs-test-files/200.jxs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this file jxs-test-files/200.jxs
Nothing special, I simply documented my workflow as it was saved in the
console stack. You are right, the use of Build/linux/build.sh <release |
debug> is more clear and straightforward. Should I edit my text per
your comment or would you please do it?
…On Tue, Mar 18, 2025 at 5:30 PM Tomasz ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In README.md
<#8 (comment)>
:
> + - `cmake -S . -B builddebug -DCMAKE_BUILD_TYPE=Debug` OR
+ - `cmake -S . -B buildrelease -DCMAKE_BUILD_TYPE=Release` OR
Why do you removed simplistic and easy to use
cd Build/linux
./build.sh <release | debug>
?
—
Reply to this email directly, view it on GitHub
<#8 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKGYDJQ5UB4P67MIBWL2U7YUBAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMOJTHA3TKNBXHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Please do it |
…d the related lines in README
Please remove this file jxs-test-files/200.jxs,
Please do it (reverse edit on build.sh)
Ok, done
…On Tue, Mar 18, 2025 at 5:38 PM Tomasz ***@***.***> wrote:
Nothing special, I simply documented my workflow as it was saved in the
console stack. You are right, the use of Build/linux/build.sh <release |
debug> is more clear and straightforward. Should I edit my text per your
comment or would you please do it?
Please do it
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKCIGKC6GNZ7VE2Z2N32U7ZR7AVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZSGYYTCOJWG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
[image: tszumski]*tszumski* left a comment (OpenVisualCloud/SVT-JPEG-XS#8)
<#8 (comment)>
Nothing special, I simply documented my workflow as it was saved in the
console stack. You are right, the use of Build/linux/build.sh <release |
debug> is more clear and straightforward. Should I edit my text per your
comment or would you please do it?
Please do it
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKCIGKC6GNZ7VE2Z2N32U7ZR7AVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZSGYYTCOJWG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
%elifidn __OUTPUT_FORMAT__,win64 | ||
%define WIN64 1 | ||
%elifidn __OUTPUT_FORMAT__,x64 | ||
%define WIN64 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for modifying/commenting-out x64inc.asm ? Is it necessary?
CMakeLists.txt of your repo DEFINEs symbols WIN64 [line 105:
set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DWIN64")] and UNIX64
[line 107: set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DUNIX64")].
Notice that the very first statements in Source/Lib/ASM_SSE2/x64inc.asm
starts UNDEFINE these symbols, in line 12: %undef WIN64 and in line 13:
%undef UNIX64. After that, one of these symbols is defined anew, depending
on the *value *of variable __OUTPUT_FORMAT__.
Now, __OUTPUT_FORMAT__ in x64inc.asm is defined by a *host *environment
(where the compiler and linker work)) and not by a *target *environment
(where the code will be used). It may seem unusual, but MSYS2 build system
should be considered a 'cross compiler' even when MSYS2 builds for the
Windows platform.
And while this technique (cross compilation) may appear to be imposing
unnecessary complications, it is no more nuisance than the existence of
multiple platforms and architectures (UNIX, Windows, ARM64) per se. Think
of the development tools for embedded systems, they inherently require
'cross-compilers'.
Back to my edit of x64inc.asm: instead of unearthing CMake-related,
assembler, or other variables suitable for passing the target environment
variables to preprocessor and on, I assigned the symbols WIN64 and UNIX64
their values in the root CMakeLists.txt, like set(CMAKE_ASM_NASM_FLAGS
"${CMAKE_ASM_NASM_FLAGS} -DWIN64=1 -DUNIX64=0") in one branch and switching
1 and 0 in another. In fact, I brought back the intentions of original
developers of this *.asm file: now you need not undefine the symbols, you
just use these as they are passed from CMakeLists.txt. Only do not forget
to replace ifdef-s with ifidn-s for these symbols.
It reminds me of your original question about 'benefits of cross-compilers
(like MSYS) vs. native tools (like MSVC for Windows, gnu-compilers for
NIX-es): indeed, there exists a real benefit of building with two different
build systems for the same platform. You see how this procedure can reveal
some intricate details of code behaviour and strengthen the code health.
…On Wed, Mar 19, 2025 at 2:07 PM Tomasz ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In Source/Lib/Common/ASM_SSE2/x64inc.asm
<#8 (comment)>
:
> @@ -9,19 +9,19 @@
; PATENTS file, you can obtain it at https://www.aomedia.org/license/patent-license.
;
-%undef WIN64
-%undef UNIX64
-%ifidn __OUTPUT_FORMAT__,win32
- %define WIN64 1
-%elifidn __OUTPUT_FORMAT__,win64
- %define WIN64 1
-%elifidn __OUTPUT_FORMAT__,x64
- %define WIN64 1
What is the reason for modifying/commenting-out x64inc.asm ?
—
Reply to this email directly, view it on GitHub
<#8 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKAAN3DO3PEHEVSEQMT2VEJTBAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMOJXGE3DANBVG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Thank you for contribution and detailed explanation, Let me test your changes(it may take a few days) |
Ok. Meanfile, I'm writing a section on FFMPEG installation to be used with
the repo's ffmpeg-plugin.
As an aside: Reading my explanation now, I have to admit that in fact I do
not explain what preprocessor actually does with the symbols in the *.asm
file, which are defined in CMakeLists.txt. I simply indicate that something
went wrong and that the developer decided to undefine these symbols and
define/assign these anew using the value of __OUTPUT_FORMAT__. Handling of
symbols in preprocessor and compiler is a delicate matter. See, for
example, Why symbols in assembly language are used before they are defined?
- Stack Overflow
<https://stackoverflow.com/questions/42711307/why-symbols-in-assembly-language-are-used-before-they-are-defined>
.
Anyway, not only defining, but also assigning the symbol a value (like
-DWIN64=1) definitely solves the problem.
…On Wed, Mar 19, 2025 at 8:13 PM Tomasz ***@***.***> wrote:
CMakeLists.txt of your repo DEFINEs symbols WIN64 [line 105:
set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DWIN64")] and UNIX64
[line 107: set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DUNIX64")].
Notice that the very first statements in Source/Lib/ASM_SSE2/x64inc.asm
starts UNDEFINE these symbols, in line 12: %undef WIN64 and in line 13:
%undef UNIX64. After that, one of these symbols is defined anew, depending
on the
*value of variable OUTPUT_FORMAT. Now, OUTPUT_FORMAT in x64inc.asm is
defined by a host environment (where the compiler and linker work)) and not
by a target environment (where the code will be used). It may seem unusual,
but MSYS2 build system should be considered a 'cross compiler' even when
MSYS2 builds for the Windows platform. And while this technique (cross
compilation) may appear to be imposing unnecessary complications, it is no
more nuisance than the existence of multiple platforms and architectures
(UNIX, Windows, ARM64) per se. Think of the development tools for embedded
systems, they inherently require 'cross-compilers'. Back to my edit of
x64inc.asm: instead of unearthing CMake-related, assembler, or other
variables suitable for passing the target environment variables to
preprocessor and on, I assigned the symbols WIN64 and UNIX64 their values
in the root CMakeLists.txt, like set(CMAKE_ASM_NASM_FLAGS
"${CMAKE_ASM_NASM_FLAGS} -DWIN64=1 -DUNIX64=0") in one branch and switching
1 and 0 in another. In fact, I brought back the intentions of original
developers of this .asm file: now you need not undefine the symbols, you
just use these as they are passed from CMakeLists.txt. Only do not forget
to replace ifdef-s with ifidn-s for these symbols. It reminds me of your
original question about 'benefits of cross-compilers (like MSYS) vs. native
tools (like MSVC for Windows, gnu-compilers for NIX-es): indeed, there
exists a real benefit of building with two different build systems for the
same platform. You see how this procedure can reveal some intricate details
of code behaviour and strengthen the code health. …
<#m_4689762278666943266_> On Wed, Mar 19, 2025 at 2:07 PM Tomasz @.> wrote:
@.* commented on this pull request. ------------------------------ In
Source/Lib/Common/ASM_SSE2/x64inc.asm <#8 (comment)
<#8 (comment)>>
: > @@ -9,19 +9,19 @@ ; PATENTS file, you can obtain it at
https://www.aomedia.org/license/patent-license. ; -%undef WIN64 -%undef
UNIX64 -%ifidn *OUTPUT_FORMAT*,win32 - %define WIN64 1 -%elifidn
*OUTPUT_FORMAT*,win64 - %define WIN64 1 -%elifidn *OUTPUT_FORMAT*,x64 -
%define WIN64 1 What is the reason for modifying/commenting-out x64inc.asm
? — Reply to this email directly, view it on GitHub <#8 (review)
<#8 (review)>>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AGT2SKAAN3DO3PEHEVSEQMT2VEJTBAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMOJXGE3DANBVG4
. You are receiving this because you authored the thread.Message ID: *@*
.***>
Thank you for contribution and detailed explanation, Let me test your
changes(it may take a few days)
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKE56UROEP6ZI64SEX32VFUOPAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZWGU4TGMBVGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
[image: tszumski]*tszumski* left a comment (OpenVisualCloud/SVT-JPEG-XS#8)
<#8 (comment)>
CMakeLists.txt of your repo DEFINEs symbols WIN64 [line 105:
set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DWIN64")] and UNIX64
[line 107: set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DUNIX64")].
Notice that the very first statements in Source/Lib/ASM_SSE2/x64inc.asm
starts UNDEFINE these symbols, in line 12: %undef WIN64 and in line 13:
%undef UNIX64. After that, one of these symbols is defined anew, depending
on the
*value of variable OUTPUT_FORMAT. Now, OUTPUT_FORMAT in x64inc.asm is
defined by a host environment (where the compiler and linker work)) and not
by a target environment (where the code will be used). It may seem unusual,
but MSYS2 build system should be considered a 'cross compiler' even when
MSYS2 builds for the Windows platform. And while this technique (cross
compilation) may appear to be imposing unnecessary complications, it is no
more nuisance than the existence of multiple platforms and architectures
(UNIX, Windows, ARM64) per se. Think of the development tools for embedded
systems, they inherently require 'cross-compilers'. Back to my edit of
x64inc.asm: instead of unearthing CMake-related, assembler, or other
variables suitable for passing the target environment variables to
preprocessor and on, I assigned the symbols WIN64 and UNIX64 their values
in the root CMakeLists.txt, like set(CMAKE_ASM_NASM_FLAGS
"${CMAKE_ASM_NASM_FLAGS} -DWIN64=1 -DUNIX64=0") in one branch and switching
1 and 0 in another. In fact, I brought back the intentions of original
developers of this .asm file: now you need not undefine the symbols, you
just use these as they are passed from CMakeLists.txt. Only do not forget
to replace ifdef-s with ifidn-s for these symbols. It reminds me of your
original question about 'benefits of cross-compilers (like MSYS) vs. native
tools (like MSVC for Windows, gnu-compilers for NIX-es): indeed, there
exists a real benefit of building with two different build systems for the
same platform. You see how this procedure can reveal some intricate details
of code behaviour and strengthen the code health. …
<#m_4689762278666943266_> On Wed, Mar 19, 2025 at 2:07 PM Tomasz @.> wrote:
@.* commented on this pull request. ------------------------------ In
Source/Lib/Common/ASM_SSE2/x64inc.asm <#8 (comment)
<#8 (comment)>>
: > @@ -9,19 +9,19 @@ ; PATENTS file, you can obtain it at
https://www.aomedia.org/license/patent-license. ; -%undef WIN64 -%undef
UNIX64 -%ifidn *OUTPUT_FORMAT*,win32 - %define WIN64 1 -%elifidn
*OUTPUT_FORMAT*,win64 - %define WIN64 1 -%elifidn *OUTPUT_FORMAT*,x64 -
%define WIN64 1 What is the reason for modifying/commenting-out x64inc.asm
? — Reply to this email directly, view it on GitHub <#8 (review)
<#8 (review)>>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AGT2SKAAN3DO3PEHEVSEQMT2VEJTBAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMOJXGE3DANBVG4
. You are receiving this because you authored the thread.Message ID: *@*
.***>
Thank you for contribution and detailed explanation, Let me test your
changes(it may take a few days)
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKE56UROEP6ZI64SEX32VFUOPAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZWGU4TGMBVGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
README.md
Outdated
- __You are ready to build the SVT-JPEG-XS package__ | ||
- `git clone https://github.com/OpenVisualCloud/SVT-JPEG-XS.git` | ||
- `cd SVT-JPEG-XS` | ||
- `cd Build/linux/build.sh <release | debug>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cd Build/linux && build.sh <release | debug>
Added (maybe excessively detailed) readme-s. The gist for building with universal c runtime in MSYS2 is to always work with UCRT64 terminal and install only packets and tools from mingw-w64-ucrt-x86_64 'namespace', the only exception maybe is the 'git' tool. In doing so, you'll never receive messages like 'Native MSYS builds are discouraged, please use the MINGW environment'. Added the folder ffmpeg_enable-libsvtjpegxs explaining MSYS2- and WDL2-builds plus data for FFMPEG plugin |
Hi @vasilich-tregub , unfortunately I cannot accept whole ffmpeg repository here (folder ffmpeg_enable-libsvtjpegxs) |
…eplaced my fancy ci/cd instructions with standard ci/cd patch instructions of ffmpeg-plugin
Hi @tszumski, moved readme.md of ffmpeg_enable-libsvtjpegxs to wsl2msys2.md
in ffmpeg-plugin, replaced my fancy ci/cd instructions for patching ffmped
with standard ci/cd patch instructions of ffmpeg-plugin.
DELETED ffmpeg_enable-libsvtjpegxs entirely
…On Thu, Mar 27, 2025 at 5:39 PM Tomasz ***@***.***> wrote:
Hi @vasilich-tregub <https://github.com/vasilich-tregub> , unfortunately
I cannot accept whole ffmpeg repository here (folder
ffmpeg_enable-libsvtjpegxs)
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKGRNZJLJWWKBLSX3GT2WPBMXAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJXGU2TMMBTGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
[image: tszumski]*tszumski* left a comment (OpenVisualCloud/SVT-JPEG-XS#8)
<#8 (comment)>
Hi @vasilich-tregub <https://github.com/vasilich-tregub> , unfortunately
I cannot accept whole ffmpeg repository here (folder
ffmpeg_enable-libsvtjpegxs)
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKGRNZJLJWWKBLSX3GT2WPBMXAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJXGU2TMMBTGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Also, I did not remove '#Windows ffmpeg plugin' section from
ffmpeg-plugin's readme, which instructs to use MINGW64 terminal, because I
do not think that this necessarily results in bug; maybe instructions need
more details to succeed, but mingw64 certainly can be used to build
executables. I did not go this route because (beginning 2023) both
Microsoft and MSYS2 insist on using the universal c runtime for Windows
native builds. You, the maintainer, can edit the readme as you see fit. Of
course, I am ready to edit the document on my own, according to your
instruction.
On Fri, Mar 28, 2025 at 11:32 AM Valdemar Svoboda ***@***.***>
wrote:
… Hi @tszumski, moved readme.md of ffmpeg_enable-libsvtjpegxs
to wsl2msys2.md in ffmpeg-plugin, replaced my fancy ci/cd instructions for
patching ffmped with standard ci/cd patch instructions of ffmpeg-plugin.
DELETED ffmpeg_enable-libsvtjpegxs entirely
On Thu, Mar 27, 2025 at 5:39 PM Tomasz ***@***.***> wrote:
> Hi @vasilich-tregub <https://github.com/vasilich-tregub> , unfortunately
> I cannot accept whole ffmpeg repository here (folder
> ffmpeg_enable-libsvtjpegxs)
>
> —
> Reply to this email directly, view it on GitHub
> <#8 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGT2SKGRNZJLJWWKBLSX3GT2WPBMXAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJXGU2TMMBTGE>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
> [image: tszumski]*tszumski* left a comment
> (OpenVisualCloud/SVT-JPEG-XS#8)
> <#8 (comment)>
>
> Hi @vasilich-tregub <https://github.com/vasilich-tregub> , unfortunately
> I cannot accept whole ffmpeg repository here (folder
> ffmpeg_enable-libsvtjpegxs)
>
> —
> Reply to this email directly, view it on GitHub
> <#8 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGT2SKGRNZJLJWWKBLSX3GT2WPBMXAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJXGU2TMMBTGE>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
Thank you @vasilich-tregub for your contribution, readme/instruction looks fine, just one more question is CMakeSettings.json file required ? I do not see any reference in readme to that file |
Sorry for CMakeSettings.json intrusion. I'd believed that .gitignore would
have to suppress it, but it does not. It is important when working with
MSVC, but is not required in repo, as MSVC generates it when the user
selects configuration (or something like this, I believe). And, of course,
it is not required for MSYS2 build, I believe. I've just deleted it from my
fork
…On Tue, Apr 1, 2025 at 1:26 PM Tomasz ***@***.***> wrote:
Thank you @vasilich-tregub <https://github.com/vasilich-tregub> for your
contribution, readme/instruction looks fine, just one more question is
CMakeSettings.json
<https://github.com/OpenVisualCloud/SVT-JPEG-XS/pull/8/files#diff-beac31e96a7fc03512b0e5279050fae446468306832280e8f75f6e14df9fe345>
file required ? I do not see any reference in readme to that file
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKACLV4SJ5PKBJIBL3L2XIWQNAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRYGI4TSMJWHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
[image: tszumski]*tszumski* left a comment (OpenVisualCloud/SVT-JPEG-XS#8)
<#8 (comment)>
Thank you @vasilich-tregub <https://github.com/vasilich-tregub> for your
contribution, readme/instruction looks fine, just one more question is
CMakeSettings.json
<https://github.com/OpenVisualCloud/SVT-JPEG-XS/pull/8/files#diff-beac31e96a7fc03512b0e5279050fae446468306832280e8f75f6e14df9fe345>
file required ? I do not see any reference in readme to that file
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKACLV4SJ5PKBJIBL3L2XIWQNAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRYGI4TSMJWHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Thanks, |
Hi, I will do my best to test it in the beginning of the next week (April 7th or 8th). Sorry for that delay. |
To be on the safe side, I've decided to test my own PR one more time. In my
first version of PR, I did no ffmpeg version checkout: I supplied source
file diffs and left it to the user to manually edit necessary files. The
task is cumbersome but still doable, only it requires some patience and
meticulousness on the user's part. Later, when merging two ffmpeg plugin
folders on the maintainer's request, one folder for Windows/Linux and one
for WSL2/MSYS2, I dropped the manual edit task, returning to
checkout/patches. Unfortunately, svt-av1 also requires patching for ffmpeg
version < 7.1, even for 7.0. To simplify the patching routine, in this last
edit I dropped svt-av1 from ffmpeg configuration entirely, and
everything is OK, the build is successful and tests succeed. What a pity,
svt-av1 is very good for configuration with minimum count of components.
The good news for those connected with SVT is that ffmpeg ver. 7.1 can work
with the codec without any patches, you simply include
--enable-libsvtav1 and --enable-encoder=libsvtav1 into configuration.
Summing up, with this last edit of my fork, ffmpeg can be built with the
libsvtjpegxs codec, tests succeed, and I hope that you will accept my PR.
…On Tue, Apr 1, 2025 at 4:33 PM Mateusz Grabuszyński < ***@***.***> wrote:
Hi, I will do my best to test it in the beginning of the next week (April
7th or 8th). Sorry for that delay.
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKFA2RO6LTF57D2UKJ32XJMQLAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRYG43DGNJVGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
[image: MateuszGrabuszynski]*MateuszGrabuszynski* left a comment
(OpenVisualCloud/SVT-JPEG-XS#8)
<#8 (comment)>
Hi, I will do my best to test it in the beginning of the next week (April
7th or 8th). Sorry for that delay.
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKFA2RO6LTF57D2UKJ32XJMQLAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRYG43DGNJVGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
… both MSVC and MSYS2 builds
One more time, I self-reviewed my PR. Dropped unnecessary mention of
libsvtav1, as we cannot use it without additional patches. A minor edit of
CMAKE_ASM_NASM_FLAGS_DEBUG settings in CMakeLists.txt to pass a debug
configuration test.
On Tue, Apr 1, 2025 at 8:29 PM Valdemar Svoboda ***@***.***>
wrote:
… To be on the safe side, I've decided to test my own PR one more time. In
my first version of PR, I did no ffmpeg version checkout: I supplied source
file diffs and left it to the user to manually edit necessary files. The
task is cumbersome but still doable, only it requires some patience and
meticulousness on the user's part. Later, when merging two ffmpeg plugin
folders on the maintainer's request, one folder for Windows/Linux and one
for WSL2/MSYS2, I dropped the manual edit task, returning to
checkout/patches. Unfortunately, svt-av1 also requires patching for ffmpeg
version < 7.1, even for 7.0. To simplify the patching routine, in this last
edit I dropped svt-av1 from ffmpeg configuration entirely, and
everything is OK, the build is successful and tests succeed. What a pity,
svt-av1 is very good for configuration with minimum count of components.
The good news for those connected with SVT is that ffmpeg ver. 7.1 can work
with the codec without any patches, you simply include
--enable-libsvtav1 and --enable-encoder=libsvtav1 into configuration.
Summing up, with this last edit of my fork, ffmpeg can be built with the
libsvtjpegxs codec, tests succeed, and I hope that you will accept my PR.
On Tue, Apr 1, 2025 at 4:33 PM Mateusz Grabuszyński <
***@***.***> wrote:
> Hi, I will do my best to test it in the beginning of the next week (April
> 7th or 8th). Sorry for that delay.
>
> —
> Reply to this email directly, view it on GitHub
> <#8 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGT2SKFA2RO6LTF57D2UKJ32XJMQLAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRYG43DGNJVGQ>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
> [image: MateuszGrabuszynski]*MateuszGrabuszynski* left a comment
> (OpenVisualCloud/SVT-JPEG-XS#8)
> <#8 (comment)>
>
> Hi, I will do my best to test it in the beginning of the next week (April
> 7th or 8th). Sorry for that delay.
>
> —
> Reply to this email directly, view it on GitHub
> <#8 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGT2SKFA2RO6LTF57D2UKJ32XJMQLAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRYG43DGNJVGQ>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
I tested the repo using a newer CMake version for build. To comply with
changes in CMake version 4.0, I changed the version parameter in
CMAKE_MINIMUM_REQUIRED of two CMakeLists.txt files to 3.10. Also, corrected
typos in wsl2msys2.md.
On Fri, Apr 4, 2025 at 3:37 PM Valdemar Svoboda ***@***.***>
wrote:
… One more time, I self-reviewed my PR. Dropped unnecessary mention of
libsvtav1, as we cannot use it without additional patches. A minor edit of
CMAKE_ASM_NASM_FLAGS_DEBUG settings in CMakeLists.txt to pass a debug
configuration test.
On Tue, Apr 1, 2025 at 8:29 PM Valdemar Svoboda ***@***.***>
wrote:
> To be on the safe side, I've decided to test my own PR one more time. In
> my first version of PR, I did no ffmpeg version checkout: I supplied source
> file diffs and left it to the user to manually edit necessary files. The
> task is cumbersome but still doable, only it requires some patience and
> meticulousness on the user's part. Later, when merging two ffmpeg plugin
> folders on the maintainer's request, one folder for Windows/Linux and one
> for WSL2/MSYS2, I dropped the manual edit task, returning to
> checkout/patches. Unfortunately, svt-av1 also requires patching for ffmpeg
> version < 7.1, even for 7.0. To simplify the patching routine, in this last
> edit I dropped svt-av1 from ffmpeg configuration entirely, and
> everything is OK, the build is successful and tests succeed. What a pity,
> svt-av1 is very good for configuration with minimum count of components.
> The good news for those connected with SVT is that ffmpeg ver. 7.1 can work
> with the codec without any patches, you simply include
> --enable-libsvtav1 and --enable-encoder=libsvtav1 into configuration.
>
> Summing up, with this last edit of my fork, ffmpeg can be built with the
> libsvtjpegxs codec, tests succeed, and I hope that you will accept my PR.
>
> On Tue, Apr 1, 2025 at 4:33 PM Mateusz Grabuszyński <
> ***@***.***> wrote:
>
>> Hi, I will do my best to test it in the beginning of the next week
>> (April 7th or 8th). Sorry for that delay.
>>
>> —
>> Reply to this email directly, view it on GitHub
>> <#8 (comment)>,
>> or unsubscribe
>> <https://github.com/notifications/unsubscribe-auth/AGT2SKFA2RO6LTF57D2UKJ32XJMQLAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRYG43DGNJVGQ>
>> .
>> You are receiving this because you were mentioned.Message ID:
>> ***@***.***>
>> [image: MateuszGrabuszynski]*MateuszGrabuszynski* left a comment
>> (OpenVisualCloud/SVT-JPEG-XS#8)
>> <#8 (comment)>
>>
>> Hi, I will do my best to test it in the beginning of the next week
>> (April 7th or 8th). Sorry for that delay.
>>
>> —
>> Reply to this email directly, view it on GitHub
>> <#8 (comment)>,
>> or unsubscribe
>> <https://github.com/notifications/unsubscribe-auth/AGT2SKFA2RO6LTF57D2UKJ32XJMQLAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRYG43DGNJVGQ>
>> .
>> You are receiving this because you were mentioned.Message ID:
>> ***@***.***>
>>
>
|
…it checkout release/7.1
added instructions for git checkout release/7.1 and the
0001-commit-to-enable-libsvtjpegxs.patch file. release/7.1 reconciles use
of both flags --enable-libsvtav1 and --enable-libsvtjpegxs in configure.
Also, Netflix's vmaf codec can be included with configure; vmaf is used by
jpegxs developers to assess perceptual visual quality of video.
On Sat, Apr 5, 2025 at 2:30 PM Valdemar Svoboda ***@***.***>
wrote:
… I tested the repo using a newer CMake version for build. To comply with
changes in CMake version 4.0, I changed the version parameter in
CMAKE_MINIMUM_REQUIRED of two CMakeLists.txt files to 3.10. Also, corrected
typos in wsl2msys2.md.
On Fri, Apr 4, 2025 at 3:37 PM Valdemar Svoboda ***@***.***>
wrote:
> One more time, I self-reviewed my PR. Dropped unnecessary mention of
> libsvtav1, as we cannot use it without additional patches. A minor edit of
> CMAKE_ASM_NASM_FLAGS_DEBUG settings in CMakeLists.txt to pass a debug
> configuration test.
>
> On Tue, Apr 1, 2025 at 8:29 PM Valdemar Svoboda <
> ***@***.***> wrote:
>
>> To be on the safe side, I've decided to test my own PR one more time. In
>> my first version of PR, I did no ffmpeg version checkout: I supplied source
>> file diffs and left it to the user to manually edit necessary files. The
>> task is cumbersome but still doable, only it requires some patience and
>> meticulousness on the user's part. Later, when merging two ffmpeg plugin
>> folders on the maintainer's request, one folder for Windows/Linux and one
>> for WSL2/MSYS2, I dropped the manual edit task, returning to
>> checkout/patches. Unfortunately, svt-av1 also requires patching for ffmpeg
>> version < 7.1, even for 7.0. To simplify the patching routine, in this last
>> edit I dropped svt-av1 from ffmpeg configuration entirely, and
>> everything is OK, the build is successful and tests succeed. What a pity,
>> svt-av1 is very good for configuration with minimum count of components.
>> The good news for those connected with SVT is that ffmpeg ver. 7.1 can work
>> with the codec without any patches, you simply include
>> --enable-libsvtav1 and --enable-encoder=libsvtav1 into configuration.
>>
>> Summing up, with this last edit of my fork, ffmpeg can be built with the
>> libsvtjpegxs codec, tests succeed, and I hope that you will accept my PR.
>>
>> On Tue, Apr 1, 2025 at 4:33 PM Mateusz Grabuszyński <
>> ***@***.***> wrote:
>>
>>> Hi, I will do my best to test it in the beginning of the next week
>>> (April 7th or 8th). Sorry for that delay.
>>>
>>> —
>>> Reply to this email directly, view it on GitHub
>>> <#8 (comment)>,
>>> or unsubscribe
>>> <https://github.com/notifications/unsubscribe-auth/AGT2SKFA2RO6LTF57D2UKJ32XJMQLAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRYG43DGNJVGQ>
>>> .
>>> You are receiving this because you were mentioned.Message ID:
>>> ***@***.***>
>>> [image: MateuszGrabuszynski]*MateuszGrabuszynski* left a comment
>>> (OpenVisualCloud/SVT-JPEG-XS#8)
>>> <#8 (comment)>
>>>
>>> Hi, I will do my best to test it in the beginning of the next week
>>> (April 7th or 8th). Sorry for that delay.
>>>
>>> —
>>> Reply to this email directly, view it on GitHub
>>> <#8 (comment)>,
>>> or unsubscribe
>>> <https://github.com/notifications/unsubscribe-auth/AGT2SKFA2RO6LTF57D2UKJ32XJMQLAVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONRYG43DGNJVGQ>
>>> .
>>> You are receiving this because you were mentioned.Message ID:
>>> ***@***.***>
>>>
>>
|
Hi @vasilich-tregub , few comments:
|
Please create another PR with FFmpeg 7.1 patches(and revert changes in this PR). Patches should follow the same scheme as for 6.1/7.0, do not to create single large patch-file. |
Hi @tszumski,
I do not think that splitting into multiple PRs is reasonable: this repo is
already (using git parlance) many 'commits' behind ISO21122-ed3, we should
move forward as soon as possible.
About 'Patches should follow the same scheme as for 6.1/7.0, do not to
create single large patch-file': I do not have access to your internal
guidelines on issuing patch files, and do not understand how your multiple
tier scheme is better than a single file. Let me, however, note that the
21122 standard and consequently its SVT-JPEG-XS implementation are in their
experimental phase, so we can have a quite experienced developer (as user)
in mind.
Also, let me note that I filled in the PR template form with the option
'Allow edits from maintainers' set. You, the maintainer, can select from my
fork only items that you approve for merge, and also edit them any way you
like.
I trust you while selecting this option, as I believe that your edits would
be directed by the intention to make the revision more focused.
Do not hesitate to ask me questions, for example, about my selection of
flags and parameters for ffmpeg customized configure, as well as other
questions you find relevant for deciding the fate of my PR.
…On Mon, Apr 7, 2025 at 5:21 PM Tomasz ***@***.***> wrote:
Please create another PR with FFmpeg 7.1 patches(and revert changes in
this PR). Patches should follow the same scheme as for 6.1/7.0, do not to
create single large patch-file.
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKBOSLIE646YBQOBBHL2YJGR3AVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOOBSHAZTAMBSGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
[image: tszumski]*tszumski* left a comment (OpenVisualCloud/SVT-JPEG-XS#8)
<#8 (comment)>
Please create another PR with FFmpeg 7.1 patches(and revert changes in
this PR). Patches should follow the same scheme as for 6.1/7.0, do not to
create single large patch-file.
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGT2SKBOSLIE646YBQOBBHL2YJGR3AVCNFSM6AAAAABZFJYMX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOOBSHAZTAMBSGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi @vasilich-tregub,
With regards to multiple PR:
Also once again I am asking you to not to reply via email, thats how github notification looks like: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partial review - 6 out of 11 files viewed.
Hello,
Thank you for the contribution. I am sorry, but I stopped somewhere in the middle. I am not able to spend more time on this PR. The change is not going to be accepted in the current form. Some of the issues I found are:
-
The PR message mentions MSYS2 build of SVT JPEG XS - but the change does cover a lot more - WSL builds, patches to FFmpeg 7.1, etc.
-
Those patches to FFmpeg 7.1 are not to be inserted in the patch file (please see the associated comment). In my opinion, the proper folder structure is a must as well. The folders and files for FFmpeg 6.1 and 7.0 are already provided there, please follow your changes to be done in the same manner.
-
Mostly subjective, but the instructions are a bit too chaotic for my liking - the installation process should cover the bare minimum version, not adding or removing any FFmpeg dependencies, like AV1 (dav1d). Also, if something can be done using pacman alone, I do not see a need to install pacboy.
-
And the most important, I have tried to build the JPEG XS according to the provided instructions, but am not able to, encountering a following error:
Logs
$ cmake --build ~/install-dir/builddebug -j4 --config Debug --target install System is unknown to cmake, create: Platform/MINGW64_NT-10.0-22631 to use this system, please post your config file on discourse.cmake.org so it can be added to cmake Your CMakeCache.txt file was copied to CopyOfCMakeCache.txt. Please post that file on discourse.cmake.org. -- BUILD_SHARED_LIBS: OFF (...) CMake Warning at third_party/cpuinfo/CMakeLists.txt:61 (MESSAGE): Target processor architecture "unknown" is not supported in cpuinfo. cpuinfo will compile, but cpuinfo_initialize() will always fail. CMake Warning at third_party/cpuinfo/CMakeLists.txt:74 (MESSAGE): Target operating system "MINGW64_NT-10.0-22631" is not supported in cpuinfo. cpuinfo will compile, but cpuinfo_initialize() will always fail. -- Configuring done (0.4s) -- Generating done (0.6s) (...) [ 84%] Building C object Source/Lib/Encoder/Codec/CMakeFiles/ENCODER_CODEC.dir/encoder_dsp_rtcd.c.obj [ 84%] Built target DECODER_CODEC [ 85%] Building C object Source/App/SampleDecoder/CMakeFiles/SvtJpegxsSampleDecoder.dir/main.c.obj [ 87%] Linking C executable /home/usr123/jxs-ffmpeg-8/SVT-JPEG-XS/Bin/Debug/SvtJpegxsSampleDecoder [ 87%] Built target ENCODER_CODEC C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/14.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: ../../../third_party/cpuinfo/CMakeFiles/cpuinfo.dir/src/init.c.obj:init.c:(.rdata$.refptr.cpuinfo_x86_windows_init[.refptr.cpuinfo_x86_windows_init]+0x0): undefined reference to `cpuinfo_x86_windows_init' C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/14.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: ../../Lib/Common/Codec/CMakeFiles/COMMON_CODEC.dir/common_dsp_rtcd.c.obj:common_dsp_rtc:(.rdata$.refptr.cpuinfo_isa[.refptr.cpuinfo_isa]+0x0): undefined reference to `cpuinfo_isa' [ 88%] Linking C static library /home/usr123/jxs-ffmpeg-8/SVT-JPEG-XS/Bin/Debug/libSvtJpegxs.a collect2.exe: error: ld returned 1 exit status [ 89%] Building C object Source/App/SampleEncoder/CMakeFiles/SvtJpegxsSampleEncoder.dir/main.c.obj make[2]: *** [Source/App/SampleDecoder/CMakeFiles/SvtJpegxsSampleDecoder.dir/build.make:138: /home/usr123/jxs-ffmpeg-8/SVT-JPEG-XS/Bin/Debug/SvtJpegxsSampleDecoder] Error 1 make[1]: *** [CMakeFiles/Makefile2:866: Source/App/SampleDecoder/CMakeFiles/SvtJpegxsSampleDecoder.dir/all] Error 2 make[1]: *** Waiting for unfinished jobs.... [ 90%] Linking C executable /home/usr123/jxs-ffmpeg-8/SVT-JPEG-XS/Bin/Debug/SvtJpegxsSampleEncoder C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/14.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: ../../../third_party/cpuinfo/CMakeFiles/cpuinfo.dir/src/init.c.obj:init.c:(.rdata$.refptr.cpuinfo_x86_windows_init[.refptr.cpuinfo_x86_windows_init]+0x0): undefined reference to `cpuinfo_x86_windows_init' C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/14.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: ../../Lib/Common/Codec/CMakeFiles/COMMON_CODEC.dir/common_dsp_rtcd.c.obj:common_dsp_rtc:(.rdata$.refptr.cpuinfo_isa[.refptr.cpuinfo_isa]+0x0): undefined reference to `cpuinfo_isa' collect2.exe: error: ld returned 1 exit status make[2]: *** [Source/App/SampleEncoder/CMakeFiles/SvtJpegxsSampleEncoder.dir/build.make:149: /home/usr123/jxs-ffmpeg-8/SVT-JPEG-XS/Bin/Debug/SvtJpegxsSampleEncoder] Error 1 make[1]: *** [CMakeFiles/Makefile2:832: Source/App/SampleEncoder/CMakeFiles/SvtJpegxsSampleEncoder.dir/all] Error 2 [ 90%] Built target SvtJpegxsLib make: *** [Makefile:136: all] Error 2
I do not think that splitting into multiple PRs is reasonable: this repo is
already (using git parlance) many 'commits' behind ISO21122-ed3, we should
move forward as soon as possible.
Is this exact change adding anything to the repository's ISO compliance?
Let me, however, note that the
21122 standard and consequently its SVT-JPEG-XS implementation are in their
experimental phase, so we can have a quite experienced developer (as user)
in mind.
Either way, we should not give up on clarity and maintainability. After the PR is merged, we do not know when it will be changed again, so (if possible) we should prepare it in the best possible way, from the very start.
Also, let me note that I filled in the PR template form with the option
'Allow edits from maintainers' set. You, the maintainer, can select from my
fork only items that you approve for merge, and also edit them any way you
like.
We are aware of that option. :) We can split the PR into multiple ones ourselves, but please keep in mind, changing anything from our end requires more time.
@@ -94,20 +94,22 @@ set(CMAKE_CXX_STANDARD 11) | |||
|
|||
# BUILD_SHARED_LIBS is a standard CMake variable, but we declare it here to | |||
# make it prominent in the GUI. | |||
option(BUILD_SHARED_LIBS "Build shared libraries (DLLs)." ON) | |||
option(BUILD_SHARED_LIBS "Build shared libraries (DLLs)." OFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[q] BUILD_SHARED_LIBS
is turned OFF, and there is never a way to turn it back ON. Is that an expected behavior?
else() | ||
set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DUNIX64") | ||
set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DUNIX64=1 -DWIN64=0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[minor] Keep the same order for readability
set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DUNIX64=1 -DWIN64=0") | |
set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DWIN64=0 -DUNIX64=1") |
set(CMAKE_ASM_NASM_FLAGS_DEBUG "${CMAKE_ASM_NASM_FLAGS_DEBUG} -gcv8") | ||
if(MSVC) | ||
set(CMAKE_ASM_NASM_FLAGS_DEBUG "${CMAKE_ASM_NASM_FLAGS_DEBUG} -gcv8") | ||
else() | ||
set(CMAKE_ASM_NASM_FLAGS_DEBUG "${CMAKE_ASM_NASM_FLAGS_DEBUG} -gcv8") | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MSYS2build.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand, this is supposed to cover building with MSYS2 on Windows, but the doc is much longer and covers supported formats and Linux-related topics as well. Please avoid duplicating information that is already in the main README.md
file.
@@ -65,6 +65,18 @@ Supported OS versions: | |||
- __API headers location__ | |||
- API headers can be found under `Source/API` | |||
|
|||
### Windows Services for Linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
### Windows Services for Linux | |
### Windows Subsystem for Linux (WSL) |
sudo apt install libdav1d-dev | ||
sudo apt install libsdl2-dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sudo apt install libdav1d-dev | |
sudo apt install libsdl2-dev | |
sudo apt install libdav1d-dev libsdl2-dev |
The library libsvtav1 can also be apt installed, only you have to install two | ||
packages: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The library libsvtav1 can also be apt installed, only you have to install two | |
packages: | |
If checking out to version 7.1 of FFmpeg (`release/7.1`), then install additional packages: |
``` | ||
sudo apt install libsvtav1-dev libsvtav1enc-dev | ||
``` | ||
You only may need the libsvtav1 codec if you plan to checkout ffmpeg to `release/7.1`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You only may need the libsvtav1 codec if you plan to checkout ffmpeg to `release/7.1`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I strictly do not agree with that approach.
- This patch file should be placed under a proper version folder (
ffmpeg-plugin/7.1/0001-commit-to-enable-libsvtjpegxs.patch
), to avoid confusion. - There is a way to use the
libavcodec/libsvtjpegxsdec.c
andlibavcodec/libsvtjpegxsenc.c
files without the need of copying their contents into the patch. Although it is more manual, it avoids misalignment between the commits. Modifying anything inside those two C files would not propagate into the patch. A mirror of the change inside the patch file would be required, which is unwanted.
--disable-doc --disable-everything --disable-network \ | ||
--enable-libdav1d --enable-libsvtjpegxs \ | ||
--enable-decoder='aac,ac3,h264,hevc,libdav1d,libsvtjpegxs' \ | ||
--enable-encoder='aac,ac3,wrapped_avframe,libsvtjpegxs' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--enable-encoder='aac,ac3,wrapped_avframe,libsvtjpegxs' | |
--enable-encoder='aac,ac3,wrapped_avframe,libsvtjpegxs' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All your comments are fair and useful; I'm working on satisfying your requirements, but the test of build-ability is paramount for my work. To begin with, I edited the lines
50 MESSAGE(STATUS "CMAKE_SYSTEM_PROCESSOR is "${CMAKE_SYSTEM_PROCESSOR}"")
and
66 MESSAGE(STATUS "CMAKE_SYSTEM_NAME is "${CMAKE_SYSTEM_NAME}"")
of SVT-JPEG-XS/third_party/cpuinfo/CMakeLists.txt (in the original CMakeLists, lines 50 and 66 are blank), and ran
$ cmake -S . -B builddebug -DCMAKE_BUILD_TYPE=Debug -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=$HOME/install-dir > generatebuilddebug.log
in MSYS2 UCRT64 terminal. It reports
...
-- C:/msys64/home/User/SVT-JPEG-XS/Source/Lib/Decoder/ASM_AVX512 CXXFLAGS: -Wall -Wextra -Wformat -Wformat-security -mxsave -fno-asynchronous-unwind-tables -Wno-unused-parameter -mavx512f -mavx512bw -mavx512dq -mavx512vl
-- CMAKE_SYSTEM_PROCESSOR is "AMD64"
-- CMAKE_SYSTEM_NAME is "Windows"
-- Configuring done (32.4s)
...
while your log has
CMake Warning at third_party/cpuinfo/CMakeLists.txt:61 (MESSAGE):
Target processor architecture "unknown" is not supported in cpuinfo.
cpuinfo will compile, but cpuinfo_initialize() will always fail.
CMake Warning at third_party/cpuinfo/CMakeLists.txt:74 (MESSAGE):
Target operating system "MINGW64_NT-10.0-22631" is not supported in
cpuinfo. cpuinfo will compile, but cpuinfo_initialize() will always fail.
i.e., CMAKE_SYSTEM_PROCESSOR is "unknown" and CMAKE_SYSTEM_NAME is "MINGW64_NT-10.0-22631".
I'm struggling to find out how your system would have come up with these entirely different values for CMAKE_SYSTEM_PROCESSOR and CMAKE_SYSTEM_NAME variables this place. The SVT-JPEG-XS sources never define these variables explicitly. I gather, their values could come from the toolset.
I'm investigating hypothetical scenarios of your possible MSYS2 install and preliminary actions that may result in this disagreement. Maybe you have any ideas? What cmake flavor does your pacman -Q
show? does it show mingw-w64-ucrt-x86_64-cmake 3.31.6-1
or mingw-w64-ucrt-x86_64-cmake <other version>
or what? I installed toolchain and cmake two weeks ago in the MSYS2 UCRT64 terminal with commands
pacboy -S toolchain:u
pacboy -S cmake:u
aiming intentionally to delegate decision making, on what versions and triplets is to be installed, to the MSYS2 UCRT64 terminal. 'Make things, especially with installs, as simple as it is reasonable'.
It is important that the toolchain components and cmake are all from the 'mingw-w64-ucrt-x86_64-' triplet. I see that your ld.exe is of UCRT64 toolchain but cannot understand what cmake you use.
The edits here enable use of MSYS2 build environment for SVT JPEG XS codec repository. The base repository recommends use of MSYS2 to build FFMPEG with libsvtjpegxs enabled; in the process, SVT-JPEG-XS has to be built in the MSYS2 environment.
A lot of lines in the root CMakeLists.txt and Source/Lib/Common/ASM_SSE2/x64inc.asm had to be edited to enable this build with MSYS2 (UCRT) terminal. In this fork, I made the necessary edits and edited the related sections of the README files. For convenience of users with MSVC background, the sections describing Linux and MSYS builds are written assuming the fresh installs of these systems. I expanded the section on building the project in the MSYS2 (UCRT) environment and transferred this section from the ffmpeg-plugin README to the root README. Also, the test file 200.jxs from ISO_IEC 21122 Part 4 test files is added to the repo for the expedient testing of a sample decoder.