Skip to content

Conversation

@Mr-Neutr0n
Copy link

Summary

When clipping multiple segments (concatenated clips), the audio and video offsets (start_ost/end_ost) are applied twice — once when computing start/end, and again when slicing the data or computing time_acc_ost. This causes incorrect clip boundaries for any multi-segment operation.

In clip(): start and end already include the offsets after the clamping step (lines 128–129), but the data slice for subsequent segments was data[start+start_ost*16:end+end_ost*16], applying the offsets a second time. Fixed to use data[start:end].

In video_clip(): start and end already incorporate start_ost/1000.0 and end_ost/1000.0, but time_acc_ost was calculated as end+end_ost/1000.0 - (start+start_ost/1000.0), double-counting the offsets. Fixed to use end - start.

Note that the first segment in both methods was already handled correctly — the bug only affected the 2nd, 3rd, ... segments when multiple periods are matched and concatenated.

Fixes #129
Fixes #144

Test plan

  • Clip a single segment with non-zero start_ost/end_ost — should behave identically to before (no change for single segments)
  • Clip multiple segments with non-zero offsets — boundaries should now match the expected offset-adjusted positions instead of double-shifting
  • Verify subtitle timing (time_acc_ost) aligns correctly with concatenated video clips

In clip(), the start and end variables already include start_ost and
end_ost offsets after the clamping step, but the data slice for
subsequent segments was applying the offsets a second time via
data[start+start_ost*16:end+end_ost*16]. Use data[start:end] instead.

Similarly in video_clip(), the time_acc_ost calculation was adding the
offsets again even though start and end already incorporated them.
Use (end - start) directly since the duration already reflects the
offset-adjusted boundaries.

Fixes modelscope#129
Fixes modelscope#144
@gemini-code-assist
Copy link

Summary of Changes

Hello @Mr-Neutr0n, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a critical bug where audio and video offsets were applied twice during the clipping of concatenated segments, leading to incorrect clip boundaries and subtitle timings. The changes ensure that offsets are correctly accounted for only once, improving the accuracy of multi-segment audio and video processing.

Highlights

  • Audio clipping logic: Corrected the clip() function to prevent double application of audio offsets (start_ost/end_ost) when concatenating multiple segments, ensuring data[start:end] is used instead of data[start+start_ost*16:end+end_ost*16].
  • Video clipping and subtitle timing: Fixed the video_clip() function's time_acc_ost calculation to avoid double-counting video offsets, changing the formula from end+end_ost/1000.0 - (start+start_ost/1000.0) to end - start.
  • Scope of fix: Ensured that the fix specifically addresses issues in the 2nd, 3rd, and subsequent segments during multi-segment operations, as the first segment was already handled correctly.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • funclip/videoclipper.py
    • Corrected audio data slicing to prevent double application of offsets for concatenated segments.
    • Adjusted time_acc_ost calculation to avoid double-counting offsets for video clips, ensuring accurate subtitle synchronization.
Activity
  • No human activity has been recorded on this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a critical bug where audio and video offsets were applied twice when clipping multiple concatenated segments. The changes in funclip/videoclipper.py correctly resolve this issue.

In the clip function, the redundant offset application during data slicing for subsequent segments is removed, which prevents incorrect audio clip boundaries.

Similarly, in the video_clip function, the calculation for time_acc_ost is corrected for both the first and subsequent segments to prevent double-counting the offsets, ensuring correct subtitle timing for concatenated video clips.

The fixes are accurate, well-targeted, and directly address the problem described in the pull request description. The code changes are of high quality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant