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

Fix wrong retimed detection on image sequence clip. #897

Merged
merged 42 commits into from
Oct 3, 2024

Conversation

robin-ynput
Copy link
Contributor

@robin-ynput robin-ynput commented Sep 17, 2024

Changelog Description

This PR addresses the bug described on #895 regarding retiming OTIO clip from image sequence.

  • Need to conform source_range to available_range rate
  • Retiming logic was when input media does not start 0 (image sequence, metadata...)
  • Ensure embedded TC are treated correctly
  • Need to preserve accurate discreet frame number when dealing with img sequences

Testing notes:

Environment prep

  1. copy paste items form both tool.poetry.dependencies and ayon.runtimeDependencies sections from ayon-core\client\pyproject.toml into root level pyproject.toml under tool.poetry.dev-dependencies
  2. remove ayon-core\poetry.lock
  3. run .\tools\manage.ps1 create-env (windows)

Your .poetry and .venv should be installed in root of repository

Testing

  1. Run the unit tests
    run .\tools\manage.ps1 run pytest .\tests\client\ayon_core\pipeline\editorial\ (windows)

AY-979

@robin-ynput robin-ynput marked this pull request as ready for review September 17, 2024 22:44
@jakubjezek001
Copy link
Member

jakubjezek001 commented Sep 18, 2024

For the testing data, we also need to check the retime effects (LinearTimeWarp, FreezeFrame, TimeEffect). Since the method name relates to retiming, this will cover the full range of functions. Great job on practically developing the unittest we really need! I'm thinking about whether we should add the test to the appropriate folder. @kalisp and @antirotor could lend a hand here, especially since ayon-core doesn't have a test folder yet.

@jakubjezek001 jakubjezek001 assigned kalisp and unassigned robin-ynput Sep 18, 2024
@jakubjezek001
Copy link
Member

Before the introduction of Resolve's native OTIO, we mainly relied on two attributes:

  • The clip's source start frame as a relative range to the available media range.
  • The clip's media range as an absolute range, including the metadata timecode value.

With Resolve's interpretation of OTIO now in play, we consider:

  • The clip's source start frame refers to a trimmed absolute range that is available.
  • The media range remains absolute, including metadata.

We reference the media source to an absolute value with included metadata because of FFMPEG. Since we use FFMPEG for video trimming, it's crucial to have the timecode correctly defined.

First, figure out if the clip source range is relative or absolute to the media range. Next, find the relative source frame value and continue as before, making sure to clear up any confusing code that's mistakenly referring to me (old me 😄).

@antirotor
Copy link
Member

... especially since ayon-core doesn't have a test folder yet.

but it can - I added one for my own tests, see feature/OP-1188_better-representation-model branch.

@robin-ynput robin-ynput self-assigned this Sep 18, 2024
@jakubjezek001 jakubjezek001 removed their assignment Sep 30, 2024
Copy link
Member

@iLLiCiTiT iLLiCiTiT left a comment

Choose a reason for hiding this comment

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

Code looks ok. @jakubjezek001 merge if is ready please.

iLLiCiTiT and others added 4 commits October 2, 2024 11:30
Fix extract_otio_review extraction logic for corner-case media
for project_name\nUpdate Anatomy NamedTuple to specify project_name as string type. This change ensures consistency and clarity in the codebase.
Copy link
Member

@jakubjezek001 jakubjezek001 left a comment

Choose a reason for hiding this comment

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

Tests in Hiero didn't go well.

Copy link
Member

@jakubjezek001 jakubjezek001 left a comment

Choose a reason for hiding this comment

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

Fantastic work @robin-ynput ! Thanks a lot! 💫 💥

@iLLiCiTiT iLLiCiTiT merged commit 51d806b into develop Oct 3, 2024
1 check passed
@iLLiCiTiT iLLiCiTiT deleted the bugfix/#895_fix_otio_subset_resources_img_sequence branch October 3, 2024 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/S type: bug Something isn't working
Projects
Archived in project
7 participants