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

Add decoding and encoding to string for Graph #5

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
43 changes: 43 additions & 0 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 Down Expand Up @@ -120,6 +132,37 @@ defmodule Imageflow.Graph do
|> append_node(%{encode: %{io_id: io_id, preset: preset_for(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(%{io_count: io_count} = graph, encoder \\ :png, opts \\ %{}) do
io_id = io_count + 1

graph
|> add_output(io_id, :bytes)
|> append_node(%{encode: %{io_id: io_id, preset: preset_for(encoder, opts)}})
end
naps62 marked this conversation as resolved.
Show resolved Hide resolved

@doc """
Creates a new branch, allowing you do add multiple paths and outputs to the
graph
Expand Down
34 changes: 28 additions & 6 deletions lib/imageflow/graph_runner.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ defmodule Imageflow.GraphRunner do
: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, results} <- print_results(job, graph.outputs) do
if results == [], do: :ok, else: {:ok, results}
Copy link
Owner

Choose a reason for hiding this comment

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

I'm fine with a breaking change here, if a new result type needs to be used.

Returning different types based on the result leads to an inconsistent API.
I'd rather return {:ok, []}, or even {:ok, %Result{}} (with a result object that we could later mutate further if needed)`.

Copy link
Owner

Choose a reason for hiding this comment

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

Another option would be to just add a separate Graph.get_results(job) function to retrieve these values afterwards.

Might be worth checking how the .NET wrapper does it, since this one was mostly based on it

Copy link
Author

Choose a reason for hiding this comment

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

I actually liked the get_results idea, but I still had to change return structure.

end
end

Expand All @@ -18,6 +18,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 @@ -29,10 +30,13 @@ defmodule Imageflow.GraphRunner do
defp add_outputs(job, inputs) do
inputs
|> Enum.reduce_while(:ok, fn
{id, _}, :ok ->
with :ok <- Native.add_output_buffer(job, id) do
{:cont, :ok}
else
{id, value}, :ok ->
case value do
{:file, _path} -> Native.add_output_buffer(job, id)
:bytes -> Native.add_output_buffer(job, id)
naps62 marked this conversation as resolved.
Show resolved Hide resolved
end
|> case do
:ok -> {:cont, :ok}
{:error, _} = error -> {:halt, error}
end
end)
Expand All @@ -44,6 +48,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 All @@ -52,6 +58,22 @@ defmodule Imageflow.GraphRunner do
end)
end

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

defp send_task(job, graph) do
with {:ok, _response} <- Native.message(job, "v0.1/execute", graph) do
:ok
Expand Down
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