google / android-riscv64

Issues and discussions around RISC-V support in AOSP.
Apache License 2.0
206 stars 13 forks source link

external/libpng: optimization #37

Open enh-google opened 1 year ago

enh-google commented 1 year ago

there's arm/intel-specific .c source that uses neon or sse intrinsics; we'll want similar for risc-v.

dragostis commented 1 year ago

I'm currently taking a look at this one.

unicornx commented 1 year ago

one question: upstream of https://android.googlesource.com/platform/external/libpng/ is from chromium(https://chromium.googlesource.com/chromium/src/third_party/libpng/) or form it's official repo, which I think is https://github.com/glennrp/libpng?

unicornx commented 1 year ago

I'm currently taking a look at this one.

@dragostis how is it going at your end? I'm also interested on this porting for riscv.

enh-google commented 1 year ago

one question: upstream of https://android.googlesource.com/platform/external/libpng/ is from chromium(https://chromium.googlesource.com/chromium/src/third_party/libpng/) or form it's official repo, which I think is https://github.com/glennrp/libpng?

for most projects in external/ you should be able to look at the METADATA file to answer this question. in this case, it's https://android.googlesource.com/platform/external/libpng/+/refs/heads/master/METADATA which says github.

(it looks like chromium only has a couple of small changes on top of "true" upstream anyway? https://chromium.googlesource.com/chromium/src/third_party/libpng/+/refs/heads/main/patches/)

dragostis commented 1 year ago

@unicornx, I have a PR ready. It's currently sitting on SourceForge because I was under the impression that GitHub was just a mirror since many of the latest PRs have no comment on them.

I tried contacting the maintainer a couple of times and also the impl group without much success until now.

mschlaegl commented 1 year ago

Hi, I wanted to inform you that we have also implemented a libpng for RVV and evaluated the performance of different implementation candidates with our RVVRadar tool on an Allwinner D1 (RVV 0.7). Details can be found here:: https://github.com/mschlaegl/libpng_rvv-doc

There are also two older pull requests: https://github.com/glennrp/libpng/pull/405 https://github.com/libpng/libpng/pull/3 The original maintainer Glenn Randers-Patterson sadly passed away in 2018. For some time it looked like glennrp/libpng would no longer be actively maintained and the work would be moved to libpng/libpng. However, now it seems glennrp/libpng is active again.

enh-google commented 1 year ago

anyone familiar enough with upstream to know if/when they plan on releasing v1.6.40? (we prefer to stick to published releases, but if that's not likely any time soon we can try to switch to a SHA instead...)

dragostis commented 1 year ago

@enh-google it was already released.

enh-google commented 1 year ago

ah, that's unfortunate...

* f135775a Release libpng version 1.6.40
* afc6c595 Fix a build regression on Solaris
* e6c5bf46 Ensure that only one eXIf chunk is written in the entire datastream
* 8be5c147 Don't report a valid tRNS chunk if it was canceled
* f7abe3c4 Avoid a memory leak when allocation of a pCAL buffer fails
* 82097c21 Fix a doc typo in pnglibconf.dfa
* e519af8b cmake: Allow overwriting the debug postfix of library filenames
* efc96c9d cmake: Redo the fix for Clang support on Windows
* aab24fa1 Clean up CMakeLists.txt
* efa9c2e9 cmake: Rename the custom targets to have the png_ prefix
* c22ef3b2 Revert "cmake: Fix the Clang support on Windows"
* 66fede80 ci: Update the CI scripts
* 59fa76e1 ci: Add CMake+Ninja+Clang to the AppVeyor CI matrix
* 70fda183 cmake: Fix the Clang support on Windows
* aeb26da4 cmake: Use CMAKE_SHARED_LIBRARY_C_FLAGS in version script checks
* cd0ea2a7 Fix for universal binaries on macOS
* 9923515f Update the copyright year
* 61bfdb0c Update the configuration for Travis CI and AppVeyor CI
* 9db8cff6 ci: Fix verification under Cygwin Bash + CMake + Visual Studio; update
* b445aade ci: Rename the ci_* scripts
* 3c152a8e Fix typos
* 5a0b7e9c manuals: Remove references to libpngpf(3); update links
* b126f807 Reorganize and update the README file; add Markdown formatting
* 3c761b51 Bump version to 1.6.40.git

i'll take that update anyway (https://android-review.googlesource.com/c/platform/external/libpng/+/2636112/), but no riscv stuff in there :-(

unicornx commented 1 year ago

I am wondering which one of the below list would be the upstream for aosp:

enh-google commented 1 year ago

I am wondering which one of the below list would be the upstream for aosp...

see https://github.com/google/android-riscv64/issues/37#issuecomment-1584607217 where i already answered this question (both for libpng specifically, but also for all projects in external/).

unicornx commented 1 year ago

I am wondering which one of the below list would be the upstream for aosp...

see #37 (comment) where i already answered this question (both for libpng specifically, but also for all projects in external/).

Yes, I had already read your comment before. I asked again due to I am wondering if the upstream of libpng (which is mentioned in the METADATA as "https://github.com/glennrp/libpng.git") will be changed. From the input from mschlaegl, it looks something is changing......

enh-google commented 1 year ago

no, nothing is changing. (remember anyone can comment here!)

prashanthswami commented 7 months ago

Looking back through this thread, it seems a part of the confusion is that it's not particularly clear what glennrp/libpng refers to, because there's the original SourceForge, but our METADATA points to a Github. However, clicking through to that Github routes you to pnggroup/libbpng. Perhaps these folks took over the project formally?

At a glance, it does appear that pnggroup/libpng is the closest to glennrp/libpng, they seem to own the redirect, and they also seem to be the most active libpng fork.

@enh-google Should we consider updating the METADATA to just point to the redirected project?

@mschlaegl regarding those PRs on the project, that thread seems to have revived in December, is this something you still intend to pursue landing? (No pressure, just trying to avoid duplicate assigning this to others who might be interested.)

enh-google commented 7 months ago

@enh-google Should we consider updating the METADATA to just point to the redirected project?

yeah, that sgtm. i'll let @sadafebrahimi make that change (because i'm curious whether that's something that external-updater will be confused by, or whether it will just work?). looks like we're a couple of tagged releases behind too :-)

we should probably think about whether we should switch to the chromium upstream instead, but -- as we've seen with other projects -- that's a bit of a mixed bag.