AAFWriter: added support for AAF user comments (#22)

[timlehr]

Fixes #22

Summarize your change.

I added support for translating user comments in the OTIO metadata onto the mob objects.

• clip["AAF"]["UserComments"] will be written onto the clip MasterMob
• clip.media_reference["AAF"]["UserComments"] will be written onto the clip MasterMob and overwrites clip comments
  -timeline["AAF"]["UserComments"] will be written onto the CompositionMob

Reference associated tests.

test_aaf_adapter.py

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.24%. Comparing base (74298e1) to head (09682e6).
Report is 5 commits behind head on main.

❗ Current head 09682e6 differs from pull request most recent head d9c5c87. Consider uploading reports for the commit d9c5c87 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #23      +/-   ##
==========================================
+ Coverage   90.12%   90.24%   +0.12%     
==========================================
  Files           3        3              
  Lines        1124     1138      +14     
==========================================
+ Hits         1013     1027      +14     
  Misses        111      111              

Flag	Coverage Δ
unittests	90.24% <100.00%> (+0.12%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jminor]

This else clause will catch strings, but also various object types, None, arrays, etc. Does using str() give you a Python description string for those? In particular it would be good to think through whether you want unexpected types to pass through as a best-guess string versus throwing an error. As a concrete example, I could imagine someone attempting to put a SerializableObject through here.

For strings, is there a length limit that should be checked?

[timlehr]

That broad conversion to string was intentional to make sure we can at least store, some form of the comment in there. Do you prefer for explicit type coverage and things like lists and other objects to fail? Right now it should give us a string with varying quality - depending on how __str__ is implemented for the object. I am not sure if there is a character limit for strings. @markreidvfx do you have the answer for that? Being able to put a SerializableObject object in there as JSON string would be handy to roundtrip studio-specific schemas.

[markreidvfx]

@timlehr I'll need to double check this, I don't think I've really tested property size limits in pyaaf. I naively assume its UINT16_MAX bytes but checking the the spec, it might actually be saying they want simple data properties to only be "64k bytes"? aaf-stored-format-spec.

ugh it hurts my head every time I try and read this spec :p

This would be the size of the string in bytes when encoded to utf16le including 2 NULL terminator bytes.
There is also a 17 byte header for indirect types like TaggedValues, so minus that too.

If the indirect type of the TaggedValue was set to a stream type, then the size limit would be around UINT64_MAX but I'm not sure if anything would be compatible with that other then pyaaf.

[timlehr]

Unless explicitly supported, unsupported data will be ignored with a warning now.

[timlehr]

@jminor @markreidvfx I changed the code to omit unsupported types of data in the comments with a warning. I also added some more test cases for unicode, lists, dicts and OTIO schemas.

[jminor]

Looks great! Thanks for addressing those notes :)

[markreidvfx]

LGTM
Can I request we rebase this on top of main? I personally find the complex merges very difficult reason about.