From 69f535794b3c670b22802ae12cffbcdd8837fc30 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 24 Dec 2025 11:43:27 -0800 Subject: [PATCH] fix DecodeConfig in dynamic mode too Like #9, but for dynamic mode. And more tests. (including exporting a bool to let tests in callers like Perkeep exercise both the wasm + dynamic code paths) This also makes adds the common macOS homebrew libheif path as a location that's tried in dynamic mode, and installs it for tests in CI. This also disables dynamic mode on Windows, as it doesn't work yet. Updates #9 Updates #11 Signed-off-by: Brad Fitzpatrick --- .github/workflows/test.yml | 26 ++++++- decode_dynamic.go | 9 ++- decode_test.go | 153 ++++++++++++++++++------------------- go.mod | 2 +- go.sum | 4 +- heic.go | 13 +++- purego_darwin.go | 18 +++-- purego_windows.go | 21 +++-- 8 files changed, 143 insertions(+), 103 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9a34520..1bd504d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -4,7 +4,6 @@ jobs: test: strategy: matrix: - go-version: [1.23.x] os: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.os }} env: @@ -15,6 +14,27 @@ jobs: - name: Install Go uses: actions/setup-go@v5 with: - go-version: ${{ matrix.go-version }} + go-version-file: go.mod + - name: "[macos] install libheif" + if: runner.os == 'macOS' + run: brew install libheif + - name: "[linux] install libheif" + if: runner.os == 'Linux' + run: sudo apt-get update && sudo apt-get install -y libheif-dev libheif1 + - name: "[windows] cache vcpkg artifacts" + if: runner.os == 'Windows' + uses: actions/cache@v4 + id: vcpkg-cache + with: + path: C:\Users\runneradmin\AppData\Local\vcpkg\archives + key: vcpkg-libheif-v1 + - name: "[windows] install libheif" + if: runner.os == 'Windows' + run: | + vcpkg install libheif:x64-windows + vcpkg integrate install + dir C:/vcpkg/installed/x64-windows/bin + Get-ChildItem -Path "C:\vcpkg\installed\x64-windows\bin\*.dll" | Copy-Item -Destination . + dir - name: Test - run: go test + run: go test -v -bench=. -benchtime=1x diff --git a/decode_dynamic.go b/decode_dynamic.go index c0554cc..bb06020 100644 --- a/decode_dynamic.go +++ b/decode_dynamic.go @@ -19,8 +19,7 @@ func decodeDynamic(r io.Reader, configOnly bool) (image.Image, image.Config, err var data []byte if configOnly { - data = make([]byte, heifMaxHeaderSize) - _, err = r.Read(data) + data, err = io.ReadAll(io.LimitReader(r, heifMaxHeaderSize)) if err != nil { return nil, cfg, fmt.Errorf("read: %w", err) } @@ -188,6 +187,12 @@ func decodeDynamic(r io.Reader, configOnly bool) (image.Image, image.Config, err } func init() { + if runtime.GOOS == "windows" { + dynamic = false + dynamicErr = fmt.Errorf("dynamic library loading not supported on windows yet; see https://github.com/gen2brain/heic/issues/11") + return + } + var err error defer func() { if r := recover(); r != nil { diff --git a/decode_test.go b/decode_test.go index 277e792..f6dbe77 100644 --- a/decode_test.go +++ b/decode_test.go @@ -3,11 +3,12 @@ package heic import ( "bytes" _ "embed" - "fmt" "image" "image/jpeg" "io" "os" + "runtime" + "strconv" "sync" "testing" ) @@ -96,11 +97,23 @@ func TestDecodeGray(t *testing.T) { } } -func TestDecodeDynamic(t *testing.T) { +var inCI, _ = strconv.ParseBool(os.Getenv("CI")) + +func requireDynamic(t testing.TB) { if err := Dynamic(); err != nil { - fmt.Println(err) - t.Skip() + if runtime.GOOS == "windows" { + t.Skip("skipping dynamic library test on Windows in CI; it doesn't work yet: https://github.com/gen2brain/heic/issues/11") + } + if inCI { + t.Fatalf("libheif should be available in CI on %s, but got: %v", runtime.GOOS, err) + } + t.Helper() + t.Skipf("skipping dynamic library test; libheif not available: %v", err) } +} + +func TestDecodeDynamic(t *testing.T) { + requireDynamic(t) img, _, err := decodeDynamic(bytes.NewReader(testHeic), false) if err != nil { @@ -120,10 +133,7 @@ func TestDecodeDynamic(t *testing.T) { } func TestDecode8Dynamic(t *testing.T) { - if err := Dynamic(); err != nil { - fmt.Println(err) - t.Skip() - } + requireDynamic(t) img, _, err := decodeDynamic(bytes.NewReader(testHeic8), false) if err != nil { @@ -143,10 +153,7 @@ func TestDecode8Dynamic(t *testing.T) { } func TestDecode12Dynamic(t *testing.T) { - if err := Dynamic(); err != nil { - fmt.Println(err) - t.Skip() - } + requireDynamic(t) img, _, err := decodeDynamic(bytes.NewReader(testHeic12), false) if err != nil { @@ -166,10 +173,7 @@ func TestDecode12Dynamic(t *testing.T) { } func TestDecodeGrayDynamic(t *testing.T) { - if err := Dynamic(); err != nil { - fmt.Println(err) - t.Skip() - } + requireDynamic(t) img, _, err := decodeDynamic(bytes.NewReader(testGray), false) if err != nil { @@ -189,50 +193,34 @@ func TestDecodeGrayDynamic(t *testing.T) { } func TestImageDecode(t *testing.T) { - img, _, err := image.Decode(bytes.NewReader(testHeic8)) - if err != nil { - t.Fatal(err) - } + testBothWays(t, func(t *testing.T) { + img, _, err := image.Decode(bytes.NewReader(testHeic8)) + if err != nil { + t.Fatal(err) + } - err = jpeg.Encode(io.Discard, img, nil) - if err != nil { - t.Error(err) - } + err = jpeg.Encode(io.Discard, img, nil) + if err != nil { + t.Error(err) + } + }) } func TestDecodeConfig(t *testing.T) { - _, cfg, err := decode(bytes.NewReader(testHeic8), true) - if err != nil { - t.Fatal(err) - } - - if cfg.Width != 512 { - t.Errorf("width: got %d, want %d", cfg.Width, 512) - } - - if cfg.Height != 512 { - t.Errorf("height: got %d, want %d", cfg.Height, 512) - } -} - -func TestDecodeConfigDynamic(t *testing.T) { - if err := Dynamic(); err != nil { - fmt.Println(err) - t.Skip() - } - - _, cfg, err := decodeDynamic(bytes.NewReader(testHeic8), true) - if err != nil { - t.Fatal(err) - } + testBothWays(t, func(t *testing.T) { + cfg, err := DecodeConfig(bytes.NewReader(testHeic8)) + if err != nil { + t.Fatal(err) + } - if cfg.Width != 512 { - t.Errorf("width: got %d, want %d", cfg.Width, 512) - } + if cfg.Width != 512 { + t.Errorf("width: got %d, want %d", cfg.Width, 512) + } - if cfg.Height != 512 { - t.Errorf("height: got %d, want %d", cfg.Height, 512) - } + if cfg.Height != 512 { + t.Errorf("height: got %d, want %d", cfg.Height, 512) + } + }) } func TestDecodeSync(t *testing.T) { @@ -257,10 +245,7 @@ func TestDecodeSync(t *testing.T) { } func TestDecodeSyncDynamic(t *testing.T) { - if err := Dynamic(); err != nil { - fmt.Println(err) - t.Skip() - } + requireDynamic(t) wg := sync.WaitGroup{} ch := make(chan bool, 2) @@ -296,19 +281,35 @@ func (r smallChunkReader) Read(p []byte) (int, error) { } func TestDecodeConfigViaImagesPackage(t *testing.T) { - cfg, typ, err := image.DecodeConfig(smallChunkReader{bytes.NewReader(testHeic)}) - if err != nil { - t.Fatal(err) - } - if g, w := cfg.Width, 1346; g != w { - t.Fatalf("invalid width: got %d, want %d", g, w) - } - if g, h := cfg.Height, 1346; g != h { - t.Fatalf("invalid height: got %d, want %d", g, h) - } - if typ != "heic" { - t.Fatalf("invalid type; got %q; want %q", typ, "heic") - } + testBothWays(t, func(t *testing.T) { + cfg, typ, err := image.DecodeConfig(smallChunkReader{bytes.NewReader(testHeic)}) + if err != nil { + t.Fatal(err) + } + if g, w := cfg.Width, 1346; g != w { + t.Fatalf("invalid width: got %d, want %d", g, w) + } + if g, h := cfg.Height, 1346; g != h { + t.Fatalf("invalid height: got %d, want %d", g, h) + } + if typ != "heic" { + t.Fatalf("invalid type; got %q; want %q", typ, "heic") + } + }) +} + +// testBothWays runs fn in both wasm mode and dynamic library mode, if possible. +func testBothWays(t *testing.T, fn func(t *testing.T)) { + t.Run("wasm", func(t *testing.T) { + was := ForceWasmMode + ForceWasmMode = true + t.Cleanup(func() { ForceWasmMode = was }) + fn(t) + }) + t.Run("dynamic", func(t *testing.T) { + requireDynamic(t) + fn(t) + }) } func BenchmarkDecode(b *testing.B) { @@ -321,10 +322,7 @@ func BenchmarkDecode(b *testing.B) { } func BenchmarkDecodeDynamic(b *testing.B) { - if err := Dynamic(); err != nil { - fmt.Println(err) - b.Skip() - } + requireDynamic(b) for i := 0; i < b.N; i++ { _, _, err := decodeDynamic(bytes.NewReader(testHeic8), false) @@ -344,10 +342,7 @@ func BenchmarkDecodeConfig(b *testing.B) { } func BenchmarkDecodeConfigDynamic(b *testing.B) { - if err := Dynamic(); err != nil { - fmt.Println(err) - b.Skip() - } + requireDynamic(b) for i := 0; i < b.N; i++ { _, _, err := decodeDynamic(bytes.NewReader(testHeic8), true) diff --git a/go.mod b/go.mod index 743cc8d..c4cfcf8 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,6 @@ module github.com/gen2brain/heic go 1.23 require ( - github.com/ebitengine/purego v0.8.3 + github.com/ebitengine/purego v0.9.1 github.com/tetratelabs/wazero v1.9.0 ) diff --git a/go.sum b/go.sum index 39be1f7..0b3a443 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,4 @@ -github.com/ebitengine/purego v0.8.3 h1:K+0AjQp63JEZTEMZiwsI9g0+hAMNohwUOtY0RPGexmc= -github.com/ebitengine/purego v0.8.3/go.mod h1:iIjxzd6CiRiOG0UyXP+V1+jWqUXVjPKLAI0mRfJZTmQ= +github.com/ebitengine/purego v0.9.1 h1:a/k2f2HQU3Pi399RPW1MOaZyhKJL9w/xFpKAg4q1s0A= +github.com/ebitengine/purego v0.9.1/go.mod h1:iIjxzd6CiRiOG0UyXP+V1+jWqUXVjPKLAI0mRfJZTmQ= github.com/tetratelabs/wazero v1.9.0 h1:IcZ56OuxrtaEz8UYNRHBrUa9bYeX9oVY93KspZZBf/I= github.com/tetratelabs/wazero v1.9.0/go.mod h1:TSbcXCfFP0L2FGkRPxHphadXPjo1T6W+CseNNY7EkjM= diff --git a/heic.go b/heic.go index b1a7ecc..1eb3287 100644 --- a/heic.go +++ b/heic.go @@ -19,7 +19,7 @@ func Decode(r io.Reader) (image.Image, error) { var err error var img image.Image - if dynamic { + if dynamic && !ForceWasmMode { img, _, err = decodeDynamic(r, false) if err != nil { return nil, err @@ -39,7 +39,7 @@ func DecodeConfig(r io.Reader) (image.Config, error) { var err error var cfg image.Config - if dynamic { + if dynamic && !ForceWasmMode { _, cfg, err = decodeDynamic(r, true) if err != nil { return image.Config{}, err @@ -54,6 +54,15 @@ func DecodeConfig(r io.Reader) (image.Config, error) { return cfg, nil } +// ForceWasmMode, if true, forces using the WASM-based decoder even if a +// dynamic/shared library is available. +// +// This exists mainly for testing purposes. +// +// It is not safe to change this concurrently with any other use of this +// package. +var ForceWasmMode bool + // Dynamic returns error (if there was any) during opening dynamic/shared library. func Dynamic() error { return dynamicErr diff --git a/purego_darwin.go b/purego_darwin.go index 7c453a7..a858a32 100644 --- a/purego_darwin.go +++ b/purego_darwin.go @@ -3,8 +3,6 @@ package heic import ( - "fmt" - "github.com/ebitengine/purego" ) @@ -12,11 +10,15 @@ const ( libname = "libheif.dylib" ) -func loadLibrary() (uintptr, error) { - handle, err := purego.Dlopen(libname, purego.RTLD_NOW|purego.RTLD_GLOBAL) - if err != nil { - return 0, fmt.Errorf("cannot load library: %w", err) +func loadLibrary() (handle uintptr, err error) { + for _, path := range []string{ + libname, + "/opt/homebrew/lib/libheif.dylib", + } { + handle, err = purego.Dlopen(path, purego.RTLD_NOW|purego.RTLD_GLOBAL) + if err == nil { + return handle, nil + } } - - return uintptr(handle), nil + return 0, err } diff --git a/purego_windows.go b/purego_windows.go index dba8a35..00d84ae 100644 --- a/purego_windows.go +++ b/purego_windows.go @@ -11,11 +11,20 @@ const ( libname = "libheif.dll" ) -func loadLibrary() (uintptr, error) { - handle, err := syscall.LoadLibrary(libname) - if err != nil { - return 0, fmt.Errorf("cannot load library %s: %w", libname, err) +func loadLibrary() (handle uintptr, err error) { + paths := []string{ + libname, + "heif.dll", // what vcpkg builds & names it } - - return uintptr(handle), nil + var firstErr error + for _, path := range paths { + sysHandle, err := syscall.LoadLibrary(path) + if err == nil { + return uintptr(sysHandle), nil + } + if firstErr == nil { + firstErr = err + } + } + return 0, fmt.Errorf("cannot load library %s: %w", libname, firstErr) }