Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ jobs:
test:
strategy:
matrix:
elixir: ['1.8', '1.9', '1.10', '1.11']
elixir: ["1.8", "1.9", "1.10", "1.11"]

runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: actions/setup-elixir@v1
- uses: erlef/setup-elixir@v1
with:
otp-version: '22.3.4'
otp-version: "22.3.4"
elixir-version: ${{ matrix.elixir }}

- uses: actions-rs/toolchain@v1
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ erl_crash.dump
imageflow_ex-*.tar

native/imageflow_ex/target/
priv/
49 changes: 47 additions & 2 deletions lib/imageflow/graph.ex
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,18 @@ defmodule Imageflow.Graph do
|> append_node(%{decode: %{io_id: io_id}})
end

@doc """
Appends a string to be decoded
"""
@spec decode_string(t, binary) :: t
def decode_string(%{io_count: io_count} = graph, string) do
io_id = io_count + 1

graph
|> add_input(io_id, {:bytes, string})
|> append_node(%{decode: %{io_id: io_id}})
end

@doc """
Specifies a destination file for the current branch of the pipeline

Expand All @@ -112,11 +124,42 @@ defmodule Imageflow.Graph do
Check the official [encoding documentation](https://docs.imageflow.io/json/encode.html) to see the parameters available to each encoder
"""
@spec encode_to_file(t, binary, binary | atom, map) :: t
def encode_to_file(%{io_count: io_count} = graph, path, encoder \\ :png, opts \\ %{}) do
def encode_to_file(graph, path, encoder \\ :png, opts \\ %{}) do
add_encode_node(graph, {:file, path}, encoder, opts)
end

@doc """
Returns the converted image as a string.

No further processing operations should be appended at the current branch after this call.

The last two arguments specify the encoder and optional encoding parameters.

The following parameters are valid encoders:
* `:jpg`: Alias to `:mozjpeg`
* `:jpeg`: Alias to `:mozjpeg`
* `:png`: Alias to `:lodepng`
* `:webp: Alias to `:webplossless
* `:mozjpeg`
* `:gif`
* `:lodepng`: Lossless PNG
* `:pngquant`: Lossy PNG
* `:webplossy`: Lossy WebP
* `:webplossless`: Lossless WebP

Check the official [encoding documentation](https://docs.imageflow.io/json/encode.html) to see the parameters available to each encoder
"""

@spec encode_to_string(t, binary | atom, map) :: t
def encode_to_string(graph, encoder \\ :png, opts \\ %{}) do
add_encode_node(graph, :bytes, encoder, opts)
end

defp add_encode_node(%{io_count: io_count} = graph, output_type, encoder, opts) do
io_id = io_count + 1

graph
|> add_output(io_id, {:file, path})
|> add_output(io_id, output_type)
|> append_node(%{encode: %{io_id: io_id, preset: preset_for(encoder, opts)}})
end

Expand Down Expand Up @@ -312,6 +355,8 @@ defmodule Imageflow.Graph do
GraphRunner.run(graph)
end

def get_results(job, graph), do: GraphRunner.get_results(job, graph)

defp add_input(%{io_count: io_count, inputs: inputs} = graph, io_id, value) do
%{graph | io_count: io_count + 1, inputs: Map.put(inputs, io_id, value)}
end
Expand Down
33 changes: 29 additions & 4 deletions lib/imageflow/graph_runner.ex
Original file line number Diff line number Diff line change
@@ -1,14 +1,36 @@
defmodule Imageflow.GraphRunner do
alias Imageflow.{Graph, Native}
alias Imageflow.{Graph, Native, Result}

def run(%Graph{} = graph) do
with {:ok, job} <- Native.create(),
:ok <- add_inputs(job, graph.inputs),
:ok <- add_outputs(job, graph.outputs),
:ok <- send_task(job, graph),
:ok <- save_outputs(job, graph.outputs),
:ok <- Native.destroy(job) do
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this step got left out entirely.
I'm not sure where to fit it now, since I'm assuming we need to do this only after get_results, but it doesn't seem correct to never de-allocate the job

:ok
:ok <- save_outputs(job, graph.outputs) do
{:ok, job, graph}
end
end

def get_results(job, %Graph{outputs: outputs} = graph) do
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I'm the one who suggested this function in the first place, but the API doesn't look correct. As per Elixir standards, Graph should at least be the first argument.
But even with that, the fact that the caller needs to keep track of two separate values (the original graph, and the new job) makes this a clunky API

