AcademySoftwareFoundation / OpenTimelineIO

Open Source API and interchange format for editorial timeline information.
http://opentimeline.io
Apache License 2.0
1.4k stars 276 forks source link

add rendering notes #1679

Closed meshula closed 6 months ago

meshula commented 7 months ago

This PR adds notes on rendering since rendering behavior is unclear from the specification itself.

codecov-commenter commented 7 months ago

Codecov Report

Merging #1679 (86b0074) into main (cbef407) will decrease coverage by 6.16%. Report is 81 commits behind head on main. The diff coverage is n/a.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1679/graphs/tree.svg?width=650&height=150&src=pr&token=mfnfUB4h7u&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation)](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1679?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) ```diff @@ Coverage Diff @@ ## main #1679 +/- ## ========================================== - Coverage 85.99% 79.84% -6.16% ========================================== Files 200 197 -3 Lines 20934 21792 +858 Branches 2459 4358 +1899 ========================================== - Hits 18002 17399 -603 + Misses 2331 2232 -99 - Partials 601 2161 +1560 ``` | [Flag](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1679/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | Coverage Δ | | |---|---|---| | [py-unittests](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1679/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | `79.84% <ø> (-6.16%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#carryforward-flags-in-the-pull-request-comment) to find out more. [see 132 files with indirect coverage changes](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1679/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1679?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1679?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Last update [cbef407...86b0074](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1679?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation).
darbyjohnston commented 7 months ago

What would also be really useful is the equivalent of the "project settings" in Final Cut Pro for establishing the output resolution, audio format, etc: https://support.apple.com/guide/final-cut-pro/final-cut-pro-project-settings-ver1b946a4ff/10.7/mac/13.5

Currently in tlRender I make a guess at the settings by using the video and audio from the first clip in the timeline. This works mostly OK but fails in some situations and also requires opening the media.

Properties_01 Properties_04

reinecke commented 7 months ago

I spun the project settings suggestion @darbyjohnston made into #1683 - I know it'd been talked about before but we'd failed to capture it as an issue. Thanks for highlighting it again!

meshula commented 7 months ago

@reinecke @darbyjohnston Does my edit on compositing ring true?

darbyjohnston commented 7 months ago

Looks good to me, thanks. For transparent clips that allow the background to show through, we assume the background color is black?

meshula commented 7 months ago

@darbyjohnston yes, I added a bit about zero values, and 100% values for alpha. Note that I specified zero, not black, because if we say "black" then it invites discussion about black levels and what we expect to happen for non-zero blacks. I also said 100% so that no one wonders if we mean "1", "1.0", or "255" or ...

darbyjohnston commented 7 months ago

Thanks, I think the note about "zero" is good so we don't get into color issues as you mentioned. Why the 100% alpha? For viewers it is nice to preserve the alpha channels for checking renders, though that could also be achieved by viewing the individual elements instead of the final composite.

reinecke commented 7 months ago

This update looks really good to me. On the background color, I think 100% opaque zero color values is a good baseline to match most NLEs. That said, if an application wants to allow people to override that behavior, there’s nothing stopping them.

meshula commented 6 months ago

If someone wants to go ahead and approve, we can merge.

darbyjohnston commented 6 months ago

It looks like the DCO check is failing...

meshula commented 6 months ago

right, if we accept changes in the online form, as we I did, the dco is not attached. heavy sigh.

meshula commented 6 months ago

Does anyone know a trick for repairing that? If not, I'm inclined to just merge, as there are not enough hours in the day to recreate a PR and go through the whole rigamarole again

reinecke commented 6 months ago

I forced DCO pass and merged - thanks for updating the docs @meshula