From 4823b1a757f81d2e5cc829eca7c050deffb27a7c Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Tue, 17 Oct 2023 14:02:40 -0400 Subject: [PATCH 1/2] fix memory corruption when reading palette The palette is an output parameter, so this was corrupting the Array structure. It would sort of accidentally always work before, but this is much better. --- src/io.jl | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/io.jl b/src/io.jl index e390268..8d6459f 100644 --- a/src/io.jl +++ b/src/io.jl @@ -191,24 +191,20 @@ function _load(png_ptr, info_ptr; gamma::Union{Nothing,Float64}=nothing, expand_ # Gamma correction is applied to a palette after `png_read_update_info` is called if read_as_paletted - # TODO: Figure out the length of palette before calling png_get_PLTEs - # `png_get_palette_max(png_ptr, info_ptr)` seems like a good option - # I can't make it work (I only ever get it to return zero). Note that it - # has to be called after png_read_image / png_read_png. - palette_length = Ref{Cint}() - palette_buffer = Vector{RGB{N0f8}}(undef, PNG_MAX_PALETTE_LENGTH) - GC.@preserve palette_buffer begin - png_get_PLTE(png_ptr, info_ptr, pointer_from_objref(palette_buffer), palette_length) - palette = palette_buffer[1:palette_length[]] - end + palette_length = Ref{Cint}(0) + palette = Ref{Ptr{PNGFiles.png_color_struct}}(C_NULL) + png_get_PLTE(png_ptr, info_ptr, palette, palette_length) + palette = unsafe_wrap(Array, Ptr{RGB{N0f8}}(palette[]), palette_length[]) if valid_tRNS - alpha_buffer = Vector{N0f8}(undef, palette_length[]) - alphas_cnt = Ref{Cint}() - GC.@preserve alpha_buffer begin - png_get_tRNS(png_ptr, info_ptr, pointer_from_objref(alpha_buffer), alphas_cnt, C_NULL) - end - alpha_buffer[alphas_cnt[]+1:palette_length[]] .= one(N0f8) # palette entries are opaque by default - palette = map(RGBA, palette, alpha_buffer) + alpha_buffer = Ref{Ptr{UInt8}}(C_NULL) + alphas_cnt = Ref{Cint}(0) + png_get_tRNS(png_ptr, info_ptr, alpha_buffer, alphas_cnt, C_NULL) + alpha = fill(one(N0f8), palette_length[]) # palette entries are opaque by default + @assert palette_length[] >= alphas_cnt[] + GC.@preserve alpha unsafe_copyto!(pointer(alpha), Ptr{N0f8}(alpha_buffer[]), min(palette_length[], alphas_cnt[])) + palette = map(RGBA, palette, alpha) + else + copy(palette) end buffer_eltype = UInt8 end From a4c4545d51263a817cee26f2d487b1d11e80755b Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Tue, 17 Oct 2023 19:37:39 -0400 Subject: [PATCH 2/2] Update io.jl --- src/io.jl | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/io.jl b/src/io.jl index 8d6459f..37b1f49 100644 --- a/src/io.jl +++ b/src/io.jl @@ -192,9 +192,10 @@ function _load(png_ptr, info_ptr; gamma::Union{Nothing,Float64}=nothing, expand_ # Gamma correction is applied to a palette after `png_read_update_info` is called if read_as_paletted palette_length = Ref{Cint}(0) - palette = Ref{Ptr{PNGFiles.png_color_struct}}(C_NULL) - png_get_PLTE(png_ptr, info_ptr, palette, palette_length) - palette = unsafe_wrap(Array, Ptr{RGB{N0f8}}(palette[]), palette_length[]) + palette_buffer = Ref{Ptr{PNGFiles.png_color_struct}}(C_NULL) + png_get_PLTE(png_ptr, info_ptr, palette_buffer, palette_length) + palette = Vector{RGB{N0f8}}(undef, palette_length[]) + GC.@preserve palette unsafe_copyto!(pointer(palette), Ptr{RGB{N0f8}}(palette_buffer[]), length(palette)) if valid_tRNS alpha_buffer = Ref{Ptr{UInt8}}(C_NULL) alphas_cnt = Ref{Cint}(0) @@ -203,14 +204,12 @@ function _load(png_ptr, info_ptr; gamma::Union{Nothing,Float64}=nothing, expand_ @assert palette_length[] >= alphas_cnt[] GC.@preserve alpha unsafe_copyto!(pointer(alpha), Ptr{N0f8}(alpha_buffer[]), min(palette_length[], alphas_cnt[])) palette = map(RGBA, palette, alpha) - else - copy(palette) end buffer_eltype = UInt8 end # We transpose to work around libpng expecting row-major arrays - buffer = Array{buffer_eltype}(undef, width, height) + buffer = Matrix{buffer_eltype}(undef, width, height) @debug( "Read PNG info:",