Fix crash when exporting image sequence items

[peter-targett]

image-sequence.otio.zip

$ otioconvert -i image-sequence.otio -o image-sequence.xml
Traceback (most recent call last):
  File "/Users/petert/work/src/baselight/otio/branch-dev-bug-59757/build/baselight/DarwinArm-clang-1600/debug/Baselight-7.0.unknown.app/Contents/python/3.12-v7-venv/bin/otioconvert", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/Users/petert/work/src/baselight/otio/branch-dev-bug-59757/build/baselight/DarwinArm-clang-1600/debug/Baselight-7.0.unknown.app/Contents/python/3.12-v7-venv/lib/python3.12/site-packages/opentimelineio/console/otioconvert.py", line 273, in main
    otio.adapters.write_to_file(
  File "/Users/petert/work/src/baselight/otio/branch-dev-bug-59757/build/baselight/DarwinArm-clang-1600/debug/Baselight-7.0.unknown.app/Contents/python/3.12-v7-venv/lib/python3.12/site-packages/opentimelineio/adapters/__init__.py", line 203, in write_to_file
    return adapter.write_to_file(
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/petert/work/src/baselight/otio/branch-dev-bug-59757/build/baselight/DarwinArm-clang-1600/debug/Baselight-7.0.unknown.app/Contents/python/3.12-v7-venv/lib/python3.12/site-packages/opentimelineio/adapters/adapter.py", line 195, in write_to_file
    result = self.write_to_string(input_otio, **adapter_argument_map)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/petert/work/src/baselight/otio/branch-dev-bug-59757/build/baselight/DarwinArm-clang-1600/debug/Baselight-7.0.unknown.app/Contents/python/3.12-v7-venv/lib/python3.12/site-packages/opentimelineio/adapters/adapter.py", line 290, in write_to_string
    return self._execute_function(
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/petert/work/src/baselight/otio/branch-dev-bug-59757/build/baselight/DarwinArm-clang-1600/debug/Baselight-7.0.unknown.app/Contents/python/3.12-v7-venv/lib/python3.12/site-packages/opentimelineio/plugins/python_plugin.py", line 144, in _execute_function
    return (getattr(self.module(), func_name)(**kwargs))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/petert/work/src/otio-fcp-adapter/src/otio_fcp_adapter/fcp_xml.py", line 2017, in write_to_string
    _build_sequence_for_timeline(
  File "/Users/petert/work/src/otio-fcp-adapter/src/otio_fcp_adapter/fcp_xml.py", line 1272, in wrapper
    elem = func(item, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/petert/work/src/otio-fcp-adapter/src/otio_fcp_adapter/fcp_xml.py", line 1897, in _build_sequence_for_timeline
    _add_stack_elements_to_sequence(
  File "/Users/petert/work/src/otio-fcp-adapter/src/otio_fcp_adapter/fcp_xml.py", line 1955, in _add_stack_elements_to_sequence
    track_elements = _build_top_level_track(track, track_rate, br_map)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/petert/work/src/otio-fcp-adapter/src/otio_fcp_adapter/fcp_xml.py", line 1833, in _build_top_level_track
    _build_item(item, timeline_range, transition_offsets, br_map)
  File "/Users/petert/work/src/otio-fcp-adapter/src/otio_fcp_adapter/fcp_xml.py", line 1787, in _build_item
    return _build_clip_item(
           ^^^^^^^^^^^^^^^^^
  File "/Users/petert/work/src/otio-fcp-adapter/src/otio_fcp_adapter/fcp_xml.py", line 1272, in wrapper
    elem = func(item, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/petert/work/src/otio-fcp-adapter/src/otio_fcp_adapter/fcp_xml.py", line 1659, in _build_clip_item
    clip_item_e.append(_build_file(clip_item.media_reference, br_map))
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/petert/work/src/otio-fcp-adapter/src/otio_fcp_adapter/fcp_xml.py", line 1272, in wrapper
    elem = func(item, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/petert/work/src/otio-fcp-adapter/src/otio_fcp_adapter/fcp_xml.py", line 1519, in _build_file
    has_video = (os.path.splitext(url_path)[1].lower() not in audio_exts)
                                  ^^^^^^^^
UnboundLocalError: cannot access local variable 'url_path' where it is not associated with a value

Issue assuming url_path is set, which is not the case for OTIO image sequences.

Adding a simple test resolves the crash so a file can be exported, resulting export can now be imported into supporting NLEs.

[apetrynet]

Hi @peter-targett

Seems like the CI needs some updates with regards to the python version matrix. While we deal with that, could you please write tests to make sure new and old behavior works as expected?

Also, I know there are some old tickets related to image sequences and the FCP7 adapter in the main OTIO repo (Here's one) for instance
In your tests, have you successfully managed to import the resulting xml in an NLE or similar?

[peter-targett]

While we deal with that, could you please write tests to make sure new and old behavior works as expected?

I guess the old behaviour is, it crashes. Let me look at some test cases, though the change looks fairly harmless.

Also, I know there are some old tickets related to image sequences and the FCP7 adapter in the main OTIO repo (Here's one) for instance.

Ah, I didn't spot those over in the main repo, perhaps this bug should be to stop the crash today, leaving those to add/test full support for image sequences?

I didn't realise this adapter was still behind what was added in OTIO for some time. Perhaps I can help out when time.

In your tests, have you successfully managed to import the resulting xml in an NLE or similar?

I have checked files import into NLEs.

[apetrynet]

I guess the old behaviour is, it crashes. Let me look at some test cases, though the change looks fairly harmless.

Haha, yeah. Should of been more precise. What I meant, was of course making sure non image sequence behavior still behaves. For instance, I noticed GeneratorReference is mentioned in the same function. Will those still get through correctly?

It would be great if you're able to provide a sample output of your work as well. Just add it to the collection of sample data.

I have checked files import into NLEs.

That's great! Out of curiosity, how are image sequences represented in the fcp xml?

Thank you for work on this!

[peter-targett]

What I meant, was of course making sure non image sequence behavior still behaves. For instance, I noticed GeneratorReference is mentioned in the same function. Will those still get through correctly?

It would be great if you're able to provide a sample output of your work as well. Just add it to the collection of sample data.

I'll come up with a .otio that includes all the media references types and include the sample .xml output to the pull.

That's great! Out of curiosity, how are image sequences represented in the fcp xml?

AcademySoftwareFoundation/OpenTimelineIO#1352 (comment)

[peter-targett]

I'll come up with a .otio that includes all the media references types and include the sample .xml output to the pull.

Noticed the tests/sample_data/premiere_example.xml file has examples where clip items have video and/or audio tags, inclined to say this fix stands as it is.

[apetrynet]

If you pull the latest changes from the main branch, you should unlock the ci.

[apetrynet]

@peter-targett, does the xml file still work as expected in your NLEs after this change?

[peter-targett]

@peter-targett, does the xml file still work as expected in your NLEs after this change?

It does.

[apetrynet]

Hi, @peter-targett
Thanks for bearing with me.
This PR is close to landing. Just have a couple minor descriptive suggestions for our future selves.
Also, could you please add "W-O" to the "Image Sequence Reference" row of the feature matrix in the README.md file?

[apetrynet]

This looks good to me.
Thank you very much for your contribution @peter-targett !

[peter-targett]

My original change was relevant, it will still crash for generator and missing reference types; fixed in my repo.

[apetrynet]

Hmm.. That sounds like a more general issue not necessarily linked to image sequences?

Don't any of the previous tests cover these cases?

Perhaps further discussion should happen in its own issue as this PR is closed. Better visibility that way.