Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AAF Reader Metadata #739

Merged

Conversation

stefanschulze
Copy link
Contributor

No description provided.

Stefan Schulze added 2 commits June 24, 2020 15:51
update pyaaf2 pin to use 1.4.0 (latest pyaaf2 release). This contains some important fixes
(like understanding the pulldown AAF object) required for some upcoming AAF Reader changes.
…only do 1-pass transcribe.

This change to the AAF reader is mainly motivated by discovering relevant metadata from clips for some (special) case scenarios which have not been covered by the current code.

When traversing the AAF object-tree, at the point we reach a SourceClip, we try to discover the masterMob to then extract the metadata (UserComments) from the MasterMob.

The way we discover the MasterMob is that either the referenced mob in the SourceClip is the masterMob or we walk pack the parent-chain to find the masterMob.

However, there is an additional case, where the SourceClip can reference a CompositionMob which then references the masterMob. This was first discovered in one of our AAFs using subclips.

This aaf-reader-update now includes this CompositionMob discovery, so when subclips are used, the expected UserComments metadata is discovered properly.

In addition to this discovery there are two more scenarios:

The SourceClip can reference a CompositionMob, but instead of holding a masterMob, the CompositionMob itself told the UserComments. I believe this is a way AVID realizes sharing of the same metadata between SourceClips.

The SourceClip can reference a CompositionMob with UserComments AND hold a MasterMob. In this case the MasterMob seems to represent the valid UserComments, so in this scenario, we use the UserComments of the MasterMob.

I added three test-cases for the scenarios described above:

test_aaf_subclip_metadata()
test_aaf_composition_metadata()
test_aaf_composition_metadata_mastermob()

In addition I added another debug flag, transcribe_debug. If enabled, it will output the recursive traversal of
_transcribe(). This is helpful in order to see and understand the traversal-process.

Finally I removed the first transcribe-pass and instead of building up a cache of MasterMobs, transcribe the MasterMob in the second pass 'on-demand'. Since the first pass used to transcribe any AAF object
(and then basically threw away the results) this improves the time it takes to parse the AAF by a fairly large number (around 30-40%)
Copy link
Collaborator

@ssteinbach ssteinbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left notes suggesting we push some of the formatting towards pep8. I guess one question for the room is whether we want debugging code in the library (like transcribe_log -- is there a way to turn this on with an adapter argument? If not then I'd rather prune it. If there is, then we should include it).

Copy link
Collaborator

@jminor jminor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did another review pass - the changes look great. I marked all but a handful of the review comments as resolved. Could you take a look at the last few? They should show up in the "Conversation" tab, and inline in the "Files changed" tab.

@ssteinbach
Copy link
Collaborator

@jminor
Copy link
Collaborator

jminor commented Jun 26, 2020

Oh, you're right. I was going by this documentation which says it was introduced in 3.1: https://docs.python.org/3/library/unittest.html

@jminor jminor merged commit 4656de3 into AcademySoftwareFoundation:master Jun 30, 2020
@ssteinbach ssteinbach added this to the Public Beta 13 milestone Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants