diff mbox series

[bug#64748] gnu: webrtc-for-telegram-desktop: Update to a45d8b8

Message ID 20230720171841.31901-1-distopico@riseup.net
State New
Headers show
Series [bug#64748] gnu: webrtc-for-telegram-desktop: Update to a45d8b8 | expand

Commit Message

Distopico July 20, 2023, 5:18 p.m. UTC
Fixes telegram-desktop call not working with openSSL 3

see: https://github.com/telegramdesktop/tdesktop/issues/26108

* gnu/packages/telegram.scm (webrtc-for-telegram-desktop): Update to a45d8b8 revision commit
---
 gnu/packages/telegram.scm | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)


base-commit: 13cb9b302868b5a966a6ae177412c474084f4bf1

Comments

Saku Laesvuori July 20, 2023, 7:33 p.m. UTC | #1
> Fixes telegram-desktop call not working with openSSL 3
> 
> see: https://github.com/telegramdesktop/tdesktop/issues/26108
> 
> * gnu/packages/telegram.scm (webrtc-for-telegram-desktop): Update to a45d8b8 revision commit

You should probably mention adding `libsrtp-for-telegram-desktop` in the
ChangeLog.

> @@ -96,6 +97,21 @@ (define libyuv-for-telegram-desktop
>         (base32
>          "0mm56p8iapfild2xdw4w8zi35c3xm06fgagiali644gnxdmnym6c")))))
>  
> +(define libsrtp-for-telegram-desktop
> +  (let ((commit "a566a9cfcd619e8327784aa7cff4a1276dc1e895")
> +        (revision "1494"))
> +    (origin
> +      (method git-fetch)
> +      (uri (git-reference
> +            (url "https://github.com/cisco/libsrtp")
> +            (commit commit)))
> +      (file-name (git-file-name
> +                  "libsrtp-for-telegram-desktop"
> +                  (git-version "0" revision commit)))
> +      (sha256
> +       (base32
> +        "1ichw2v9s2mggi5p2wbbmlg55q4r48dxi3ks7ykfcfkmh7pb1w1s")))))
> +

Have you tried using the libsrtp version packaged in Guix as an input to
`webrtc-for-telegram-desktop` instead of bundling it? If that doesn't
work, have you tried packaging libsrtp 2.5.0 and using it as an input?
It would probably be relatively easy with record inheriting.

I'm quite new to Guix (and couldn't push changes even if they were
perfect) so someone else might have more comments on this.
Distopico July 20, 2023, 8:21 p.m. UTC | #2
On 2023-07-20, Saku Laesvuori <saku@laesvuori.fi> wrote:

>
> You should probably mention adding `libsrtp-for-telegram-desktop` in the
> ChangeLog.

Looks like it's not longer necessary, Changelog is auto-generated, that
said in the main Changelog

>
> Have you tried using the libsrtp version packaged in Guix as an input to
> `webrtc-for-telegram-desktop` instead of bundling it? If that doesn't
> work, have you tried packaging libsrtp 2.5.0 and using it as an input?
> It would probably be relatively easy with record inheriting.
>
> I'm quite new to Guix (and couldn't push changes even if they were
> perfect) so someone else might have more comments on this.
>

I tried but it require source code inside libsrtp folder, for example
'/crypto/cipher/cipher.c', also libsrtp package is the compiled version
and webrtc-.. require source code from libsrtp.
Saku Laesvuori July 20, 2023, 9:06 p.m. UTC | #3
> > You should probably mention adding `libsrtp-for-telegram-desktop` in the
> > ChangeLog.
> 
> Looks like it's not longer necessary, Changelog is auto-generated, that
> said in the main Changelog

I mean the commit message. Now your commit message's ChangeLog only
mentions updating webrtc-for-telegram-desktop. See my last telegram
commit 6192acf8b77 for an example of what I'm talking about.

I would assume the ChangeLog file is autogenerated based on the commit
messages.

> > Have you tried using the libsrtp version packaged in Guix as an input to
> > `webrtc-for-telegram-desktop` instead of bundling it? If that doesn't
> > work, have you tried packaging libsrtp 2.5.0 and using it as an input?
> > It would probably be relatively easy with record inheriting.
> >
> > I'm quite new to Guix (and couldn't push changes even if they were
> > perfect) so someone else might have more comments on this.
> 
> I tried but it require source code inside libsrtp folder, for example
> '/crypto/cipher/cipher.c', also libsrtp package is the compiled version
> and webrtc-.. require source code from libsrtp.

You would probably have to patch the `CMakeLists.txt` so it doesn't try
to build libsrtp from source, see what I did with crc32c in the
mentioned commit for reference. It is quite possible that telegram
doesn't actually need the source code and they are simply using a git
submodule to simplify their own dependency versioning and development.

You might also have to add a new libsrtp version because the one
packaged in Guix is not configured to use openssl unlike telegram
expects. I'd still try the packaged version first if compiling telegram
a few more times isn't a big problem for you.
Distopico July 20, 2023, 11:09 p.m. UTC | #4
On 2023-07-21, Saku Laesvuori <saku@laesvuori.fi> wrote:

>
> I mean the commit message. Now your commit message's ChangeLog only
> mentions updating webrtc-for-telegram-desktop. See my last telegram
> commit 6192acf8b77 for an example of what I'm talking about.
>
> I would assume the ChangeLog file is autogenerated based on the commit
> messages.
>
Make sense, I'll update the patch to add more description

>
> You would probably have to patch the `CMakeLists.txt` so it doesn't try
> to build libsrtp from source, see what I did with crc32c in the
> mentioned commit for reference. It is quite possible that telegram
> doesn't actually need the source code and they are simply using a git
> submodule to simplify their own dependency versioning and development.
>
> You might also have to add a new libsrtp version because the one
> packaged in Guix is not configured to use openssl unlike telegram
> expects. I'd still try the packaged version first if compiling telegram
> a few more times isn't a big problem for you.
>
I tried but didn't work, this change is similar to **libyuv**, that have
reference to the source code of that external library: example
https://github.com/desktop-app/tg_owt/blob/83-sdk/src/api/video/i010_buffer.cc#L17

So the case of **libsrtp** is similar, check for example:
- https://github.com/desktop-app/tg_owt/blob/master/src/pc/srtp_session_unittest.cc#L25
- https://github.com/desktop-app/tg_owt/blob/master/src/pc/external_hmac.cc#L18
- https://github.com/desktop-app/tg_owt/blob/master/src/pc/BUILD.gn#L174

So change that will require patch several files, not just
`CMakeLists.txt`, patch all those file will be make this package hard to
maintain.

but you can try if you want and let me know if works.
Saku Laesvuori July 21, 2023, 7:22 a.m. UTC | #5
> > You would probably have to patch the `CMakeLists.txt` so it doesn't try
> > to build libsrtp from source, see what I did with crc32c in the
> > mentioned commit for reference. It is quite possible that telegram
> > doesn't actually need the source code and they are simply using a git
> > submodule to simplify their own dependency versioning and development.
> >
> > You might also have to add a new libsrtp version because the one
> > packaged in Guix is not configured to use openssl unlike telegram
> > expects. I'd still try the packaged version first if compiling telegram
> > a few more times isn't a big problem for you.
>
> I tried but didn't work, this change is similar to **libyuv**, that have
> reference to the source code of that external library: example
> https://github.com/desktop-app/tg_owt/blob/83-sdk/src/api/video/i010_buffer.cc#L17
> 
> So the case of **libsrtp** is similar, check for example:
> - https://github.com/desktop-app/tg_owt/blob/master/src/pc/srtp_session_unittest.cc#L25
> - https://github.com/desktop-app/tg_owt/blob/master/src/pc/external_hmac.cc#L18
> - https://github.com/desktop-app/tg_owt/blob/master/src/pc/BUILD.gn#L174
> 
> So change that will require patch several files, not just
> `CMakeLists.txt`, patch all those file will be make this package hard to
> maintain.

I see. I think libsrtp is small enough that unbundling it is not worth
the effort of patching all those files.
Raghav Gururajan July 22, 2023, 6:36 a.m. UTC | #6
If libsrtp in Guix uses something other than openssl by default, then we 
could create a package-definition "libsrtp-openssl" which inherits from 
"libsrtp" and build-flagged to use openssl. We can then use that for 
telegram desktop.

