Modernize codebase: auto-detect GPU/CPU, replace print with logging, update deps

[joonson]

Summary

• GPU/CPU auto-detection: SyncNetInstance and S3FD now auto-detect CUDA availability and fall back to CPU. All .cuda() calls replaced with .to(device).
• Logging: Replaced all print() calls with Python logging module across every script.
• Subprocess safety: Replaced subprocess.call(str, shell=True) with subprocess.run(list) to prevent command injection. Replaced pdb.set_trace() error handling with proper exceptions.
• Deprecated API fixes: Removed torch.autograd.Variable, added weights_only=True to torch.load(), fixed np.int → np.intp, modernized NMS tensor operations in S3FD to remove deprecated out= resize pattern.
• Scenedetect 0.6 migration: Updated from pinned scenedetect==0.5.1 to 0.6.7.1, rewrote scene_detect() to use the modern open_video / SceneManager API.
• General cleanup: super() without args, removed trailing semicolons, #!/usr/bin/env python3 shebangs, mkdir -p in download script, removed unused imports, os.makedirs(exist_ok=True).
• README: Clarified expected output is approximate, separated model download step, added pipeline stage descriptions.

Test plan

• Run sh download_model.sh — verify model and weights download without errors
• Run python demo_syncnet.py --videofile data/example.avi --tmp_dir data/work/pytmp — verify AV offset ~3, confidence ~10
• Run full pipeline (run_pipeline.py → run_syncnet.py → run_visualise.py) on a test video
• Test on a CPU-only machine (or with CUDA_VISIBLE_DEVICES="") to verify fallback works
• Test on a CUDA machine to verify GPU path still works

[gemini-code-assist]

Code Review

This pull request modernizes the repository by updating to Python 3, replacing print statements with a structured logging configuration, and transitioning from requirements.txt to Conda environment files. Key improvements include updating ffmpeg subprocess calls for better error handling, removing deprecated torch.autograd.Variable usage, and refactoring the scenedetect integration to support newer API versions. Review feedback highlights a potential TypeError when logging PyTorch tensors without converting them to scalars and identifies compatibility issues with weights_only=True in torch.load when handling full model pickles.

[gemini-code-assist]

The logger.info calls will fail with a TypeError because offset, minval, and conf are PyTorch tensors. The %d and %.3f format specifiers require real numbers (int/float), and PyTorch tensors do not automatically convert to these types when passed as arguments to the logging methods (which use the % operator internally). Use .item() to convert these 0-dimensional tensors to Python scalars.

Suggested change

	logger.info('AV offset: \t%d', offset)
	logger.info('Min dist: \t%.3f', minval)
	logger.info('Confidence: \t%.3f', conf)
	logger.info('AV offset: \t%d', offset.item())
	logger.info('Min dist: \t%.3f', minval.item())
	logger.info('Confidence: \t%.3f', conf.item())

[gemini-code-assist]

Using weights_only=True here is inconsistent with the save function on line 9, which uses torch.save(model, f) to save the entire model object. weights_only=True is intended for loading state dictionaries and will fail to unpickle custom classes like S. To maintain compatibility with the save function in this same file, weights_only should be set to False, or the save function should be updated to save only the state_dict.

Suggested change

	net = torch.load(filename, weights_only=True)
	net = torch.load(filename, weights_only=False)

[gemini-code-assist]

Setting weights_only=True may break loading for existing pretrained models if they were saved as full model objects (pickles) rather than state dictionaries. Given that SyncNetModel.save defaults to saving the full model, it is likely that many .model files in this ecosystem are full pickles. If you want to ensure backward compatibility with such files, set weights_only=False.

Suggested change

	loaded_state = torch.load(path, map_location=lambda storage, loc: storage, weights_only=True)
	loaded_state = torch.load(path, map_location=lambda storage, loc: storage, weights_only=False)

[joonson]

/gemini review

[gemini-code-assist]

Code Review

This pull request modernizes the repository by migrating to Python 3, replacing print statements with the logging module, and introducing conda environment files for dependency management. Significant changes include updating PySceneDetect API usage, improving FFmpeg command execution via subprocess.run, and enhancing security by using weights_only=True in torch.load. Review feedback identifies a missed logger conversion in SyncNetModel.py and a redundant file-opening operation in run_pipeline.py.

[gemini-code-assist]

The pull request description states that all print() calls were replaced with the logging module across every script. However, this print() call remains. For consistency and to adhere to the modernization goal, please replace it with a logger call. Note that you will also need to import logging and initialize a logger in this file.

[gemini-code-assist]

This call to open_video(video_path) is redundant. The video object was already opened on line 223 and remains valid and in scope here. Metadata properties like base_timecode and duration can be accessed directly from the existing object.

Suggested change

	video = open_video(video_path)
	scene_list = [(video.base_timecode, video.base_timecode + video.duration)]
	scene_list = [(video.base_timecode, video.base_timecode + video.duration)]

[joonson]

/gemini review

[gemini-code-assist]

Code Review

This pull request modernizes the codebase by upgrading to Python 3, introducing structured logging, and improving environment management via Conda. Key technical improvements include the removal of deprecated PyTorch features like Variable, the use of weights_only=True for secure model loading, and the transition to subprocess.run for more robust external command execution. The review feedback highlights two important reliability issues: the need for math.isclose when comparing floating-point durations to avoid precision errors, and the requirement to explicitly release video resources in run_pipeline.py to prevent potential file handle leaks.

[gemini-code-assist]

Comparing floating-point values for exact equality is generally unreliable due to precision issues. It is safer to use math.isclose with an appropriate tolerance, especially when comparing durations derived from different sources (audio samples vs. video frames).

Suggested change

	if (float(len(audio))/16000) != (float(len(images))/25) :
	if not math.isclose(len(audio) / 16000.0, len(images) / 25.0, abs_tol=1e-4):

[gemini-code-assist]

The VideoSource object returned by open_video should be explicitly released to free system resources (like file handles or hardware decoders). Using a try...finally block ensures the video is released even if an error occurs during scene detection.

Suggested change

	video = open_video(video_path)
	scene_manager.detect_scenes(frame_source=video_manager)
	scene_manager = SceneManager()
	scene_manager.add_detector(ContentDetector())
	scene_manager.detect_scenes(video)
	scene_list = scene_manager.get_scene_list(base_timecode)
	scene_list = scene_manager.get_scene_list()
	savepath = os.path.join(opt.work_dir,opt.reference,'scene.pckl')
	if scene_list == []:
	scene_list = [(video_manager.get_base_timecode(),video_manager.get_current_timecode())]
	if not scene_list:
	scene_list = [(video.base_timecode, video.base_timecode + video.duration)]
	with open(savepath, 'wb') as fil:
	pickle.dump(scene_list, fil)
	print('%s - scenes detected %d'%(os.path.join(opt.avi_dir,opt.reference,'video.avi'),len(scene_list)))
	logger.info('%s - scenes detected %d', video_path, len(scene_list))
	return scene_list
	video_path = os.path.join(opt.avi_dir, opt.reference, 'video.avi')
	video = open_video(video_path)
	try:
	scene_manager = SceneManager()
	scene_manager.add_detector(ContentDetector())
	scene_manager.detect_scenes(video)
	scene_list = scene_manager.get_scene_list()
	if not scene_list:
	scene_list = [(video.base_timecode, video.base_timecode + video.duration)]
	finally:
	video.release()