Skip to content

Fix orphan range queries with null parents - #2049

Open
Ishubhammohole wants to merge 3 commits into
AcademySoftwareFoundation:mainfrom
Ishubhammohole:fix-2044-null-parent
Open

Ishubhammohole wants to merge 3 commits into
AcademySoftwareFoundation:mainfrom
Ishubhammohole:fix-2044-null-parent

Conversation

@Ishubhammohole

Copy link
Copy Markdown

Summary

  • Return immediately from orphan Item and Transition parent-range queries instead of calling through a null parent.
  • Make composition ancestry traversal reject an orphan child without dereferencing a null composition, including when no error-status pointer is supplied.
  • Add a C++ regression covering clip, transition, and composition orphan-range paths with and without ErrorStatus.

The regression failed against the original implementation before the production fix.

Testing

  • cmake --build /tmp/otio-2044-build -j4 — passed
  • ctest --test-dir /tmp/otio-2044-build --output-on-failure -E "^test_bundle$" — 9/9 passed
  • ctest --test-dir /tmp/otio-2044-build --output-on-failure -R "^test_bundle$" --timeout 300 — 1/1 passed
  • python -m unittest tests.test_composition tests.test_transition — 50 passed
  • make test — 481 passed, 2 skipped
  • make lint — passed
  • make manifest — passed
  • clang-format --dry-run --Werror ... — not run; clang-format is not installed in this environment

Fixes #2044

@linux-foundation-easycla

linux-foundation-easycla Bot commented Sep 14, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

Signed-off-by: Shubham Mohole <95623059+Ishubhammohole@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Sep 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.56%. Comparing base (696150b) to head (a96cb7b).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2049      +/-   ##
==========================================
+ Coverage   83.52%   83.56%   +0.04%     
==========================================
  Files         182      182              
  Lines       13533    13554      +21     
  Branches     1255     1260       +5     
==========================================
+ Hits        11303    11326      +23     
+ Misses       2057     2055       -2     
  Partials      173      173              
Flag Coverage Δ
py-unittests 83.56% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/opentimelineio/composition.cpp 80.70% <100.00%> (+0.37%) ⬆️
src/opentimelineio/item.cpp 94.44% <100.00%> (+2.58%) ⬆️
src/opentimelineio/transition.cpp 100.00% <100.00%> (ø)
tests/test_composition.py 99.72% <100.00%> (+<0.01%) ⬆️

Continue to review full report in Codecov by Harness.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Shubham Mohole <95623059+Ishubhammohole@users.noreply.github.com>
@jminor

jminor commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Hi @Ishubhammohole - thanks, this PR is looking pretty good. We'll arrange for an OTIO committer to review it once the CLA requirement is met. Let us know if you have questions about that step.

Out of curiosity, how did you come across this issue, and what lead you to contribute to OTIO?

@Ishubhammohole

Copy link
Copy Markdown
Author

Hi @Ishubhammohole - thanks, this PR is looking pretty good. We'll arrange for an OTIO committer to review it once the CLA requirement is met. Let us know if you have questions about that step.

Out of curiosity, how did you come across this issue, and what lead you to contribute to OTIO?

Thanks! I came across #2044 while looking through OTIO’s open issues for C++ bugs where I could make a meaningful contribution. This one caught my attention because it involved a clear null-parent crash and had a reproducible edge case.

I decided to contribute to OTIO because I wanted to work on a mature C++ codebase and get more experience with real-world debugging, regression testing, and contributing to an established open-source project.

@jminor

jminor commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

@Ishubhammohole thanks - and how did you find OTIO?

@Ishubhammohole

Copy link
Copy Markdown
Author

@Ishubhammohole thanks - and how did you find OTIO?
I found OTIO through the Academy Software Foundation while exploring active open-source C++ projects. I was looking for projects with real production use and approachable issues where I could contribute, and OTIO stood out because of its C++ codebase and the interesting problems around timeline and media data. After going through the repository and open issues, I found #2044 and decided to dig into it.

@Ishubhammohole

Copy link
Copy Markdown
Author

Merged current upstream main without rewriting history and resolved the composition-test conflict by retaining both the orphan-range regression and the new nested-child regression. Rebuilt with CMake (OTIO_SHARED_LIBS=OFF, OTIO_PYTHON_INSTALL=OFF); all 10 C++ CTest tests passed. The initial shared-library build hit local macOS linker errors; the existing pip build directory also referenced an expired temporary CMake installation. No Python suite was rerun for this merge. This follow-up was prepared with Codex assistance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crashes when Clip parent is null

3 participants