Thoughts?

Regards,
RG.
Saku Laesvuori July 22, 2023, 8:25 a.m. UTC | #7
> If libsrtp in Guix uses something other than openssl by default, then we 
> could create a package-definition "libsrtp-openssl" which inherits from 
> "libsrtp" and build-flagged to use openssl. We can then use that for 
> telegram desktop.

Yes, but we would still need to patch webrtc-for-telegram-desktop as the
code currently refers to headers in the submodule directory. Now that I
think of it, it could actually be relatively easy with a regex substitution
(something like s!^#include "third_party/(srtp/[^"]*)"$!#include <\1>!)
on every file. That's of course assuming that the code only refers to
headers in the submodule.
Distopico July 24, 2023, 5:07 p.m. UTC | #8
I just tested trying to replace all the libsrtp but not luck from side,
they use for example:
- third_party/libsrtp/crypto/include/crypto_types.h
- third_party/libsrtp/include/srtp.h

not exposed by "libsrtp" package, also they use a custom configuration
for that library
https://github.com/desktop-app/tg_owt/blob/master/src/third_party/libsrtp_config/config.h

Also there is many other references to `libsrtp` .C source code
https://github.com/desktop-app/tg_owt/blob/a45d8b8f0a99bd0e5118dda1dc4a8b7b3ad5dcfd/cmake/libsrtp.cmake#L4

I was checking nix implementation and they just use the submodules
https://github.com/NixOS/nixpkgs/blob/nixos-23.05/pkgs/applications/networking/instant-messengers/telegram/telegram-desktop/tg_owt.nix#L19
and not "srtp" package.

So I think the patch that i sent is a good solution, similar to the nix
one, but if someone else want to try another option would be great.


On 2023-07-22, Saku Laesvuori <saku@laesvuori.fi> wrote:

> [[PGP Signed Part:Undecided]]
>> If libsrtp in Guix uses something other than openssl by default, then we 
>> could create a package-definition "libsrtp-openssl" which inherits from 
>> "libsrtp" and build-flagged to use openssl. We can then use that for 
>> telegram desktop.
>
> Yes, but we would still need to patch webrtc-for-telegram-desktop as the
> code currently refers to headers in the submodule directory. Now that I
> think of it, it could actually be relatively easy with a regex substitution
> (something like s!^#include "third_party/(srtp/[^"]*)"$!#include <\1>!)
> on every file. That's of course assuming that the code only refers to
> headers in the submodule.
>
> [[End of PGP Signed Part]]
Raghav Gururajan July 24, 2023, 8:17 p.m. UTC | #9
Hello,

> I just tested trying to replace all the libsrtp but not luck from side,
> they use for example:
> - third_party/libsrtp/crypto/include/crypto_types.h
> - third_party/libsrtp/include/srtp.h
>
>> Yes, but we would still need to patch webrtc-for-telegram-desktop as the
>> code currently refers to headers in the submodule directory. Now that I
>> think of it, it could actually be relatively easy with a regex substitution
>> (something like s!^#include "third_party/(srtp/[^"]*)"$!#include <\1>!)
>> on every file. That's of course assuming that the code only refers to
>> headers in the submodule.

I was wondering if we could add new phase after unpack that copies 
contents from libsrtp package to `[build-dir]/third_party/libsrtp`.

Regards,
RG.
Saku Laesvuori July 26, 2023, 7:24 a.m. UTC | #10
> I was wondering if we could add new phase after unpack that copies 
> contents from libsrtp package to `[build-dir]/third_party/libsrtp`.

Simply replacing #$libsrtp-for-telegram-desktop with #$(package-source
libsrtp) in the unpack-additional-sources phase seems to work (builds fine and
telegram doesn't crash; I haven't tried calls).
Distopico July 26, 2023, 6:54 p.m. UTC | #11
Tested with calls and works fine, changes in the last path

On 2023-07-26, Saku Laesvuori <saku@laesvuori.fi> wrote:

> [[PGP Signed Part:Undecided]]
>> I was wondering if we could add new phase after unpack that copies 
>> contents from libsrtp package to `[build-dir]/third_party/libsrtp`.
>
> Simply replacing #$libsrtp-for-telegram-desktop with #$(package-source
> libsrtp) in the unpack-additional-sources phase seems to work (builds fine and
> telegram doesn't crash; I haven't tried calls).
>
> [[End of PGP Signed Part]]
diff mbox series

Patch

diff --git a/gnu/packages/telegram.scm b/gnu/packages/telegram.scm
index aa01c2f692..aeab249198 100644
--- a/gnu/packages/telegram.scm
+++ b/gnu/packages/telegram.scm
@@ -2,6 +2,7 @@ 
 ;;; Copyright © 2021 Raghav Gururajan <rg@raghavgururajan.name>
 ;;; Copyright © 2022 Hilton Chain <hako@ultrarare.space>
 ;;; Copyright © 2023 Saku Laesvuori <saku@laesvuori.fi>
+;;; Copyright © 2023 Camilo Q.S. (Distopico) <distopico@riseup.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -96,6 +97,21 @@  (define libyuv-for-telegram-desktop
        (base32
         "0mm56p8iapfild2xdw4w8zi35c3xm06fgagiali644gnxdmnym6c")))))
 
