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

Last Frame is not being played #1365

Open
dirkdamerau opened this issue Feb 4, 2021 · 10 comments
Open

Last Frame is not being played #1365

dirkdamerau opened this issue Feb 4, 2021 · 10 comments

Comments

@dirkdamerau
Copy link

dirkdamerau commented Feb 4, 2021

If playing back DNxHD (mxf), ProRes (mov) or Animation (mov) 1080p50 the last frame is never being displayed.

If not looping, the file pauses on the frame before the last one and if the file is looping the last frame is skipped. Loop is not seamless.
If playing back mp4/h264 not looping, the last frame is displayed. If looping mp4 the last frame is displayed more than once so looping isnt't seamless at all, but with mp4/h264 that's obvious.

Looking at the filed with other players (mpv, Quicktime, djv_view, ffplay) the last frame is definitely in all files.

All this is only happening in all 2.3.x builds. In 2.2.0-stable the last frame is always displayed as expected and looping is seamless. Just mp4/h264 is not, but that is again obvious.

Server version: >2.3.0 (tested with 2.3.0 LTS stable, 2.3.1 LTS stable and 2.3.2 LTS beta)
Configuration: 1 Channel Key/Fill with Decklink Output 1080p50
Operating system: Win10_2004 Build 19041.685
Hardware: HPZ420, 6-Core Xeon, Nvidia P2200, 64 GB RAM, NVME, Decklink Duo2

@rubu
Copy link

rubu commented Jul 8, 2021

So, we hit the same issue, did a little debugging and it seems that this happens because of this commit: 0fbd420

Can anyone comment why frame.duration is being added two times and in the same time it is substracted from duration. These two operations seem counter intuitive.

In my case, duration is 12000000, frame.duration is 33333 (30fps file) and when frame @ 11933333 has been read, end is evaluated to 11966667, next_pts is evaluated to 11966666, and time to 11999999 thus frame @ 11966666 is dropped.

@scriptorian can you please elaborate what am I missing and doesn't the change basically always drop the last frame?

@scriptorian
Copy link
Contributor

You may already have found that the change was prompted by this issue, leading to this pull request that contains a little discussion of the change.
It is certainly possible that I didn't consider non-integer frame durations carefully enough and that might explain @rubu's issue. @dirkdamerau's issue appears to be different and would need more investigation.

@rubu
Copy link

rubu commented Jul 10, 2021

@scriptorian thanks for the reply:) Well I'd say that looking at the change the last frame will always be dropped in all cases (since the duration is added to next_pts twice). I'm writing this from a phone so its hard to chech the diffs, but maybe the double addition was actually already there before your commit. I can have a look on the bad mp4 file in the issue you mentioned on Monday, but it seems to me that the check should just be "is pts past the duration of the file". The current logic that takes the end minus a single duration would also break if there is a bad file for which the pts of the last frame is still in bounds but the duration exceeds it (in which case I guess the duration of the frame should just be clamped?). @dirkdamerau - do you have any small sample files?

@dirkdamerau
Copy link
Author

Thanks for looking into this issue. I prepared some testfiles for you.
SimpleLoop.zip
All files are made with Adobe Media Encoder on macOS.

  • Simple Loop DNxHD 3s - not looping correctly in 2.3.3 (skipping last frame)
  • Simple Loop DNxHD 3s + 1f - loops correct in 2.3.3, single black frame at the end of the file is not shown
  • Simple Loop H264 3s - just to demonstrate different behavior (freezing on last frame before looping)

Just let me know if you need anything else.

@rubu
Copy link

rubu commented Jul 14, 2021

@dirkdamerau so yeah I had a look at your files - the Simple Loop DNxHD 3s (w/o the one extra frame) suffers from the same issue as my video - duration is 3000000, and it stops on frame with pts 2960000 and skips the last one at 2980000, since the duration is added twice. As for your mp4 case , it freezes because you have 5s of nothing at the end - the containers length is 3:05 not 3 as the mxf but last frame still is 2980000, so you can just fix the container.

So I still see that this is simply a math bug, will go over the original issue one more time to try to understand the cause for the "tightening".

This situation simply is bad, because yeah, now all our designers add uneeded frames at the end of the assets.

@rubu
Copy link

rubu commented Jul 14, 2021

@scriptorian for the ANDREWS_12.mp4 file from the original issue the current code still cuts some frames - it stops at pts 31520000, while there still is a frame at 31560000.

With the original (old) code, the code looped back to the frame identical to the first one just because the container duration is wrong (and the issue actually contains an advice to fix the file) and then stops, but the old code logically behaves as it should - plays everything up to the duration. As far as I can see, it would be super logical to revert this change.

@rubu
Copy link

rubu commented Jul 27, 2021

@scriptorian any thoughts on this?

@rubu
Copy link

rubu commented Dec 23, 2021

So should it be assumed that this is going to be left as is and we need to make a fork to avoid adding the empty frames at the end of all of the video assets?

@Julusian
Copy link
Member

@rubu nothing has been done on this yet because its not been enough of a problem for SVT, so they have not funded development and testing of a fix.
If someone was to raise a PR which fixes this, I will happily merge it after confirming it doesnt break other cases

@rubu
Copy link

rubu commented Dec 23, 2021

@Julusian ok, thanks, I can try to do a PR, but I guess the pain point here is that some people are most likely already padding their video assets with black frames (as mentioned in some tickets), so I guess there is actually no way to solves this without hurting somebody. Hmm, would it make sense to have this behaviour configurable vai some switch/config entry?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants