BizHawk icon indicating copy to clipboard operation
BizHawk copied to clipboard

Change when/how movie length is calculated

Open RetroEdit opened this issue 5 years ago • 4 comments

@alyosha-tas brought this up in #2197. There are actually a few reasons for this. One main reason is that loading movies for previewing currently requires loading the entire movie file so the frame count can be displayed. There are two potential solutions here: add a frame count field to the header so the movie's frame count can be previewed without excessive loading. Or just don't display the frame count in the preview.

~~Besides the basic slowness of loading entire movies just for the purpose of previewing them, I think there's some double-loading going on for some reason. Each movie appears to be loaded twice from some preliminary debugging.~~

~~Note: The double-loading is caused by the initial hash check loading the movies to check if the hash matches the current game (PlayMovie.MatchHashCheckBox_CheckedChanged), and then the movies actually being loaded for display (PlayMovie.PlayMovie_Load).~~ (fixed in fe845ce8efae8fa3b11d04079d3ae20ec844ea8a)

2024-10-03 note: this is largely low priority for me now because of some efficiency improvements in other parts of the emulator since this issue was originally filed.

RetroEdit avatar Jul 05 '20 14:07 RetroEdit

Fix the double loading, don't add a frame count.

I very much oppose the frame count idea due to having to keep up with it on every frame change (or it be out of sync by assuming current frame = framecount, see cyclecount problems).

Also, I have big concerns about someone misusing it to be an accurate frame count when the input log count is the source of truth

adelikat avatar Jul 05 '20 14:07 adelikat

Couldn't we just deal with it upon serialization: get the length of the input log when serializing the movie file and that way it will be definite.

Also, it doesn't have to be exposed to the general Bk2Movie API; it could just be specific to preloading and serialization by design.

The reason I would really like to see this happen is because loading entire movies just for the purpose of getting the frame count is extremely wasteful. I understand the rationale of not wanting the field to be misused, but wasting time on previewing movie information is extremely annoying.

RetroEdit avatar Jul 05 '20 14:07 RetroEdit

That's true, it's completely calculable without a core, so it could be at least consistently accurate. Meh, I guess I support this. If anyone starts using it in the tasvideos parser, I will just scold them appropriately

adelikat avatar Jul 05 '20 14:07 adelikat

I've addressed the unnecessary loading that was a series of regressions from the original intent. Now it only loads the header and input log when pre-loading, and the necessary loading of the file twice was eliminated.

This is the extent of what I wanted to accomplish for the 2.5 milestone, so I'm removed the tag.

Leaving this ticket open to track the change to have frame count in the header. In pre-loading, if this value is present, it should skip loading of the input log, but if it is not there, it should continue to load it.

adelikat avatar Jul 11 '20 22:07 adelikat