+(define libsrtp-for-telegram-desktop
+  (let ((commit "a566a9cfcd619e8327784aa7cff4a1276dc1e895")
+        (revision "1494"))
+    (origin
+      (method git-fetch)
+      (uri (git-reference
+            (url "https://github.com/cisco/libsrtp")
+            (commit commit)))
+      (file-name (git-file-name
+                  "libsrtp-for-telegram-desktop"
+                  (git-version "0" revision commit)))
+      (sha256
+       (base32
+        "1ichw2v9s2mggi5p2wbbmlg55q4r48dxi3ks7ykfcfkmh7pb1w1s")))))
+
 (define cmake-helpers-for-telegram-desktop
   (origin
     (method git-fetch)
@@ -265,8 +281,8 @@  (define tgcalls-for-telegram-desktop
       "193m2gkvipijqbfd6a8mhg9nd63wlnshzgspk3pip57vk21l709z"))))
 
 (define-public webrtc-for-telegram-desktop
-  (let ((commit "5098730b9eb6173f0b52068fe2555b7c1015123a")
-        (revision "328"))
+  (let ((commit "a45d8b8f0a99bd0e5118dda1dc4a8b7b3ad5dcfd")
+        (revision "388"))
     (hidden-package
      (package
        (name "webrtc-for-telegram-desktop")
@@ -282,14 +298,14 @@  (define-public webrtc-for-telegram-desktop
           (file-name
            (git-file-name name version))
           (sha256
-           (base32 "1lk54zlrff59rj5k9dylsgz4sdds4728psrk8m3v9qn5y8d6z8qy"))
+           (base32 "1qs3ikkd6l56brj40cv6wlhx5gj5avisj9mj8ypjfwcyw9hb2n5y"))
           (modules '((guix build utils)
                      (ice-9 ftw)
                      (srfi srfi-1)))
           (snippet
            #~(begin
                (let ((keep
-                      '("libsrtp" "rnnoise"
+                      '("rnnoise" "libsrtp_config"
                         ;; Not available in Guix.
                         "pffft")))
                  (with-directory-excursion "src/third_party"
@@ -311,9 +327,12 @@  (define-public webrtc-for-telegram-desktop
              (add-after 'unpack 'unpack-additional-sources
                (lambda _
                  (let* ((third-party (string-append (getcwd) "/src/third_party"))
-                        (libyuv-to (string-append third-party "/libyuv")))
+                        (libyuv-to (string-append third-party "/libyuv"))
+                        (libsrtp-to (string-append third-party "/libsrtp")))
                    (copy-recursively #$libyuv-for-telegram-desktop
-                                     libyuv-to)))))))
+                                     libyuv-to)
+                   (copy-recursively #$libsrtp-for-telegram-desktop
+                                     libsrtp-to)))))))
        (native-inputs (list pkg-config python-wrapper yasm))
        (inputs
         (list abseil-cpp-cxxstd17