search

imageio / imageio-binaries

Repo to place the (relatively small) binary libraries
144 stars 139 forks source link

Add aarch64 ffmpeg binary #17

Closed odidev closed 3 years ago

odidev commented 3 years ago

Added ffmpeg aarch64 binary from https://johnvansickle.com/ffmpeg/

FirefoxMetzger commented 3 years ago

This is a very nice contribution; thanks for that! I think we should add this library to the binaries.

That said, I am hesitant to merge this PR because we don't have good means of reviewing the binary. I'd feel much more comfortable if it came from one of the core members. I don't mean this in an offensive way; I just really want to avoid accidentally committing and serving a binary that might contain an unwanted payload (such as a virus or a backdoor) to a lot of people.

That said, if @almarklein is happy to merge this, we could go ahead. That said, it may be useful to discuss an good and secure procedure for adding binaries for missing architectures, since this is not an isolated issue (see also https://github.com/imageio/imageio/issues/628)

almarklein commented 3 years ago

Referencing https://github.com/imageio/imageio-ffmpeg/issues/52 where this contribution was discussed :)

FirefoxMetzger commented 3 years ago

@almarklein I just read through the discussion. I liked your suggestion of using a GH action for this; it can also use docker files to do the work, similar to what has been reported in the issue. I think we should make it a dedicated issue.

I understand that the contribution comes with an associated issue that describes the steps taken to create the binary. I also don't have any particular reason to believe that anything other than what was discussed in the issue has happened in the process of creating this binary. At the same time, I have no particular means of verifying (or reviewing) that what was discussed in the issue did happen in the process of creating this library (and nothing else).

I, again, don't mean this in an offensive way. In fact, @odidev seems like a really competent person (judging by the GitHub profile). It is mainly me considering the worst-case scenario and, potentially, being too conservative about it.

almarklein commented 3 years ago

You are right that we should be careful with this. I had better contributed the file myself. I just downloaded the file and compared to what was contributed in this PR, and the MD5 hashes are equal.

Eventually it would indeed be much better to build all binaries ourselves. Or to make use of a source that does this, which is more transparent.

FirefoxMetzger commented 3 years ago

I just downloaded the file and compared to what was contributed in this PR, and the MD5 hashes are equal.

You mean you compared the binary in this file and our binary? That's reassuring.

Eventually it would indeed be much better to build all binaries ourselves. Or to make use of a source that does this, which is more transparent.

Agreed. I can make this the next feature I'll be working on after the new API is merged and documented.

almarklein commented 3 years ago

You mean you compared the binary in this file and our binary? That's reassuring.

Yes, so it's about as good as having prepped this PR myself. Now we only have to assume.hope that this John van Sickle is not including evil code in the binaries that he publishes.