add validateAndAdjustResourceDurations method for Imperor #383
Conversation
| long remainder = 0; | ||
|
|
||
| if (resourceEditRateDenominator == 1001) { | ||
| // Non-integer frame rates must be evenly divisible by 5 in order to align with audio samples |
There was a problem hiding this comment.
Unfortunately, the math gets more complicated than this.
The issue isn't entirely related to non-integer rates. But instead, it is caused when the rates of two tracks are not multiples with one another, such that each video frame ends exactly on the bounds of an audio sample.
That means that both the video AND audio rate are relevant. Luckily, the IMF core constraints limit the audio sample rate to only a few rates that are easy to work with (2067-2:2020 5.3.2.2). So really, we only need to worry about the video frame rate.
Most of the common non-integer video frame rates actually are compatible with the allowed audio sample rates. 24000/1001 fps to 48kHz works out to be exactly 2002 samples-per-frame.
The only common frame rates that are really problematic are 30000/1001 and 60000/1001. Thats where you end up needing to align on every fifth video frame.
But this check should still allow non-integer frame rates for 23.976 and 47.952
| long totalResourceDuration = resources.stream() | ||
| .mapToLong(resource -> { | ||
| if (resource != null) { | ||
| return resource.getSourceDuration().longValue(); |
There was a problem hiding this comment.
I believe you should use resource.getDuration() instead of getSourceDuration().
Although it is pretty uncommon, it is possible for a Resource to set a 'repeatCount'. The getDuration method will include the total for all repeats.
|
Maybe it would be better if, rather than automatically adjusting the duration of the last Resource, you were to simply return an error, or throw an Exception? That would allow/require the caller to make decisions about how they want to fix the timeline they provided. For your specific business case, what you wrote makes sense. You are always willing to drop the last few units to make things line up. We shouldn't presume that making cuts to the end is always best. Since Photon is intended to be an open source, general purpose IMF tool, it is always good to try to avoid letting our own business logic dictate the "default" behavior for non-Netflix users. |
|
|
||
| // Validate and adjust the durations of the resources in the track | ||
| List<IMFTrackFileResourceType> adjustedResources = SequenceDurationValidator.validateAndAdjustResourceDurations(resources, simpleTimeline.getEditRate().getNumerator(), simpleTimeline.getEditRate().getDenominator()); | ||
|
|
There was a problem hiding this comment.
It might be worth considering performing the duration validation on "all tracks" together, rather than on "all Resources within one track". So maybe after the virtualTracks list has been populated, but before the buildIMP call.
One advantage of this, is that it would catch other kinds of track duration mismatches also. Not just fractional edit units due to EditRate mismatches, but it would also be able to catch a lot of user-error, from the provided timeline.
As far as I can tell, SimpleTimeline doesn't have any sort of track duration validation. So it would be possible for the caller to (mistakenly) provide a video track with duration 30 seconds, and an audio track with duration 35 seconds.
This isn't a high priority, and is outside the scope of the problem you are currently try to address. But if you end up needing to shuffle any of this logic around anyway, then it might make sense to just handle both cases in an all-purpose track-duration-validator.
| editRate, | ||
| lastResource.getIntrinsicDuration(), | ||
| lastResource.getEntryPoint(), | ||
| BigInteger.valueOf(lastResource.getSourceDuration().longValue() - remainder), |
There was a problem hiding this comment.
Be careful with adjusting the sourceDuration, without first checking if repeatCount > 1.
I don't think it will impact your specific use-case. But for other users, in the unlikely event that the last Resource in the track uses repeats, then your adjustment will decrease the duration of each loop.
No description provided.