media
media copied to clipboard
Fix indeterminate z-order of EditedMediaItemSequences
Root Cause:
The indeterminate z-order issue arises due to the non-deterministic loading of assets. As each asset finishes loading, DefaultVideoCompositor.registerInput is invoked, linking the z-order to the unpredictable asset loading sequence. Consequently, the final z-order in the output is uncertain, as it's contingent on the load completion sequence of the assets.
Proposed Fix:
To resolve this, I've explored two options:
- Option A: Modify TransformerInternal.java to aggregate all loaded assets first, then sequentially invoke registerInput in the intended order.
- Option B: Introduce a sequenceIndex parameter to registerInput, decoupling the z-order from the invocation sequence and aligning it with this index instead.
Implemented Solution:
I opted for Option B due to its straightforward implementation. Post-fix, the z-order issue was no longer reproducible, indicating the effectiveness of this solution.
Links
I'm open to feedback and willing to assist further to ensure a robust resolution to this z-order inconsistency.
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.
Thank you for the quick PR! I'll try to take a look in the next few days. Also, cc @claincly who is pretty familiar with these files too
I have less familiarity than @claincly here so I'll defer to him, but just noting that without as much context, I think the general option B selected in this PR sounds good to me (haven't looked to make sure the implementation style/architecture makes sense though). Option A would also require adding additional state and/or blocking, so I'd prefer B over A.
@claincly I implemented the changes you requested. Please take a look and resolve it if it's OK. Let me know if there's anything else we need to change before this PR can be merged.
I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!
@claincly Any updates on this? Can I assist in any way?
Hey Aradi, we are discussing it internally as it's an API change, will keep you in the loop!
@claincly could you provide some information (if it's possible) when it potentially can be merged and released so that this fix is available to use in production?
The CL is still in review. We will try get back to you with a status update shortly