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

Replace underlying Exif library #8586

Closed
chrillek opened this issue May 28, 2021 · 9 comments · Fixed by #12651
Closed

Replace underlying Exif library #8586

chrillek opened this issue May 28, 2021 · 9 comments · Fixed by #12651
Assignees
Milestone

Comments

@chrillek
Copy link

I suggest to replace the current Exif library, which has not been worked on in the last two years and poses some problems with
https://github.com/dsoprea/go-exif
That one is actively developed and seems to be able to read IPTC data, too. Which would open up a lot more possibilities for image handling.
Cf. https://discourse.gohugo.io/t/chance-of-changing-the-exif-module/33112

@bep bep added Enhancement and removed Proposal labels May 28, 2021
@bep bep added this to the v0.84 milestone May 28, 2021
@bep
Copy link
Member

bep commented May 28, 2021

I have labeled this GoodFirstIssue, which I'm not totally sure about -- but it's a very isolated issue that does not require deep knowledge of Hugo's internals.

Anyhow, PR is welcome.

We should try to preserve the existing API if possible. If we need to extend it for the IPTC data, we will do that.

@danedavid
Copy link

Hi, I'd like a shot at this.
From what I understand we need to remove this possibly outdated Exif library and add a new one, possibly https://github.com/dsoprea/go-exif , update all imports while keeping the API same, right?

@bep
Copy link
Member

bep commented Jun 1, 2021

update all imports while keeping the API same, right?

Yes, that would be a good approach.

danedavid added a commit to danedavid/hugo that referenced this issue Jun 3, 2021
Replace underlying EXIF library (rwcarlsen/goexif) with an actively developed one(dsoprea/go-exif). Add tests for disable options (date & location) while decoding EXIF.
Fixes gohugoio#8586
danedavid added a commit to danedavid/hugo that referenced this issue Jun 3, 2021
Replace underlying EXIF library (rwcarlsen/goexif) with an actively developed one(dsoprea/go-exif). Add tests for disable options (date & location) while decoding EXIF.
Fixes gohugoio#8586
danedavid added a commit to danedavid/hugo that referenced this issue Jun 4, 2021
Replace underlying EXIF library (rwcarlsen/goexif) with an actively developed one(dsoprea/go-exif). Add tests for disable options (date & location) while decoding EXIF.
Fixes gohugoio#8586
danedavid added a commit to danedavid/hugo that referenced this issue Jun 6, 2021
Replace underlying EXIF library (rwcarlsen/goexif) with an actively developed one(dsoprea/go-exif). Add tests for disable options (date & location) while decoding EXIF.
Fixes gohugoio#8586
@bep bep modified the milestones: v0.84, v0.85 Jun 18, 2021
danedavid added a commit to danedavid/hugo that referenced this issue Jun 28, 2021
Replace underlying EXIF library (rwcarlsen/goexif) with an actively developed one(dsoprea/go-exif). Add tests for disable options (date & location) while decoding EXIF.
Fixes gohugoio#8586
@bep bep modified the milestones: v0.85, v0.86 Jul 5, 2021
bep pushed a commit to bep/hugo that referenced this issue Jul 15, 2021
Replace underlying EXIF library (rwcarlsen/goexif) with an actively developed one(dsoprea/go-exif). Add tests for disable options (date & location) while decoding EXIF.
Fixes gohugoio#8586
bep added a commit to bep/hugo that referenced this issue Jul 15, 2021
bep added a commit to bep/hugo that referenced this issue Jul 15, 2021
bep added a commit to bep/hugo that referenced this issue Jul 16, 2021
Also, do not return an error when input format isn't supported. Just return `nil` exif.

Using the stream API makes it a little faster:

```
name           old time/op    new time/op    delta
DecodeExif-16    1.94ms ± 1%    1.81ms ± 1%   -6.74%  (p=0.029 n=4+4)

name           old alloc/op   new alloc/op   delta
DecodeExif-16    1.51MB ± 0%    1.13MB ± 0%  -25.32%  (p=0.029 n=4+4)

name           old allocs/op  new allocs/op  delta
DecodeExif-16     12.9k ± 0%     12.9k ± 0%   -0.15%  (p=0.029 n=4+4)
```

It's still much slower than what we had:

```
name           old time/op    new time/op    delta
DecodeExif-16     108µs ± 1%    1857µs ± 2%  +1624.52%  (p=0.029 n=4+4)

name           old alloc/op   new alloc/op   delta
DecodeExif-16     184kB ± 0%    1130kB ± 0%   +513.72%  (p=0.029 n=4+4)

name           old allocs/op  new allocs/op  delta
DecodeExif-16     1.20k ± 0%    12.91k ± 0%   +972.82%  (p=0.029 n=4+4)
```

See gohugoio#8586
bep added a commit to bep/hugo that referenced this issue Jul 18, 2021
Also, do not return an error when input format isn't supported. Just return `nil` exif.

Using the stream API and avoid creating the Ifd mapping and tag index every time makes it faster:

```
name           old time/op    new time/op    delta
DecodeExif-16    3.42ms ± 0%    0.65ms ± 0%  -81.12%  (p=0.029 n=4+4)

name           old alloc/op   new alloc/op   delta
DecodeExif-16    1.51MB ± 0%    0.59MB ± 0%  -60.77%  (p=0.029 n=4+4)

name           old allocs/op  new allocs/op  delta
DecodeExif-16     12.9k ± 0%      1.9k ± 0%  -85.48%  (p=0.029 n=4+4)
```

It's still slower than what we had, though, but correctness comes at a cost:

```

name           old time/op    new time/op    delta
DecodeExif-16     186µs ± 0%     648µs ± 1%  +249.21%  (p=0.029 n=4+4)

name           old alloc/op   new alloc/op   delta
DecodeExif-16     184kB ± 0%     593kB ± 0%  +222.38%  (p=0.029 n=4+4)

name           old allocs/op  new allocs/op  delta
DecodeExif-16     1.20k ± 0%     1.88k ± 0%   +55.99%  (p=0.029 n=4+4)
```

See gohugoio#8586
@bep bep modified the milestones: v0.86, v0.87, v0.88 Jul 26, 2021
@bep bep removed this from the v0.88 milestone Sep 2, 2021
@bep bep modified the milestones: v0.127.0, v0.128.0 Jun 8, 2024
@bep bep modified the milestones: v0.128.0, v0.129.0 Jun 21, 2024
bep added a commit to bep/hugo that referenced this issue Jul 9, 2024
bep added a commit to bep/hugo that referenced this issue Jul 9, 2024
bep added a commit to bep/hugo that referenced this issue Jul 10, 2024
bep added a commit to bep/hugo that referenced this issue Jul 12, 2024
bep added a commit to bep/hugo that referenced this issue Jul 12, 2024
bep added a commit to bep/hugo that referenced this issue Jul 12, 2024
bep added a commit to bep/hugo that referenced this issue Jul 14, 2024
bep added a commit to bep/hugo that referenced this issue Jul 18, 2024
bep added a commit to bep/hugo that referenced this issue Jul 19, 2024
bep added a commit to bep/hugo that referenced this issue Jul 19, 2024
bep added a commit to bep/hugo that referenced this issue Jul 20, 2024
bep added a commit to bep/hugo that referenced this issue Jul 20, 2024
bep added a commit to bep/hugo that referenced this issue Jul 20, 2024
bep added a commit to bep/hugo that referenced this issue Jul 20, 2024
bep added a commit to bep/hugo that referenced this issue Jul 20, 2024
bep added a commit to bep/hugo that referenced this issue Jul 20, 2024
bep added a commit to bep/hugo that referenced this issue Jul 20, 2024
@bep bep closed this as completed in 72ff937 Jul 20, 2024
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants