Create Java Bindings #753
Create Java Bindings #753
Conversation
- Create class RationalTime
|
@meshula @jminor @ssteinbach @reinecke While going through opentime tests I found that: Here, while testing for faulty timecode formatting, we're actually getting a |
|
Does anyone recall why 01:00:13;23 is meant to raise a ValueError for 24fps? The semi-colon indicates non-drop-field-2, so roughly speaking NON_DROPFRAME_RATE is mostly correct, and 23 is valid frame at 24. |
My confusion here was because of the test title. It say's faulty formatted. That's why I thought we needed to check for INVALID_TIMECODE_STRING.
What does this mean? Why do we drop frames? And what is the difference between timestring and timecode? Does timecode include frames whereas timestring uses seconds and decimal point for something less than a second? |
|
RationalTime is done and it's tests are half done. Could someone take a quick glance and make suggestions if any? So that I can take care of them for rest of the bindings. |
|
I believe it's convention to use ";" semi-colon as an indicator of a drop-frame frame rate. So 24 fps should not have semi-colon while 23.976 should. From wikipedia
|
|
@apetrynet That's true about the semicolon, but the encoding wouldn't be 23.976. The SMPTE spec doesn't allow for partial frames - Normally the spec is here - but ieee.org seems non-responsive at the moment. https://ieeexplore.ieee.org/document/9097510 |
Summary
EDIT: More backgroundWhat is fps?FPS represents a sampling or playback rate for video. It is the number of video samples (frames) collected for every second of wall clock time. The baseline frame rate cinema playback has settled on is 24fps. PAL television (as used in the UK) is 25fps. NTSC RatesTelevision in the US was originally 30fps, but due to the method in which color information was added in the middle 20th century, the rate had to be slowed to 30000/1001 fps (commonly referred to as 29.97fps). There are other NTSC rates that exist and are similarly slowed down, the most common are:
What is timecode?Timecode is a scheme for labeling video frames. It uses a numbering system that closely relates to time but can deviate from time in certain case. It is a system based on discrete time (based on whole frame increments) as opposed to continuous time (can represent an "infinite" precision of time). Timecode is structured as Note that all fields in a regular video timecode are integers. Timecode is incremented by increasing the An aside: there are some corner cases about interlacing and "audio timecode" that can encode something like partial frames, but those are not considered in common use cases or OpenTimelineIO's support. How does frame rate relate to timecode?in timecode, frame rate is used to derive how many frames are enumerated before the Let's look at that again: 23.976fps has a timecode rate of 24. So this means, when you enumerate the 24th frame of a 23.976 fps video, your timecode will read Drop-frame timecode is a system of skipping certain timecodes in the labeling scheme to bring the timecode's hours, minutes, and seconds back into sync with "Wall clock" time. This was mostly developed to make timecode time match real time so people scheduling could use timecode runtime to slot out their broadcast schedules. It is kind of the same idea but inverse of how we add a date to leap-years to keep the calendar in sync with the earth's orbit. Because drop-frame timecode is just altering the labeling scheme to make NTSC rates match wall-clock time, it does not make sense to use it with non-NTSC timecode rates. Because some people prefer to have a consistent labeling scheme for their frames, it is allowed (and very common) to use non-drop frame timecode with NTSC rates. In this cases it is just accepted that the hours, minutes, and seconds will drift from wall-clock time. To re-cap:
|
|
Thanks for the detailed but simple explanation @reinecke ! That helped. |
|
Now I understand why the error is "NON_DROPFRAME_RATE". My rule of thumb is that if a line of code requires a lot of careful explanation when someone new comes across it, it deserves formal documentation at the call site. So @KarthikRIyer I think a couple of sentences that explain to the next person what the test is for are in order. @reinecke Since you've gone to the trouble of carefully explaining an important aspect of the system could I trouble you to pretty please transplant your excellent essay into our formal docs? This should be an appendix on readthedocs...! |
|
@meshula I couldn't agree more. This is an area where it's complex for a lot of very obscure reasons. I'll look into creating a documentation section that addresses timecode use in OTIO. |
|
Will do @meshula |
Codecov Report
@@ Coverage Diff @@
## master #753 +/- ##
==========================================
+ Coverage 83.21% 83.98% +0.76%
==========================================
Files 74 74
Lines 2884 3122 +238
==========================================
+ Hits 2400 2622 +222
- Misses 484 500 +16
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Opentime done! Moving on to OTIO. |
|
@margant Now all platform native libs are included in the JAR and everything builds and runs properly. Please take a look. I've commented out the macOS portions because I don't have a mac. The process to publish would be:
Does this look ok? @reinecke did you get a chance to try building on a different mac? |
|
@KarthikRIyer Just tried a build again on my home machine: |
|
Why is it ambiguous between these four types? :/ |
|
@reinecke could you please try to build once more after explicitly casting |
|
@KarthikRIyer That looks like it did it! This should have run the tests as well, right? |
|
@reinecke yes this should have run the tests as well. I'll make the change n push it. |
|
This is w.r.t Windows. D:\OpenTimelineIO\src\java-opentimelineio\src\main\cpp\io_opentimeline_OTIONative.cpp(217,49): warning C4311: 'reinterpret_cast': pointer truncation from 'T *' to 'long' [D:\OpenTimelineIO\src\java-opentimelineio\build\natives\src\main\cpp\jotio-1.0.0.vcxproj]
with
[
T=opentimelineio::v1_0::Track
]
D:\OpenTimelineIO\src\java-opentimelineio\src\main\cpp\io_opentimeline_OTIONative.cpp(217,49): warning C4302: 'reinterpret_cast': truncation from 'T *' to 'long' [D:\OpenTimelineIO\src\java-opentimelineio\build\natives\src\main\cpp\jotio-1.0.0.vcxproj]
with
[
T=opentimelineio::v1_0::Track
]
|
|
Hi Karthik, long and long long aren't guaranteed to be able to hold a pointer, hence the warning. uintptr_t is what you are looking for if you want an integer to hold a pointer, that will work on all platforms. If you want to be able to calculate offsets with pointers, use ptrdiff_t. |
|
@meshula thanks for the suggestion. It worked! But I thought, why not directly cast to |
|
I think so too. I must have commented just on a specific commit.
…On Sat, Sep 5, 2020 at 12:46 PM Karthik Ramesh Iyer < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
src/java-opentimelineio/src/main/java/io/opentimeline/opentimelineio/Composition.java
<#753 (comment)>
:
> @@ -149,6 +150,37 @@ public void setChildren(List<Composable> children, ErrorStatus errorStatus) {
public native HashMap<Composable, TimeRange> getRangeOfAllChildren(ErrorStatus errorStatus);
+ public <T extends Composable> Stream<T> eachChild(
+ TimeRange searchRange, Class<T> descendedFrom, boolean shallowSearch, ErrorStatus errorStatus) {
+ List<Composable> children;
+ if (searchRange != null) {
@margant <https://github.com/margant> I think you might've seen an older
commit. I actually made a change in a later commit. If you choose the
option to see commits from all changes you might be able to see it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#753 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD7DUUY6WF46SMBNVNRELNTSEKISBANCNFSM4OV5VK2Q>
.
|
|
@margant @meshula @reinecke do you think this is good to merge? This thread seems to have become very long. It's becoming a little difficult to find old conversations. Also @meshula what do I need to do if we want to separate this into a different repo? It might be easier to update the bindings if we use the OTIO core as a submodule instead. |
|
OpenTimelineIO as a submodule mages sense, lets proceed in that direction! |
I think there are two easy possibilities:
Either way we can take it to the TSC to see what the community thinks of that approach. Thoughts? |
|
Is there an OTIO GitHub org or do we create the repo under the ASWF org? |
|
PixarAnimationStudios/OpentimelineIO-java would be my vote, because part of the GSOC delivery requirements be that the code reside in an organization's repo, rather than the contributor's. In terms of splitting out the repo, for Imath, Owen @oxt3749 performed a magic trick whereby only Imath relevant commits landed in the new repo - thus preserving history and having it focussed only on Imath ~ perhaps he could tell us how he did it? |
|
@meshula AFAIK GSoC doesn't require the code to reside in the org's repo. My last GSoC code resides under my username. But I still prefer your suggestion. |
I can walk @KarthikRIyer through the git steps to filter the history, I've done that before. Let me do a little research on Tuesday and I'll set up the pieces after this weekend. |
|
@meshula @ssteinbach I used Now I can change the directory structure and use OTIO as submodule. |
|
Perfect - go ahead with that. I need to poke legal/ASWF to figure out where the right spot to land this is and what boiler plate needs to go into the repo so that its set up correctly from ASWF's point of view. Thanks @KarthikRIyer! |
|
Added a simple README. And GitHub Actions for ubuntu, windows and mac. https://github.com/KarthikRIyer/otio-java/actions/runs/242947255 |
|
Ok here you go: https://github.com/OpenTimelineIO/OpenTimelineIO-Java-Bindings Make sure you include a README.md and a .gitignore. |
|
@ssteinbach do I need to change these : Should the java bindings also follow the same versioning as OTIO? So for the next release it should be 0.14.0? |
|
Someone double check me, but generally we use: |
|
@KarthikRIyer Thanks again for all your hard work. For folks following along, the java bindings are now located at: https://github.com/OpenTimelineIO/OpenTimelineIO-Java-Bindings/ |
#694
Memory management needs some discussion. One way would be to delete the object in the
finalizemethod of the class and the other would be to provide the method to delete the object and let the end user handle everything.I'm using
finalizeto delete, for now.finalizeis deprecated as of Java 9. It can cause problems as mentioned here:and here.