CasparCG / server

CasparCG Server is a Windows and Linux software used to play out professional graphics, audio and video to multiple outputs. It has been in 24/7 broadcast production since 2006. Ready-to-use downloads are available under the Releases tab https://casparcg.com.
GNU General Public License v3.0
911 stars 268 forks source link

Last Frame is not being played #1365

Open dirkdamerau opened 3 years ago

dirkdamerau commented 3 years ago

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 commented 3 years ago

So, we hit the same issue, did a little debugging and it seems that this happens because of this commit: https://github.com/CasparCG/server/commit/0fbd420c1f6bf9810e9e581a791d4050b9f08a3a

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

Just let me know if you need anything else.

rubu commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

@scriptorian any thoughts on this?

rubu commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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?