dealing with zero rate clips

[DaneBettis]

Problem:
  It is possible for Heiro to write an xml that has clips with a timebase of zero.
  This causes the opentime.RationalTime object to have a rate of zero, which means
  that the otioview application will crash with a divide by zero error.

  It also means that we are allowing RationalTime to store value*(1/0) which is,
  in my opinion, a nonsensical value. One thing I don't know is if a value of zero
  in timebase is ever okay, or is used to signify a something else all together.

  I've included a sample file that will trigger the crash and the resulting 
  stacktrace at the end of this post. The sample file is a trimmed down 
  version of a file with the offending data that I encountered in a production
  published timeline at our studio. Once the bug is fixed and/or the data is
  fixed this Sample File will successfully load up in otioview.

Possible Solutions:
  01. Have an error message that says something to the effect of:
      "file {file} sequence {seq} track {track} clip {clip} has a frame rate of zero"
      " - please correct this in the source file."
      I'm not yet sure where the best place to put that in the code would be.
  02. Don't allow a RationalTime object to be initialized with a rate of zero
      and throw an error when it is attempted,
      -OR- replace any rate of 0 with a rate of 1 and emit an warning message.
  03. Add a try/catch clause for a rate of zero to RationalTime.value_rescaled_to()
      since we already catch type errors there,
      -OR- replace any rate of 0 with a rate of 1 and emit an warning message.
  04. Add mypy type annotations and/or contract decorators to prevent 
      this class of error from creeping back in unnoticed.
  05. Submit a bug report to hiero, asking why zero frame rates are allowed.
      Naturally, this is something I will do, but I list it here so we all know
      that I am doing it.

Instructions to reproduce error:
  01. on a linux machine, run this command as root: pip install OpenTimelineIO
  02. safe the Sample File bellow as ~/hiero_bad_fps.xml
  03. from the command line run: /usr/bin/otioview ~/hiero_bad_fps.xml
  04. you should see the traceback bellow.
  05. you can replace the value 30 with 0 for any timebase in this example
      and get the same error 

Sample File:
  <xmeml version="5">
    <sequence id="trailer">
      <rate>
        <ntsc>FALSE</ntsc>
        <timebase>30</timebase>
      </rate>
      <media>
        <audio>
          <track>
            <clipitem>
              <rate>
                <ntsc>FALSE</ntsc>
                <timebase>30</timebase>
              </rate>
              <in>0.0</in>
              <out>5579.0</out>
              <start>0</start>
              <end>5579</end>
              <file id="Audio_File_With_Bad_BaseRate">
                <pathurl>file://localhost/bad_audio.wav</pathurl>
                <duration>5579</duration>
                <timecode>
                  <rate>
                    <ntsc>FALSE</ntsc>
                    <timebase>0</timebase>
                  </rate>
                  <frame>0</frame>
                </timecode>
              </file>
            </clipitem>
          </track>
        </audio>
      </media>
    </sequence>
  </xmeml>
Sample File End.

SITE_PACK = /usr/lib64/python2.7/site-packages
Traceback (most recent call last):
  /usr/bin/otioview:<module>:10
    sys.exit(main())
  SITE_PACK/opentimelineview/console.py:main:280
    window.load(args.input)
  SITE_PACK/opentimelineview/console.py:load:198
    **self.adapter_argument_map
  SITE_PACK/opentimelineio/adapters/__init__.py:read_from_file:144
    **adapter_argument_map
  SITE_PACK/opentimelineio/adapters/adapter.py:read_from_file:130
    **adapter_argument_map
  SITE_PACK/opentimelineio/plugins/python_plugin.py:_execute_function:128
    return (getattr(self.module(), func_name)(**kwargs))
  SITE_PACK/opentimelineio/adapters/fcp_xml.py:read_from_string:1905
    sequences = parser.top_level_sequences()
  SITE_PACK/opentimelineio/adapters/fcp_xml.py:top_level_sequences:641
    return [self.timeline_for_sequence(s, context) for s in sequence_iter]
  SITE_PACK/opentimelineio/adapters/fcp_xml.py:timeline_for_sequence:683
    tracks = self.stack_for_element(media_element, local_context)
  SITE_PACK/opentimelineio/adapters/fcp_xml.py:stack_for_element:734
    track_element, track_kind, local_context
  SITE_PACK/opentimelineio/adapters/fcp_xml.py:track_for_element:797
    local_context,
  SITE_PACK/opentimelineio/adapters/fcp_xml.py:item_and_timing_for_element:970
    item_element, item_range, start_offset, context
  SITE_PACK/opentimelineio/adapters/fcp_xml.py:clip_for_element:1020
    file_element, local_context
  SITE_PACK/opentimelineio/adapters/fcp_xml.py:media_reference_for_file_element:849
    start_time = start_time.rescaled_to(media_ref_rate)
  SITE_PACK/opentimelineio/opentime.py:rescaled_to:101
    self.value_rescaled_to(new_rate),
  SITE_PACK/opentimelineio/opentime.py:value_rescaled_to:118
    return float(self.value) * float(new_rate) / float(self.rate)
ZeroDivisionError: float division by zero

[jminor]

Thanks for the detailed bug report and sample file! I agree that OTIO should be strict about throwing an error when this is first encountered. A zero rate is not valid.

I'm curious to know what causes this on the Heiro side. It could, as you suggest, indicate something meaningful - or it might just be a bug. Let us know what you find out from filing the bug against Heiro. (If possible, you can post a link to the bug report on this thread.)

[DaneBettis]

Thanks for the quick response. I'll let you all know as I discover more about the hiero xml from production.

[apetrynet]

@DaneBettis do you know if all audio files get a timebase of 0 or if it's just the one you use in your example? Also, what version of Hiero did you use for this?

[Shankusu993]

Hello there, As @jminor mentioned that a zero rate is not valid. I assume the best solution is to not allow a RationalTime object to be initialized with a rate less than or equal to zero. If I got this correctly it's a simple fix where we need to add a check for the same in rationalTime.h , Can I move ahead with it or am I mistaken somewhere?

[jminor]

If you can propose a way to prevent that case, we'd be happy to consider it. I suspect the difficulty will be finding the right layer of the system to indicate the problem without introducing too much overhead. For example, adding exception handling everywhere would likely be too much hassle. There is already an is_invalid_time() method, so maybe it is a matter of adding checks for that in the appropriate places.

Related issue: https://github.com/PixarAnimationStudios/OpenTimelineIO/issues/28

[Shankusu993]

We can add a check only at the time we <I>initialize</I> _rate from rate and throw an exception the moment the object gets created. What I mean to say is this:

I do not intend to check it after every change, but if need do that, then it's like you said just a matter of adding the is_invalid_time() checks everywhere.

[thecargocultnz]

I have just come across another instance of this issue in FCP7 XML form Premiere. In my case, clips with duration and rate of 0 are causing "NaN" to be written into the OTIO JSON string. This is not valid in JSON so there's an issue there - but more importantly we do need some way to handle these cases, preferably without just bailing out completely.

In my case, it is totally possible to extract the duration of the clip in other ways and assign a correct "master" duration, despite the invalid duration field and rate. GTTestXMLs.zip

Attaching a couple of examples - with and without transitions.