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

Fix off-by-one error in loop support #62

Merged
merged 1 commit into from
Feb 24, 2022

Conversation

segfault-bilibili
Copy link
Contributor

@segfault-bilibili segfault-bilibili commented Feb 5, 2022

Not tested. Not heavily tested.

Fix #61

@segfault-bilibili segfault-bilibili marked this pull request as ready for review February 5, 2022 16:33
@segfault-bilibili
Copy link
Contributor Author

Unfixed "seam" (at loop end + 1 block offset)

Unfixed


Fixed "seam" (at loop end + 0 block offset)

Fixed


Original, the right half "tail" has not yet been overwritten by the loop start block (at loop end + 0 block offset)

Orig

@segfault-bilibili
Copy link
Contributor Author

@CaiMiao

@hozuki
Copy link
Member

hozuki commented Feb 24, 2022

LGTM.

Thanks for the fix! I don't have plenty of looped audios to test. The test results above proves itself.

@hozuki hozuki merged commit c364976 into OpenCGSS:master Feb 24, 2022
@hozuki
Copy link
Member

hozuki commented Feb 24, 2022

Hmm CI seems failing. I'll try to fix that when not occupied.

@segfault-bilibili segfault-bilibili deleted the fix-loop-off-by-one branch February 24, 2022 06:27
@segfault-bilibili
Copy link
Contributor Author

I don't have plenty of looped audios to test.

You may try Magia Record.

Hmm CI seems failing.

Hmm? Seems to be okay for me: https://ci.appveyor.com/project/segfault-bilibili/deretore/builds/42691233

@segfault-bilibili
Copy link
Contributor Author

I did nothing than authorizing AppVeyor as a GitHub app and then authorizing it to access my forked repo. Maybe you can just try to revoke access permissions for AppVeyor and then re-grant it in GitHub?

@segfault-bilibili
Copy link
Contributor Author

segfault-bilibili commented Feb 24, 2022

However I saw an error in the build log (although it didn't seem to block building process), not sure whether this matters:

https://ci.appveyor.com/project/segfault-bilibili/deretore/builds/42691233?fullLog=true#L234

WARNING: NU1603: Build depends on Cake.Frosting (>= 0.1.0-alpha0078) but Cake.Frosting 0.1.0-alpha0078 was not found. An approximate best match of Cake.Frosting 0.31.0 was resolved.
  NotFound https://www.myget.org/F/cake/api/v3/flatcontainer/system.security.accesscontrol/index.json 2201ms
nuget : NU1605: Detected package downgrade: Cake.Common from 0.31.0 to 0.28.0. Reference the package directly from the project to select a different version. 
At line:1 char:1
+ nuget restore "C:\projects\deretore\external\VGAudio\build\Build.sln"
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (NU1605: Detecte...erent version. :String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError
 
 Build -> Cake.Frosting 0.31.0 -> Cake.Common (>= 0.31.0) 
 Build -> Cake.Common (>= 0.28.0)
Committing restore...
Generating MSBuild file C:\projects\deretore\external\VGAudio\build\obj\Build.csproj.nuget.g.props.
Generating MSBuild file C:\projects\deretore\external\VGAudio\build\obj\Build.csproj.nuget.g.targets.
Writing assets file to disk. Path: C:\projects\deretore\external\VGAudio\build\obj\project.assets.json
Failed to restore C:\projects\deretore\external\VGAudio\build\Build.csproj (in 4.2 sec).
Errors in C:\projects\deretore\external\VGAudio\build\Build.csproj
    NU1605: Detected package downgrade: Cake.Common from 0.31.0 to 0.28.0. Reference the package directly from the project to select a different version.
     Build -> Cake.Frosting 0.31.0 -> Cake.Common (>= 0.31.0)
     Build -> Cake.Common (>= 0.28.0)
NuGet Config files used:

@segfault-bilibili
Copy link
Contributor Author

There's another bug to be fixed: #63

@segfault-bilibili
Copy link
Contributor Author

I'm sorry but I think this change should be reverted. It didn't actually fix the problem correctly. I'll compose a new PR if I have time.

@segfault-bilibili
Copy link
Contributor Author

Again I feel very sorry but I think my incorrect patch should be reverted.

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.

Loop support - slight bursts, maybe still not aligned etc?
2 participants