Also, coupling this with my other comment about Native.destroy being missing, wouldn't it be better to just make this a private function, and call it automatically in the original run flow?

outputs
|> Enum.reduce_while({:ok, []}, fn {id, value}, {:ok, _acc} ->
case value do
:bytes ->
case Native.get_output_buffer(job, id) do
{:ok, results} -> {:cont, {:ok, :binary.list_to_bin(results)}}
{:error, _} = error -> {:halt, error}
end

{:file, path} ->
{:cont, {:ok, path}}
end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be missreading this, but isnt' this resulting in an :output that corresponds just to whatever the last output from the job was?
I'm guessing it should instead build a map of all the existing outputs

end)
|> case do
{:ok, results} ->
{:ok, %Result{job: job, graph: graph, output: results}}

error ->
error
end
end

Expand All @@ -18,6 +40,7 @@ defmodule Imageflow.GraphRunner do
{id, value}, :ok ->
case value do
{:file, path} -> Native.add_input_file(job, id, path)
{:bytes, blob} -> Native.add_input_buffer(job, id, blob)
end
|> case do
:ok -> {:cont, :ok}
Expand All @@ -44,6 +67,8 @@ defmodule Imageflow.GraphRunner do
{id, value}, :ok ->
case value do
{:file, path} -> Native.save_output_to_file(job, id, path)
# skip
:bytes -> :ok
end
|> case do
:ok -> {:cont, :ok}
Expand Down
6 changes: 3 additions & 3 deletions lib/imageflow/native.ex
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ defmodule Imageflow.Native do
{:ok, resp} = Native.message("v0.1/get_image_info", %{io_id: 0})
"""

alias Imageflow.NIF
alias Imageflow.{Graph, NIF}

@type t :: %__MODULE__{}
@type native_ret_t :: :ok | {:error, binary}
Expand Down Expand Up @@ -62,12 +62,12 @@ defmodule Imageflow.Native do
NIF.job_save_output_to_file(id, io_id, path)
end

@spec get_output_buffer(t, number) :: {:ok, binary} | {:error, binary}
@spec get_output_buffer(t, number) :: {:ok, iolist} | {:error, binary}
def get_output_buffer(%__MODULE__{id: id} = _job, io_id) do
NIF.job_get_output_buffer(id, io_id)
end

@spec message(t, binary, binary) :: {:ok, any} | {:error, binary}
@spec message(t, binary, Graph.t()) :: {:ok, any} | {:error, binary}
def message(%__MODULE__{id: id}, method, message) do
with {:ok, resp} <- NIF.job_message(id, method, Jason.encode!(message)) do
{:ok, Jason.decode!(resp)}
Expand Down
3 changes: 3 additions & 0 deletions lib/imageflow/result.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
defmodule Imageflow.Result do
defstruct job: nil, graph: nil, output: nil
end
88 changes: 86 additions & 2 deletions test/imageflow/graph_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ defmodule Imageflow.GraphTest do
end
end

describe "decode_file/1" do
describe "decode_file/2" do
test "appends a new input" do
graph = Graph.new() |> Graph.decode_file("file.png")

Expand All @@ -23,7 +23,23 @@ defmodule Imageflow.GraphTest do
end
end

describe "encode_file/1" do
describe "decode_string/2" do
test "appends a new input" do
{:ok, string} = File.read("test/fixtures/elixir-logo.jpg")
graph = Graph.new() |> Graph.decode_string(string)

assert %{io_count: 1, inputs: %{1 => {:bytes, _string}}} = graph
end

test "appends a file decoding operation" do
{:ok, string} = File.read("test/fixtures/elixir-logo.jpg")
graph = Graph.new() |> Graph.decode_string(string)

assert %{nodes: %{1 => %{decode: %{io_id: 1}}}} = graph
end
end

describe "encode_to_file/2" do
test "appends a new output" do
graph = Graph.new() |> Graph.encode_to_file("file.png")

Expand Down Expand Up @@ -91,6 +107,74 @@ defmodule Imageflow.GraphTest do
end
end

describe "encode_to_string/2" do
test "appends a new output" do
graph = Graph.new() |> Graph.encode_to_string()

assert %{io_count: 1, outputs: %{1 => :bytes}} = graph
end

test "appends a file encoding operation" do
graph = Graph.new() |> Graph.encode_to_string()

assert %{nodes: %{1 => %{encode: %{io_id: 1}}}} = graph
end

test "allows appending jpg outputs" do
graph = Graph.new() |> Graph.encode_to_string(:jpg)

assert %{nodes: %{1 => %{encode: %{io_id: 1, preset: %{mozjpeg: %{quality: 90}}}}}} = graph
end

test "allows appending jpg outputs with custom parameters" do
graph = Graph.new() |> Graph.encode_to_string(:jpg, %{quality: 10})

assert %{nodes: %{1 => %{encode: %{io_id: 1, preset: %{mozjpeg: %{quality: 10}}}}}} = graph
end

test "allows appending png outputs" do
graph = Graph.new() |> Graph.encode_to_string(:png)

assert %{
nodes: %{
1 => %{encode: %{io_id: 1, preset: %{lodepng: %{maximum_deflate: false}}}}
}
} = graph
end

test "allows appending png outputs with custom parameters" do
graph = Graph.new() |> Graph.encode_to_string(:png, %{maximum_deflate: true})

assert %{
nodes: %{1 => %{encode: %{io_id: 1, preset: %{lodepng: %{maximum_deflate: true}}}}}
} = graph
end

test "allows appending gif outputs" do
graph = Graph.new() |> Graph.encode_to_string(:gif)

assert %{nodes: %{1 => %{encode: %{io_id: 1, preset: :gif}}}} = graph
end

test "allows appending gif outputs with custom parameters" do
graph = Graph.new() |> Graph.encode_to_string(:gif, %{a: :b})

assert %{nodes: %{1 => %{encode: %{io_id: 1, preset: :gif}}}} = graph
end

test "allows appending webp outputs" do
graph = Graph.new() |> Graph.encode_to_string(:webp)

assert %{nodes: %{1 => %{encode: %{io_id: 1, preset: :webplossless}}}} = graph
end

test "allows appending webp outputs with custom parameters" do
graph = Graph.new() |> Graph.encode_to_string(:webp, %{a: :b})

assert %{nodes: %{1 => %{encode: %{io_id: 1, preset: :webplossless}}}} = graph
end
end

describe "constrain/4" do
test "appends a constrain operation" do
graph = Graph.new() |> Graph.constrain(10, 20)
Expand Down
52 changes: 35 additions & 17 deletions test/integration/graph_test.exs
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
defmodule Imageflow.Integration.GraphTest do
use ExUnit.Case

alias Imageflow.{Graph, Native}
alias Imageflow.{Graph, Native, Result}

@input_path "test/fixtures/elixir-logo.jpg"
@output_path "/tmp/output.png"

test "can pipe multiple operations" do
assert :ok =
Graph.new()
|> Graph.decode_file(@input_path)
|> Graph.constrain(20, 20)
|> Graph.rotate_270()
|> Graph.transpose()
|> Graph.color_filter("invert")
|> Graph.encode_to_file(@output_path)
|> Graph.run()
run_result =
Graph.new()
|> Graph.decode_file(@input_path)
|> Graph.constrain(20, 20)
|> Graph.rotate_270()
|> Graph.transpose()
|> Graph.color_filter("invert")
|> Graph.encode_to_file(@output_path)
|> Graph.run()

assert match?({:ok, _job, _graph}, run_result)
end

test "can generate multiple images" do
Expand Down Expand Up @@ -47,12 +49,28 @@ defmodule Imageflow.Integration.GraphTest do
end

test "can handle multiple operations" do
assert :ok =
Graph.new()
|> Graph.decode_file(@input_path)
|> Graph.flip_vertical()
|> Graph.transpose()
|> Graph.encode_to_file("/tmp/rotated.png")
|> Graph.run()
run_result =
Graph.new()
|> Graph.decode_file(@input_path)
|> Graph.flip_vertical()
|> Graph.transpose()
|> Graph.encode_to_file("/tmp/rotated.png")
|> Graph.run()

assert match?({:ok, _job, _graph}, run_result)
end

test "can encode to file" do
{:ok, job, graph} =
Graph.new()
|> Graph.decode_file(@input_path)
|> Graph.flip_vertical()
|> Graph.transpose()
|> Graph.encode_to_string()
|> Graph.run()

{:ok, %Result{} = results} = Graph.get_results(job, graph)

assert is_bitstring(results.output)
end
end