Subject: Re: [bug#40677] [PATCH v4] gnu: Add ffmpeg-jami.

From: Mathieu Othacehe <[email-masked]>
Date: Thu, 14 May 2020 09:05:03 +0200

Hello Jan,

> +;; This procedure composes the configure flags list for ffmpeg-jami.
> +(define (ffmpeg-compose-configure-flags)
> +  (let* ((flags %ffmpeg-default-configure-flags)
> +         (append-flags (lambda (l)
> +                         (set! flags (append flags l))))

It's better to avoid using "set!".

> +         (system=? (lambda (s)
> +                     (string-prefix? %current-system s))))

This should be (%current-system), plus I think arguments should be
transposed.

> +    (if (string-contains %current-system "linux")
> +        (begin (append-flags %ffmpeg-linux-configure-flags)
> +               (cond ((or (system=? "i686")
> +                          (system=? "x86_64"))
> +                      (append-flags %ffmpeg-linux-x86-configure-flags))
> +                     ((system=? "x86_64")
> +                      (append-flags '("--arch=x86_64")))

If the first branch of the cond succeeds, we will never add this flag.

Plus, it seems than ffmpeg is able to detect the running system. So I
would suggest to do this:

--8<---------------cut here---------------start------------->8---
;; This procedure composes the configure flags list for ffmpeg-jami.
(define (ffmpeg-compose-configure-flags)
  (define (system=? s)
    (string-prefix? s (%current-system)))

  `(,@%ffmpeg-default-configure-flags
    ;; Add Linux specific flags.
    ,@(if (string-contains %current-system "linux")
          %ffmpeg-linux-configure-flags
          '())
    ,@(if (or (system=? "i686") (system=? "x86_64"))
          %ffmpeg-linux-x86-configure-flags
          '())))
--8<---------------cut here---------------end--------------->8---

What do you think?

Thanks,

Mathieu