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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

provide output folder directory when splitting videos #298

Closed
fcakyon opened this issue Nov 12, 2022 · 8 comments
Closed

provide output folder directory when splitting videos #298

fcakyon opened this issue Nov 12, 2022 · 8 comments
Milestone

Comments

@fcakyon
Copy link

fcakyon commented Nov 12, 2022

@Breakthrough thanks for this awesome project! I am utilizing it on my PhD heavily 馃憤

Issue

You can set the output directory in save_images function which is super useful in my case. However, there is no such feature for split_video_ffmpeg which is a must in my case.

Example:

from scenedetect.video_splitter import split_video_ffmpeg

split_video_ffmpeg(
    filepath,
    scene_list=scene_list,
    output_file_template="$scene-$SCENE_NUMBER.",
    output_dir=export_dir,
    show_progress=True,
)

Question:

  1. Do you have any intentions to add such a feature?
  2. How can I implement a workaround for this? Should I change these lines:
    call_list += [
    '-sn',
    filename_template.safe_substitute(
    VIDEO_NAME=video_name, SCENE_NUMBER=scene_num_format % (i + 1))
    ]
@fcakyon
Copy link
Author

fcakyon commented Nov 12, 2022

How I solved it:

update this:

            call_list += [
                '-sn',
                filename_template.safe_substitute(
                    VIDEO_NAME=video_name, SCENE_NUMBER=scene_num_format % (i + 1))
            ]

with this:

# create output dir at function start
Path(output_dir).mkdir(parents=True, exist_ok=True)

            # prepend output_dir to filename
            export_path = str(
                Path(output_dir)
                / filename_template.safe_substitute(
                    VIDEO_NAME=video_name, SCENE_NUMBER=scene_num_format % (i + 1)
                )
            )
            call_list += [
                "-sn",
                export_path,
            ]

@Breakthrough
Copy link
Owner

Hi @fcakyon;

You can replace the output_file_template parameter with an absolute path as well to work around this, e.g.:

from scenedetect.video_splitter import split_video_ffmpeg

export_dir = "C:/output_folder/"

split_video_ffmpeg(
    filepath,
    scene_list=scene_list,
    output_file_template=export_dir + "$scene-$SCENE_NUMBER.",
    show_progress=True,
)

The documentation could probably be improved here, but I probably will leave this as-is and not introduce another argument. Thanks for the report!

@Mensen
Copy link

Mensen commented Nov 16, 2022

Apologies for raising a closed issue from the dead here, but the proposed solution (#298 (comment)), doesn't work for my example.

>>> output_dir = r'F:\Gamepass_Clips'
>>> output_template = os.path.join(os.path.join(output_dir,"scene-$SCENE_NUMBER.mp4"))
>>> print(output_template)
F:\Gamepass_Clips\scene-$SCENE_NUMBER.mp4
>>> scenedetect.video_splitter.split_video_ffmpeg(video_file, scene_list,
...   output_file_template=output_template)
Error splitting video (ffmpeg returned 1).
1

My code differs from your example by using the os.path functions instead of writing the path in a regular string... but I have also tried that with no apparent difference.

I considered whether there were some potential write permissions that throws the error but when I do the actual scenedetect, I also specify a stats_file_path in precisely the same way using the os.path method

Let me know if I should have simply raised another issue here, but considering the "solution" doesn't work, I figured it might be best to follow up here.

Thanks in advance for helping out one of your sponsors :)

@Breakthrough
Copy link
Owner

@Mensen what happens if you use forward slashes instead? Does it work as expected from the CLI? If it still fails feel free to re-open a new issue, what you show above should work.

def get_and_create_path(file_path: AnyStr, output_directory: Optional[AnyStr] = None) -> AnyStr:
is how this works for the CLI.

@Mensen
Copy link

Mensen commented Nov 20, 2022

Forward slashes also does not work.

scenedetect.video_splitter.split_video_ffmpeg(video_file, scene_list[0:2], 
...   output_file_template='F:/Gamepass_Clips/Clip-$SCENE_NUMBER.mp4')
Error splitting video (ffmpeg returned 1).
1

No issue when simply specifying a file name, but this then puts the files in the code directory which, when splitting 100s of scenes are less than ideal

scenedetect.video_splitter.split_video_ffmpeg(video_file, scene_list[0:2], 
...   output_file_template='Clip-$SCENE_NUMBER.mp4')
0

In the code snip above it refers to output_directory and not necessarily output_file_template so there seems to be an intermediate step between the split_video_ffmeg and the get_and_create_path code. Could the problem lie there?

For example in:

arg_override = arg_override.replace('\\"', '"')

That would certainly replace the double \\ that os.path.join uses when combining paths to be os compatable.

And wouldn't the line:

output_file_template_iter = output_file_template_iter.replace(":", ";")

Replace the ":" with a ";" in the path I specified?

@Breakthrough
Copy link
Owner

@Mensen thanks for pointing that out, indeed that seems to be the issue here. My apologies, feel free to file a new bug report for this.

@Breakthrough
Copy link
Owner

This has come up a few times again (e.g. #363), so I will look into revisiting this for the next release.

@Breakthrough Breakthrough reopened this Dec 1, 2023
@Breakthrough Breakthrough added this to the v0.6.3 milestone Dec 1, 2023
Breakthrough added a commit that referenced this issue Jan 21, 2024
Add `formatter` argument to split_video_ffmpeg to customize filename generation when required.

Fixes #359 and #298.
@Breakthrough
Copy link
Owner

This will be included in the next release, thanks!

Breakthrough added a commit that referenced this issue Apr 18, 2024
Add `formatter` argument to split_video_ffmpeg to customize filename generation when required.

Fixes #359 and #298.
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

3 participants