start of the @Frames macro

[Wikunia]

PR Checklist

If you are contributing to Javis.jl, please make sure you are able to check off each item on this list:

• Did I update CHANGELOG.md with whatever changes/features I added with this PR?
• Did I make sure to only change the part of the file where I introduced a new change/feature?
• Did I cover all corner cases to be close to 100% test coverage (if applicable)?
• Did I properly add Javis dependencies to the Project.toml + set an upper bound of the dependency (if applicable)?
• Did I properly add test dependencies to the test directory (if applicable)?
• Did I check relevant tutorials that may be affected by changes in this PR?
• Did I clearly articulate why this PR was made the way it was and how it was made?

Link to relevant issue(s)
Closes #331

How did you address these issues with this PR? What methods did you use?
Based on our discussion in #334 I've created a @Frames macro.
One can now write prev_start() to refer to the previous starting frame of the last object or action. Similarly with prev_end.
These can be used like this in the @Frames macro.

Background(1:70, ground)
red_ball = Object(1:70, (args...) -> object(O, "red"), Point(100, 0))
act!(red_ball, Action(anim_rotate_around(2π, O)))
blue_ball = Object(@Frames(prev_start(),70), (args...) -> object(O, "blue"), Point(200, 80))
act!(blue_ball, Action(anim_rotate_around(2π, 0.0, red_ball)))
act!(blue_ball, Action(@Frames(prev_start(), 10), appear(:fade)))

The first parameter of @Frames is the start of the frame (same as when just writing a unit range but can be a function like prev_start() or something like prev_start()+10 or whatever)
The second parameter is the length of the object/action.

In the above example it's quite simple such that it can be easily achieved with unit ranges but one might want to do more complicated stuff like

objects = [
    Object(@Frames(prev_start()+2, stop=200), (args...) -> circle(O, 10, :fill), points[i]) for i in 1:npoints
]

which is now easier to write than previously:

objects = [
  Object(@Frames(prev_start()+2, stop=200), (args...) -> circle(O, 10, :fill), points[i]) for 
     (frame_start, i) in zip(1:2:(2 * npoints), 1:npoints)
]

Additionally it supports

startof(obj/act)
endof(obj/act)

which are similar to prev_start and prev_end but one can specify the object or action.

[Wikunia]

@codejaeger you might want to check whether this is enough for your work or whether I missed something.

[codecov]

Codecov Report

Merging #338 (4b08447) into master (3f2cad0) will increase coverage by 0.03%.
The diff coverage is 97.14%.

@@            Coverage Diff             @@
##           master     #338      +/-   ##
==========================================
+ Coverage   96.21%   96.24%   +0.03%     
==========================================
  Files          35       35              
  Lines        1452     1519      +67     
==========================================
+ Hits         1397     1462      +65     
- Misses         55       57       +2     

Impacted Files	Coverage Δ
src/structs/Action.jl	100.00% <ø> (ø)
src/structs/Object.jl	100.00% <ø> (ø)
src/structs/Frames.jl	96.55% <93.33%> (-3.45%)	⬇️
src/Javis.jl	96.36% <100.00%> (-0.02%)	⬇️
src/util.jl	97.02% <100.00%> (+1.79%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f2cad0...4b08447. Read the comment docs.

[codejaeger]

Hey @Wikunia , thanks for providing help on this issue! The requirements I was having for specifying actions on the node objects will be met by what you have provided in the PR. I will take a closer look into the code shortly and using the tests that I had prepared in the PR #334 I will be able to point out if anything might seem missing. Those tests might be a bit unreadable so I will mention the cases in verbose that I think should also be fulfilled with this PR.

[Wikunia]

I think we might want to add something like start(object_name) as well along side prev_start() to just have access to the frames of any object.

[Wikunia]

Did you do some more tests with this @codejaeger ?

[codejaeger]

Hey @Wikunia , yes I tried this PR for my purposes in graph animations. I was primarily looking for a way to provide a default case where the frame range for the next object can be calculated with respect to the starting frame of the previous object/action & an absolute finish frame. This PR achieves that.

There were a few more utilities that I think will be useful. For example parent_start for both objects and actions. If this was available, I would not need to pass an exact frame number to stop and could just do stop=parent_last() instead. For objects parent would be the background (objects) and for actions they should be the objects itself. I cannot think of a use case for prev_start(object) since if a user has a reference to the other object itself they could reorder creation of the objects to achieve it. We need to think if it would be useful or not.

These are probably the only use cases I can think of right now. I had written a bunch of tests to test the logical correctness of the relative frame computations in #334 . If you would like, I could rewrite them with this interface and update in this comment.

[Wikunia]

I think start(object) makes sense as the object can be created way before and reordering does not always help as one might want to write start(object)+300 or something along those lines. Regarding parent_end for objects it might be useful for layers or something but I think it's not necessary for background as one can simply use Background(1:nframes) and then just write , stop=nframes

[codejaeger]

Yes agreed, for backgrounds parent_start/end is not that useful.

[codejaeger]

@Wikunia , would it be possible to add just one other frame utility function prev_action_end which returns the last frame of the object on which an action is defined. I have a use case where I wanted to add actions serially to different objects one after another.

This requires the knowledge of the action frames across different objects. I think it would not be possible unless a reference to that action was available. This would be a problem in the implementation since the start argument of the macro is executed during rendering, but prev_action_end refers to the last action in the last object at the point of time of writing the code. A possible workaround could be this

a = Action(RFrames(1:5), appear(:fade))
act!(obj1, a)
act!(obj2, Action(@Frames(prev_action_end(a)+12), appear(:fade)))

with a possible implementation for prev_action_end being

function prev_action_end(a)
    return a.frames.frames[end] + PREVIOUS_OBJECT[1].frames.frames[1] - 1
end

Do you have an alternative around this?

[Wikunia]

Oh you're talking about applying it to different objects.
I'm not sure I understand it though. Do you mean you want it relative to the the new object or not?

So bascially

obj1 = Object(1:100...
obj2 = Object(50:100...
act!(obj1, Action(10:20...
act!(obj2, Action(@Frames(action_end(obj1), 20, ...

Should that be the same as

act!(obj2, Action(20:39...

so

act!(obj2, Action(GFrames(70:89)...

?
Hope it's not too confusing 😄

That should work as we compute the objects first as long as obj1 is defined obj2 as we compute the frames first for all objects and then for all actions ordered by their object.

[codejaeger]

Yes something like that but a little bit more flexible such that even if more actions were added to obj1 I would be able to get the frames for Action(10:20.. when I specify @Frames(action_end(obj1) i.e. get the last frame for the last action defined on the last object at the point of time of writing the code 😆 .

I could formulate this as a different problem of being able to give the animator a handle to the current active frame in the video i.e. the frame last used. This helps provide a sequential style for creating the video without worrying about global frames but the previous specification of the problem makes it more exact to what I need.

[Wikunia]

Oh based on when you call the code I missed that point. That will be quite hard to make at this stage. Maybe you can create a new issue for that and at least get this one going into Javis earlier then?

[codejaeger]

Yes, I am already working on an alternative but based on the use case below I think it might be useful.

for i in 1:8
    highlightNode(@Frames(prev_action_end()+5), graph, i)
end

I will open an issue for it.

[Wikunia]

And please give more information for the new issue without knowledge of your graph project 😉

[codejaeger]

@Wikunia can this PR be merged? I wanted to extend the CURRENT_* for a custom object type.

[Wikunia]

Feel free to merge this into your fork for now but yeah want to merge this soon to Javis. Any objections @TheCedarPrince ?

[Wikunia]

@Sov-trotter do you mind having a look at the error message here? I'm unsure how this happens and I can't reproduce it locally.

[Sov-trotter]

This seems weird. Could you try running the CI again?

[Wikunia]

Thanks for having a look. Unfortunately it has the same error as before.
I realized however that I should only delete the file after the stream was canceled. 😉

[TheCedarPrince]

Hey Ole, I am rather confused by the exact functionality of this PR. In some of my testing, I was trying to reason about where @Frames created objects were beginning and ending, and discovered that the following snippet produces different results:

using Javis

video = Video(600, 400)
Background(1:200, ground)

red_circ = Object(1:90, (args...)->circ("red"))
blue_circ = Object(@Frames(prev_start()+20, 70), (args...)->circ("blue"))
blue_circ = Object(@Frames(prev_start()+20, stop=90), (args...)->circ("blue"))

println(Javis.get_frames(blue_circ))

red_circ = Object(1:90, (args...)->circ("red"))
blue_circ = Object(21:90, (args...)->circ("blue"))
blue_circ = Object(41:90, (args...)->circ("blue"))

println(Javis.get_frames(blue_circ))

which gives:

include("tmp/tmp.jl")

nothing
41:90

So somehow, it seems like @Frames created objects are not the same as "normally" created objects. Could you explain to me what is happening here? I looked in the macro definition and it seems that you are setting the frame ranges to nothing. Why is that? Thanks!

[Wikunia]

Thanks for the review @TheCedarPrince
About your first question:
In some cases one can't know the frame range from certain objects when they get created. We have the preprocess_frames! function for that.

[Wikunia]

@Sov-trotter any idea why this might happen? https://github.com/Wikunia/Javis.jl/pull/338/checks?check_run_id=3142096015#step:7:422
I'll probably just rerun it later...

[Sov-trotter]

MIght be realted to https://trac.ffmpeg.org/ticket/6463#comment:16 ??

[Wikunia]

What this PR does which should probably be in a different one is:

• removing the gifs that are created and streamed to not add two gifs to the test directory which might potentially end up in Javis when one doesn't take care when one commits and pushes to GitHub. I might delete that from this PR.

[Wikunia]

@TheCedarPrince do you mind having another look at this i.e resolving conversations and let me know if there is anything I should change.

[TheCedarPrince]

LGTM!