Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

x265/encoder.pyx:449:26: undeclared name not builtin: Dict #4010

Closed
sagitter opened this issue Sep 20, 2023 · 10 comments
Closed

x265/encoder.pyx:449:26: undeclared name not builtin: Dict #4010

sagitter opened this issue Sep 20, 2023 · 10 comments
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@sagitter
Copy link

sagitter commented Sep 20, 2023

Hi all.

Describe the bug

x265 encoder is not compiled with Cython-0.29.33 and Python-3.11 in Fedora 38:

[26/55] Cythonizing xpra/net/bencode/cython_bencode.pyx

Error compiling Cython file:
------------------------------------------------------------
...
        self.time = 0
        self.frames = 0
        self.first_frame_timestamp = 0


    def get_info(self) -> Dict[str,Any]:
                         ^
------------------------------------------------------------

xpra/codecs/x265/encoder.pyx:449:26: undeclared name not builtin: Dict

Error compiling Cython file:
------------------------------------------------------------
...
        self.time = 0
        self.frames = 0
        self.first_frame_timestamp = 0


    def get_info(self) -> Dict[str,Any]:
                                  ^
------------------------------------------------------------

xpra/codecs/x265/encoder.pyx:449:35: undeclared name not builtin: Any
[27/55] Cythonizing xpra/net/brotli/compressor.pyx
Traceback (most recent call last):
  File "/usr/lib64/python3.11/site-packages/Cython/Build/Dependencies.py", line 1259, in cythonize_one_helper
    return cythonize_one(*m)
           ^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/site-packages/Cython/Build/Dependencies.py", line 1235, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: xpra/codecs/x265/encoder.pyx

System Information (please complete the following information):

@sagitter sagitter added the bug Something isn't working label Sep 20, 2023
@totaam totaam added the invalid This doesn't seem right label Sep 21, 2023
@totaam
Copy link
Collaborator

totaam commented Sep 21, 2023

Why would you manually enable a codec that is disabled by default?
It is disabled for a reason.

@totaam totaam closed this as not planned Won't fix, can't repro, duplicate, stale Sep 21, 2023
@totaam
Copy link
Collaborator

totaam commented Sep 21, 2023

@sagitter oh, and please try to learn from the packaging that we already do here - chances are that there are many other issues that you don't know about.
Preferably without causing our packages to break once again.

totaam added a commit that referenced this issue Sep 21, 2023
for those mad enough to want to try to enable it
@sergiomb2
Copy link

the real fix here is :
x265_fix.patch.txt

totaam added a commit that referenced this issue Sep 25, 2023
@totaam
Copy link
Collaborator

totaam commented Sep 25, 2023

@sergiomb2 No. Seriously, do NOT enable this codec!

I've tried making it difficult to enable, I've tried telling people time and time again to not enable it, but I guess the only way to prevent self harm is this: ec12676.

@sergiomb2
Copy link

Hi, @totaam , ok.
We in Fedora and RPMFusion have to build "non-free" part outside of Fedora, we do a complement package called xpra-codecs-freeworld

since x265 is not advised I ask you about openh264 and x264 the other 2 that are in freeworld package , are openh264 and x264 useful for end users in general ?

@totaam
Copy link
Collaborator

totaam commented Sep 26, 2023

we do a complement package called xpra-codecs-freeworld

I am well aware of the Fedora way of packaging things as it has caused breakage for users of our repositories in the past.
Please take a moment to review how we package things, in particular sub-packages and please try not to cause more breakage.

On the server side:

  • if the clients are running xpra then neither x264 nor openh264 are strictly needed since vpx is a decent alternative
  • for xpra-html5 clients, an h264 video encoder is recommended (preferably nvenc - which Fedora does not ship) since support for vp8 / vp9 is not as reliable in the browser. Note: x264 is still way ahead of openh264.

@sagitter
Copy link
Author

sagitter commented Sep 26, 2023

I am well aware of the Fedora way of packaging things as it has caused breakage for users of our repositories in the past.
Please take a moment to review how we package things, in particular sub-packages and please try not to cause more breakage.

I don't know what's happened in past, me and @sergiomb2 taking care of Xpra Fedora RPMs (and that unofficial one in RPMFusion) now, we must respect the packaging rules of Fedora Project but we also want to follow your recommendations.
Fedora users must understand how to manage external packages/repositories like yours regardless if our packages are "broken" or not. The external packages are extraneous to Fedora, not the opposite.

We can evaluate the chance to replicate your sub-packages organization with official RPMs of Xpra in Fedora but respecting the Packaging Rules of course; in this case external packages are useless except if they contain proprietary codecs.

@totaam
Copy link
Collaborator

totaam commented Oct 12, 2023

@sagitter here is a simple example of a commit that was added to v5 packaging to make it easier for users to upgrade / downgrade to / from v6.
These are the sort of changes that can save users major headaches, no matter which repository is "external"

@sergiomb2
Copy link

@totaam where ?

@totaam
Copy link
Collaborator

totaam commented Oct 12, 2023

@sergiomb2 oops!
Here's the missing link: 816ff93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

3 participants