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

capture: Add bxt_cap_start_segment #58

Merged
merged 2 commits into from
Feb 18, 2023
Merged

Conversation

khanghugo
Copy link
Contributor

@khanghugo khanghugo commented Dec 2, 2022

Closes #56

Still WIP but it works. There are some details worth discussing and a bug looking bit too much to fix.

Right now, it couples with "bxt_play_run/folder" and it will ignore "bxt_cap_start" argument. Not sure what the argument should mean but in the mentioned issue, I propose to have it as the prefix for the file name ("prefix_demoname.mp4" as a result). But that route needs #57 to be properly addressed. Otherwise, I think the argument could mean THE file name and following split will have number trailing ("filename_number.mp4"). That is what I can come up with to still have "bxt_cap_start" around.

If we go with the prefix, the demo name of the first demo seems a bit too much to get. I look around the source and getting the demo file name is really tricky. demo_playback.rs module does help getting the name but it still does not show the name of the first demo. Maybe I need to get good or go dirty.

For the time being, I don't think this would be useful for live TAS recording so I don't think it needs to be here.

@khanghugo khanghugo changed the title cap: Add bxt_cap_autosplit capture: Add bxt_cap_autosplit Dec 4, 2022
@YaLTeR
Copy link
Owner

YaLTeR commented Dec 5, 2022

Maybe instead of a new cvar it should be a new bxt_cap_start_with_autosplit (with a less awkward name ideally)? Then there's no problem and it's orthogonal.

@khanghugo
Copy link
Contributor Author

Alrighty then. I got your permission.

@khanghugo
Copy link
Contributor Author

So this will output all of the demos in a directory instead. Some stuffs still need adding on. First is first demo will not have its name saved anywhere, even in the game because I don't really know why. So it just needs the demo_playback module to have a state and get the name of currently played demo from there. The next thing is just some helpful console messages. One minor detail, the command is now bxt_cap_start_segment because it is easy to forget to write it and the command is shorter without it.

@khanghugo khanghugo changed the title capture: Add bxt_cap_autosplit capture: Add bxt_cap_start_segment Dec 6, 2022
@khanghugo
Copy link
Contributor Author

khanghugo commented Dec 7, 2022

This state should be good. Maybe pattern is needed but this guy on Windows says it is working for him so I am not sure if it's needed.

Take a look at this one first before you get to the other one because I add some changes of this one to another so it looks very messy...

@khanghugo khanghugo marked this pull request as ready for review December 7, 2022 22:21
@khanghugo

This comment was marked as resolved.

khanghugo added a commit to khanghugo/bxt-rs that referenced this pull request Dec 8, 2022
khanghugo added a commit to khanghugo/bxt-rs that referenced this pull request Dec 11, 2022
Copy link
Owner

@YaLTeR YaLTeR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not review everything yet

src/hooks/engine.rs Outdated Show resolved Hide resolved
Comment on lines 1946 to 1947
// demo_playback::CURRENT_DEMO should be reset here too
// but there are some stuffs with first demo so no.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you clarify this comment please, my mind reading is pretty rusty lately

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might miss something but here is what I recall vaguely happens when bxt_play_run/folder do things:

  1. Things set up and happen then demo_playback::set_next_demo is called. The name of the first demo is in the cls.demos array.
  2. Prepend the command "demos".
  3. "demos" calls Host_NextDemo.
  4. demo_playback::set_next_demo is called again. The name of the second demo is now in the array instead.
  5. The first demo is played, but cls.demos stores the name of next demo.

So, if I reset the name of currently playing demo name in that hooked function, the name of the first demo will not be known. CL_Disconnect is called along with the Host_NextDemo.

Copy link
Contributor Author

@khanghugo khanghugo Dec 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add on, CL_Disconnect is called before setting the next demo. So clearing the current demo name on disconnect will not make the name of the current demo known.

So it is better to use "bxt_cap_start_segment" with "bxt_play_run/folder". I hope no one would try to do something not intended, haha.

This all happens because of the first demo specifically. If first demo name is known then ig. But I guess this would means some changes to how "bxt_play_run/folder" works.

src/hooks/engine.rs Outdated Show resolved Hide resolved
src/hooks/engine.rs Outdated Show resolved Hide resolved
src/modules/capture/mod.rs Outdated Show resolved Hide resolved
src/modules/demo_playback.rs Outdated Show resolved Hide resolved
@khanghugo khanghugo marked this pull request as draft December 31, 2022 22:05
@khanghugo khanghugo marked this pull request as ready for review December 31, 2022 23:16
@khanghugo khanghugo marked this pull request as draft January 15, 2023 22:08
@khanghugo khanghugo marked this pull request as ready for review January 15, 2023 22:18
@YaLTeR
Copy link
Owner

YaLTeR commented Feb 17, 2023

I reworked this how I think it should be implemented and work. I renamed the command to bxt_cap_separate_start (as in separate videos). Please test it out and tell me if it works, and if it works as you expect it to work. If something doesn't work as expected, please say, so I can think how to change it.

Thanks khanghugo for doing most of the initial work.
@YaLTeR YaLTeR merged commit 0eadb75 into YaLTeR:master Feb 18, 2023
@khanghugo
Copy link
Contributor Author

Thanks.

khanghugo added a commit to khanghugo/bxt-rs that referenced this pull request Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically split video capture of multiple demos into multiple files
2 participants