MasterMob Transcribing Changes#44
Conversation
|
@markreidvfx does this PR supersede this old one? AcademySoftwareFoundation/OpenTimelineIO#1063 |
|
FYI, we spent some time comparing new/old results for the time warp test suite, and some simple production examples. The changes made to the timecode look great, and the changes to the media references and metadata seem easy to accommodate in our (Pixar) pipeline. We need to run some more complex examples to understand the nesting changes, and we plan to run a large set of older production example AAFs through to look for any problems, but in general this new approach looks great. In this week's OTIO TSC meeting, I believe that @reinecke said Netflix is already using this updated code in their pipeline with good results. Thanks @markreidvfx! |
|
Heyo! Kevin from Netflix here. Just wanted to say hello and chime in to give some context on how we are utilizing this updated adapter since it was mentioned earlier that we've been using it. It's been giving us excellent results and has resolved several major bugs for us. However, we noticed a change in how markers are gathered. With the code in this PR, markers on source clips in Avid that fall within the visible range of the clip on the timeline are collected and attached to the clip in the OTIO timeline, even if they're not visible in the Avid timeline. edit: after chatting with mark, it's because of how markers are gathered from the mastermobs now. I understand this approach is quite opinionated and may not suit everyone's needs. I also don't want to assume that the marker behavior is not intended either, but I thought it was worth sharing our experience. Thanks for the great work on the adapter! 🤗 |
| mob_timeline.tracks.append(slot_track) | ||
|
|
||
| # master mobs don't have transitions so its safe to do this | ||
| _attach_markers(mob_timeline, indent) |
There was a problem hiding this comment.
Looks like this line will attach markers that are on the source clip but not visible in the timeline. It would be great to get an argument to turn this behavior on or off. Thanks!
p.s. however, it doesn't seem to resolve the issue for markers that are on a source subclip.
| result.source_range = source_range | ||
|
|
||
| track_number = slot_track.metadata.get("AAF", {}).get("PhysicalTrackNumber") | ||
| marker_track = None |
There was a problem hiding this comment.
I can confirm that removing this block below will not attach markers that are on source subclips but not visible in the aaf timeline. If this can be excluded when using the flag as well, that would be awesome. Thanks! 🙏
|
Nice work, Mark! |
|
FYI, it appears that this PR is included in Avid Media Composer 2024.12's OpenTimelineIO export feature. |
Signed-off-by: Mark Reid <markreid@netflixanimation.com>
Signed-off-by: Mark Reid <markreid@netflixanimation.com>
- Only MasterMob tracks can become otio clips - CompositionMob referenced by SourceClip become Stacks - SourceMobs become MediaReferences - More aggressively simpify - Cache prevously processed Mobs Signed-off-by: Mark Reid <markreid@netflixanimation.com>
- OperationGroup metadata only stored on effect - Improved naming of OperationsGroups Signed-off-by: Mark Reid <markreid@netflixanimation.com> more flake crap
Signed-off-by: Mark Reid <markreid@netflixanimation.com>
Signed-off-by: Mark Reid <markreid@netflixanimation.com>
Signed-off-by: Mark Reid <markreid@netflixanimation.com>
markreidvfx
force-pushed
the
mastermob_refactor_v1
branch
from
ac19f50 to
f09a971
Compare
|
This is now rebased on top of the current main branch! |
|
Yay! Thanks @markreidvfx - we'll give it a try. Do you plan any further changes before this is ready to land? |
timlehr
left a comment
timlehr
left a comment
There was a problem hiding this comment.
Looks great @markreidvfx! Thanks so much for all the work on this. I added some comments, but I approve it anyway, because those are more nice-to-haves. I think bumping the version and dropping Python versions could be a separate PR.
| dependencies = [ | ||
| "opentimelineio >= 0.17.0", | ||
| "pyaaf2>=1.4.0" | ||
| "pyaaf2>=1.7.0" |
There was a problem hiding this comment.
Should this be in a separate PR?
Might as well require pyaaf2>=1.7.1
| os: [ubuntu-latest, macos-13, windows-latest, macos-latest] | ||
| otio-version: ["main", "0.17.0"] | ||
| python-version: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12"] | ||
| python-version: [ "3.9", "3.10", "3.11", "3.12"] |
There was a problem hiding this comment.
Should this be in a separate PR?
| for c, child in enumerate(thing): | ||
| thing[c] = _simplify(child) | ||
|
|
||
| if isinstance(thing, otio.schema.Track): |
There was a problem hiding this comment.
Maybe we can break this into a separate function, we're getting pretty nested here.
| ): | ||
| raise AAFAdapterError( | ||
| "Wrong duration? {} should be {} in {}".format( | ||
| "Wrong duration? {} should be {} {} {} in {}".format( |
There was a problem hiding this comment.
Since we're all Python 3 now, we could use f-strings for these, make it more legible.
|
|
||
| for essence_group in _iter_slot_essence_groups(slot): | ||
| essence_group_items = [] | ||
| for component in essence_group: |
There was a problem hiding this comment.
Maybe we can break this loop out into another function. Pretty deep nesting.
|
@markreidvfx lets merge this for now to get a 2.0.0 out of the door - my comments are all really minor and we can address them later. This MR has been lingering for too long and most users already leverage it in their own builds it seems. |
lbarral
left a comment
lbarral
left a comment
There was a problem hiding this comment.
There's a somewhat high abstraction/indirection between the AAF model and the code as written here, which makes it complex to review from our perspective.
Nevertheless, there are limitations concerning what can be represented in AAF and what OTIO permits. One such limitation is the marker concept, which in the AAF data model, allows for event based and timeline based markes. The former will not have a duration/length specified (they occur in an instant) but the OTIO mapping makes them have length set to 1, therefore changing their nature. This is seen in https://github.com/OpenTimelineIO/otio-aaf-adapter/pull/44/files#diff-76feb0a6a0d1a0ee6f77159db124c3ef8956a95b1a667138f54d5f65caa8a813R910
We also see some abbreviations on how the start time code is being interpreted, which do not fully translate the logic that avid employs in the AAFs it produces.
Because the mob hierarchy that can be established in the AAF data model cannot be fully reproduced in the OTIO space, some interpretation will occur in both directions, which might mean that a roundtrip isn't always possible and/or producing the same result.
| if start_tc: | ||
| start = start + start_tc.start_time.rescaled_to(edit_rate) | ||
| duration_tc = start_tc.duration.rescaled_to(edit_rate) | ||
| duration = max(duration, duration_tc) |
There was a problem hiding this comment.
Why do we take longest duration between SourceClip and TC?
| target_urls = target_urls or [None] | ||
|
|
||
| if isinstance(mob.descriptor, aaf2.essence.MultipleDescriptor): | ||
| # stream protocol here |
|
|
||
| # This is custom behavior we probably shouldn't be doing this | ||
| # keeping it here to maintain some compatibly | ||
| unc_path = metadata.get("UserComments", {}).get("UNC Path", None) |
There was a problem hiding this comment.
This is an unreliable source of information - tagged values are specific to certain workflows and the Network Locator element from the FileDescriptor is a better source of information to retrieve the path to the essence file.
| continue | ||
|
|
||
| return False | ||
| if slot['PhysicalTrackNumber'].value == 1: |
There was a problem hiding this comment.
According to the AAF specification, any Mob can have a timecode slot/track - however, for Avid, we will always derive the timecode from the lower-level FileSourceMob (a.k.a. Physical Mob, i.e. the Mob that carries a PhysicalDescriptor). Then all the offsets between the MasterMob SourceClip - which defines the output /playback position of the master clip under analysis - and the referred PhysicalSource, via the FileSourceMob (top level essence mob, with a File Descriptor) are taken into account.
The PhysicalMob, for Avid generated media, shall only have a single Timecode slot, which in most cases have a PhysicalTrackNumber == 1, but there's no restriction, so we believe this validation might be too much.
This function seems to be used generically, and not only applied to Physical Mobs, which means the interpretation of the logic above has to be done in the context where it is being used from.
|
|
||
| # if the user manually sets the timecode the mastermob | ||
| # will have an timecode track that acts as a global offset | ||
| global_start_time = _get_mob_start_tc(mob) |
There was a problem hiding this comment.
As mentioned in other comments, the start time code seems to be taken from the MasterMob in this case, which is not expected in an "Avid file" - we expect the start TC will be computed from the relationship between the MasterMob source clip reference position relative to the FileSource and then Physical Source they are referring to.
Of course the above applies to the logic Avid uses in its master clip concept.
| # this track and use it for the Timeline's global_start_time` | ||
| timecode = _get_mob_start_tc(item) | ||
| if timecode: | ||
| result.global_start_time = timecode.start_time |
There was a problem hiding this comment.
This works here, because we're getting the start timecode from the composition, which is 'new material' so it will be separate from the master clips it uses.
I'll defer to @lbarral for their opinion, but I personally think it would be better to have the OTIO marker be 0 duration for an event based marker, with spanned markers in Avid mapped to OTIO markers with durations > 0. It would be great to maintain that distinction. |
|
After discussing with @markreidvfx and @reinecke, we decided to land this PR as is, since it's already In use successfully at multiple studios. To address comments and issues with this rewrite, we should aim for smaller more targeted PRs that have less of a review / maintenance overhead. Since this is a breaking change, we will coordinate a 2.0 release for the adapter in the coming months. Thank you @markreidvfx for all the amazing work on this. |
6 participants
This PR proposes an overhaul to how the adapter transcribes AAF files. This CHANGES the structure of the OTIO file being generated in a few ways. This WILL most likely break tools rely on the old metadata structure and therefore should be a major version increase. I believe this new structure is a better mapping of the AAF to OTIO.
Improvements included:
walkmethod. Files that previously failed due to this should work nowThis rework follows a few guiding principals. ONLY SourceClips on MasterMobs become OTIO Clips. SourceClips on SourceMobs become MediaReferences.
Here a Diagram of transcribing a MasterMob hierarchy. It shows what AAF objects get converted to what OTIO objects.
Only SourceClips on MasterMobs become OTIO Clips
ONLY SourceClips on MasterMobs become OTIO Clips.
There are strict rules in the AAF Spec that define what MasterMob slots can contain. They are only allowed to contain one or more EssenceGroups or SourceClips. These components are often inside a Sequence object. I've seen some case where MasterMobs contain Filler even though not in the spec. This PR should handle all these cases.
A SourceClip in CompositionsMob is only allowed to reference slots on MasterMobs (or another CompositionsMob in some cases). They aren't allowed to reference SourceMobs. When transcribing a CompositionMob and a SourceClip is encountered, the referenced MasterMob is completely converted into a separate OTIO Timeline. Then track they are referencing by SourceClip's slot_id is the cloned into the timeline being built. The clip(s) on the cloned track will contain all the MasterMob's metadata.
With the previous behaviour, it is a little messy on what becomes a OTIO Clip. Typically the MasterMob metadata would end up on a MediaReference. Now MasterMob metadata will ALWAYS be on the OTIO Clip. This makes it very simple and consistent to find UserComments and other bin metadata.
SourceClips on SourceMobs become MediaReferences
A SourceMob referenced by a MasterMob now becomes its own media reference. More SourceMob metadata is persevered including
EssenceDescriptorobjects. ANetworkLocatorwill get converted to atarget_url. If the SourceMobs EssenceDescriptor doesn't have aNetworkLocatora OTIOMissingReferenceis created.Each MediaReference has a
available_rangebase on the timecode found on the SourceMobs. This timecode propagated back to the OTIO Clip, so it too should have the correct start timecode.I'm also converting the MasterMob's
UNC PathUserComment into a MediaReference too because this was the old behaviour, I'm considering removing that behaviour or changing it to an option. I've seen people use other bin metadata to specify media reference urls, it might be more useful for this to a user supplied list of keys to look for additional urls.SubClip CompositionMobs become stacks
A SourceClip in a CompositionMob can also reference another CompositionMob. This is often referred to as a SubClip. The Mob these SourceClips reference are converted to a OTIO Timelines and the referenced Track gets cloned in a similar fashion to MasterMobs.
A SubClip can contain useful user metadata. To preserve this data, SourceClips that reference other CompostionMobs become OTIO Stacks. Special care is also taken to preserve any Markers found SubClips.
If the SubClip has only have 1 track they could be flatten to Track instead of Stack, but I choose not to do this. I was thinking it was better keep SubClips consistent.
Changes to OperationGroup
The other structural change that might break things is all OperationGroup metadata is now stored in the OTIO Effect. This allows flattening to preserve all effect data. OperationGroup are only flattened if their effect has one input. If the effect requires multiple inputs they remain a OTIO Stack.
Changes to Simplify
More attempts are made to simplify the file after transcribing is complete. This includes some additional flattening of OTIO tracks and combining effects if possible. Simplify also makes sure all stacks have tracks. I still think this code is pretty complex since it involves a lot of recursion. More testing and documenting is required here.
Still TODO
ComponentAttributeListClip Example
Here is a small condensed example from ALab of what the changes look like to an OTIO Clip.
Old Structure
{ "OTIO_SCHEMA": "Clip.2", "metadata": { "AAF": { "ClassName": "SourceClip", "ComponentAttributeList": { "_IMAGE_BOUNDS_OVERRIDE": "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\" ?><Bounds> <Framing>-800/1 -450/1 1600/1 900/1</Framing> <Valid>-800/1 -450/1 1600/1 900/1</Valid> <Essence>-800/1 -450/1 1600/1 900/1</Essence> <Source>-800/1 -450/1 1600/1 900/1</Source></Bounds>" }, "DataDefinition": { "Description": "Picture data", "Identification": "01030202-0100-0000-060e-2b3404010101", "Name": "Picture" }, "Length": 34, "Name": "mk020_0060", "SourceID": "urn:smpte:umid:060a2b34.01010105.01010f10.13000000.f5e701d0.98459605.de43f493.9ff4a1bd", "SourceMobSlotID": 1, "SourceMobUsage": "", "StartTime": 0 } }, "name": "mk020_0060", "source_range": { "OTIO_SCHEMA": "TimeRange.1", "duration": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 34.0 }, "start_time": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 1002.0 } }, "effects": [], "markers": [], "enabled": true, "media_references": { "DEFAULT_MEDIA": { "OTIO_SCHEMA": "ExternalReference.1", "metadata": { "AAF": { "ClassName": "MasterMob", "ConvertFrameRate": false, "CreationTime": "2022-09-23 05:23:43", "LastModified": "2022-09-23 07:30:31", "MobAttributeList": { "_COLOR_B": 38144, "_COLOR_G": 11776, "_COLOR_R": 14592, "_GEN": 1663918231, "_IMPORTSETTING": "__AttributeList", "_USER_POS": 0 }, "MobID": "urn:smpte:umid:060a2b34.01010105.01010f10.13000000.f5e701d0.98459605.de43f493.9ff4a1bd", "Name": "mk020_0060", "Slots": {}, "UserComments": { "UNC Path": "C:\\Users\\nathanla\\Desktop\\mk020_shots\\mk020_0060\\mk020_minicut_comp_scene_cut_intentcut_v001.0561.png" } } }, "name": "", "available_range": { "OTIO_SCHEMA": "TimeRange.1", "duration": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 34.0 }, "start_time": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 1002.0 } }, "available_image_bounds": null, "target_url": "file://C:/Users/nathanla/Desktop/mk020_shots/mk020_0060/mk020_minicut_comp_scene_cut_intentcut_v001.0561.png" } }, "active_media_reference_key": "DEFAULT_MEDIA" }New Structure
{ "OTIO_SCHEMA": "Clip.2", "metadata": { "AAF": { "ClassName": "MasterMob", "ConvertFrameRate": false, "CreationTime": "2022-09-23 05:23:43", "LastModified": "2022-09-23 07:30:31", "MediaKind": "Picture", "MobAttributeList": { "_COLOR_B": 38144, "_COLOR_G": 11776, "_COLOR_R": 14592, "_GEN": 1663918231, "_IMPORTSETTING": "__AttributeList", "_USER_POS": 0 }, "MobID": "urn:smpte:umid:060a2b34.01010105.01010f10.13000000.f5e701d0.98459605.de43f493.9ff4a1bd", "Name": "mk020_0060", "Slots": {}, "UserComments": { "UNC Path": "C:\\Users\\nathanla\\Desktop\\mk020_shots\\mk020_0060\\mk020_minicut_comp_scene_cut_intentcut_v001.0561.png" } } }, "name": "mk020_0060", "source_range": { "OTIO_SCHEMA": "TimeRange.1", "duration": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 34.0 }, "start_time": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 1002.0 } }, "effects": [], "markers": [], "enabled": true, "media_references": { "DEFAULT_MEDIA": { "OTIO_SCHEMA": "ExternalReference.1", "metadata": {}, "name": "UNC Path", "available_range": { "OTIO_SCHEMA": "TimeRange.1", "duration": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 34.0 }, "start_time": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 1002.0 } }, "available_image_bounds": null, "target_url": "file://C:/Users/nathanla/Desktop/mk020_shots/mk020_0060/mk020_minicut_comp_scene_cut_intentcut_v001.0561.png" }, "mk020_minicut_comp_scene_cut_intentcut_v001.0561.png": { "OTIO_SCHEMA": "ExternalReference.1", "metadata": { "AAF": { "ClassName": "SourceMob", "CreationTime": "2022-09-23 05:40:44", "EssenceDescription": { "ClassName": "ImportDescriptor", "Locator": {} }, "LastModified": "2022-09-23 05:40:44", "MobAttributeList": { "_PJ": "Polaris" }, "MobID": "urn:smpte:umid:060a2b34.01010105.01010f10.13000000.f8ecb58d.98459605.c930f493.9ff4a1bd", "Name": "mk020_minicut_comp_scene_cut_intentcut_v001.0561.png", "Slots": {} } }, "name": "mk020_minicut_comp_scene_cut_intentcut_v001.0561.png", "available_range": { "OTIO_SCHEMA": "TimeRange.1", "duration": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 34.0 }, "start_time": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 1002.0 } }, "available_image_bounds": null, "target_url": "file:////C/Users/nathanla/Desktop/mk020_shots/mk020_0060/mk020_minicut_comp_scene_cut_intentcut_v001.0561.png" }, "urn:smpte:umid:060a2b34.01010105.01010f10.13000000.01c3e84a.98459605.b414f493.9ff4a1bd": { "OTIO_SCHEMA": "ExternalReference.1", "metadata": { "AAF": { "ClassName": "SourceMob", "CreationTime": "2022-09-23 07:30:31", "EssenceDescription": { "BlackReferenceLevel": 16, "ClassName": "CDCIDescriptor", "CodingEquations": "CodingEquations_ITU709", "ColorPrimaries": "ColorPrimaries_ITU709", "ColorRange": 225, "ColorSiting": "CoSiting", "ComponentWidth": 8, "Compression": "04010202-7113-0000-060e-2b340401010a", "DataOffset": 393216, "DisplayHeight": 1080, "DisplayWidth": 1920, "DisplayXOffset": 0, "DisplayYOffset": 0, "EssenceBox": { "Height": "900", "PositionX": "-800", "PositionY": "-450", "Width": "1600" }, "FrameIndexByteOrder": 18761, "FrameLayout": "FullFrame", "FrameSampleSize": 0, "HorizontalSubsampling": 2, "ImageAlignmentFactor": 4096, "ImageAspectRatio": "16/9", "ImageSize": 6406144, "Length": 34, "Locator": {}, "OffsetToFrameIndexes64": 6541799, "ResolutionID": 1253, "SampleRate": "24", "SampledHeight": 1080, "SampledWidth": 1920, "SampledXOffset": 0, "SampledYOffset": 0, "SourceBox": { "Height": "900", "PositionX": "-800", "PositionY": "-450", "Width": "1600" }, "StoredHeight": 1080, "StoredWidth": 1920, "TransferCharacteristic": "TransferCharacteristic_ITU709", "ValidBox": { "Height": "900", "PositionX": "-800", "PositionY": "-450", "Width": "1600" }, "VerticalSubsampling": 1, "VideoLineMap": {}, "WhiteReferenceLevel": 235 }, "LastModified": "2022-09-23 07:30:31", "MobAttributeList": { "_ORIGINAL_FILEMOB_ID": "060a2b340101010501010f1013-000000-f5e7025598459605-a27cf4939ff4-a1bd", "_PJ": "ALAB_project" }, "MobID": "urn:smpte:umid:060a2b34.01010105.01010f10.13000000.01c3e84a.98459605.b414f493.9ff4a1bd", "Name": "", "Slots": {} } }, "name": "urn:smpte:umid:060a2b34.01010105.01010f10.13000000.01c3e84a.98459605.b414f493.9ff4a1bd", "available_range": { "OTIO_SCHEMA": "TimeRange.1", "duration": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 34.0 }, "start_time": { "OTIO_SCHEMA": "RationalTime.1", "rate": 24.0, "value": 1002.0 } }, "available_image_bounds": null, "target_url": "file://Pixar-Nexis/Tools_Media/Avid%20MediaFiles/MXF/octave.2/V01.632D60B2_1C3E801C3E84AV.mxf" } }, "active_media_reference_key": "DEFAULT_MEDIA" }