Adding a function to easily modify the event times at a score level (like `midi.adjust_times` in PrettyMIDI)

[leleogere]

Is your feature request related to a problem? Please describe. I would like to be able to speed-up or slow down a MIDI file easily, by modifying note timings (not only the tempo events).

Describe the solution you'd like PrettyMIDI proposes the method .adjust_times:

import pretty_midi as pm

midi = pm.PrettyMIDI("file.mid")
end_time = midi.get_end_time()
midi.adjust_times([0, end_time], [0, 0.7 * end_time])

This linearly interpolate between the original timing and the new timing by stretching or shrinking time, impacting all the elements of the MIDI file (note starting time, note duration, tempo value, tempo position, time signature position, more generally all possible events). I haven't dig into what they are doing under the hood, to the relation between tempo, ticks and seconds, but this function can be quite useful.

It even allow to pass n-sized lists to allow for more complex mapping than a simple interpolation between the start and the end time.

Describe alternatives you've considered I've considered doing it manually, but it would be handier to have a dedicated function. I don't know if this kind of function have its place in the context of symusic, I would be happy to have your opinion on that.

[Yikai-Liao]

It's a reasonable feature. But the definition of the function in pretty_midi is not very clear. For example, if the time interval corresponding to the beginning and end of a note contains a time in original_times, what should we do with the end time of the note?

[Yikai-Liao]

And it seems that it could also be a member function for both Track and all kinds of list of time events like NoteList

[leleogere]

It's a reasonable feature. But the definition of the function in pretty_midi is not very clear. For example, if the time interval corresponding to the beginning and end of a note contains a time in original_times, what should we do with the end time of the note?

I would say that the first part of the note get stretched/shrunk by interpolating in the first interval, while the second part is stretched/shrunk by interpolating in the second interval. I did the following example (I might have made a mistake, the problem is simple, but it is very easy to get confused!)

The bottom line is the new timing, based on the original timing. The red lines correspond to events that are specifically indicated in original_times and new_times, while the gray ones are other times that are simply interpolated between the red lines. So the original note duration was 4 seconds, and the new duration is 7.5 seconds.

Here is another example:

And it seems that it could also be a member function for both Track and all kinds of list of time events like NoteList

Yes I agree that it makes sense for all kind list of time events.

[Yikai-Liao]

Ok, I understand. I'll try to implement it.

[Yikai-Liao]

@leleogere I have added this function in the synth branch (it will be released in 0.4.0 with the new Synthesizer) You could try it by:

git clone -b synth https://github.com/Yikai-Liao/symusic.git --recursive
pip install ./symusic

Note that you need a c++ compiler supporting c++20 to build symusic from source, like g++11.

from symusic import Score
s = Score("path to midi")
end = s.end()

# All the events are sorted by time when loading
# So you could pass is_sorted=True here.
# adjust_time is not an inplace operation, calling it will create a new object
s2 = s.adjust_time([0, end], [0, end // 2], is_sorted=True)

# Track and List of events all get this member function
notes = s.tracks[0].notes
notes2 = notes.adjust_time([0, end], [0, end // 2], is_sorted=True)

If things work as what you expect, I'll release it.

[leleogere]

Thank you!

I haven't been able to build symusic for now, I will try to find out why by end the of the week.

Error trace

```bash (base) gerel@pop-os:/tmp$ conda create -p /tmp/symusic_env python=3.11 (base) gerel@pop-os:/tmp$ conda activate /tmp/symusic_env (/tmp/symusic_env) gerel@pop-os:/tmp$ git clone -b synth https://github.com/Yikai-Liao/symusic.git --recursive (/tmp/symusic_env) gerel@pop-os:/tmp$ pip install ./symusic Processing ./symusic Installing build dependencies ... done Getting requirements to build wheel ... done Installing backend dependencies ... done Preparing metadata (pyproject.toml) ... done Collecting numpy (from symusic==0.4.0) Using cached numpy-1.26.4-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (61 kB) Collecting pySmartDL (from symusic==0.4.0) Using cached pySmartDL-1.3.4-py3-none-any.whl (20 kB) Collecting platformdirs (from symusic==0.4.0) Using cached platformdirs-4.2.0-py3-none-any.whl.metadata (11 kB) Using cached numpy-1.26.4-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (18.3 MB) Using cached platformdirs-4.2.0-py3-none-any.whl (17 kB) Building wheels for collected packages: symusic Building wheel for symusic (pyproject.toml) ... error error: subprocess-exited-with-error × Building wheel for symusic (pyproject.toml) did not run successfully. │ exit code: 1 ╰─> [201 lines of output] WARNING: Use cmake.version instead of cmake.minimum-version with scikit-build-core >= 0.8 WARNING: Use ninja.version instead of ninja.minimum-version with scikit-build-core >= 0.8 *** scikit-build-core 0.8.0 using CMake 3.28.3 (wheel) *** Configuring CMake... 2024-02-14 10:22:08,614 - scikit_build_core - WARNING - libdir/ldlibrary: /tmp/symusic_env/lib/libpython3.11.a is not a real file! 2024-02-14 10:22:08,614 - scikit_build_core - WARNING - Can't find a Python library, got libdir=/tmp/symusic_env/lib, ldlibrary=libpython3.11.a, multiarch=x86_64-linux-gnu, masd=None loading initial cache file /tmp/tmp0aq2j5ja/build/CMakeInit.txt -- The C compiler identification is GNU 11.4.0 -- The CXX compiler identification is GNU 11.4.0 -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Check for working C compiler: /usr/bin/cc - skipped -- Detecting C compile features -- Detecting C compile features - done -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Check for working CXX compiler: /usr/bin/c++ - skipped -- Detecting CXX compile features -- Detecting CXX compile features - done -- Version: 10.2.2 -- Build type: Release CMake Deprecation Warning at 3rdparty/prestosynth/3rdparty/libsamplerate/CMakeLists.txt:1 (cmake_minimum_required): Compatibility with CMake < 3.5 will be removed from a future version of CMake. Update the VERSION argument value or use a ... suffix to tell CMake that the project does not need compatibility with older versions. -- Looking for stdbool.h -- Looking for stdbool.h - found -- Looking for unistd.h -- Looking for unistd.h - found -- Looking for immintrin.h -- Looking for immintrin.h - found -- Found PkgConfig: /usr/bin/pkg-config (found version "0.29.2") -- Checking processor clipping capabilities... -- Performing Test CPU_CLIPS_POSITIVE -- Performing Test CPU_CLIPS_POSITIVE - Failed -- Performing Test CPU_CLIPS_NEGATIVE -- Performing Test CPU_CLIPS_NEGATIVE - Failed -- Checking processor clipping capabilities... none -- Performing Test HAVE_VISIBILITY -- Performing Test HAVE_VISIBILITY - Success -- Performing Test standard_math_library_linked_to_automatically -- Performing Test standard_math_library_linked_to_automatically - Success -- Standard libraries to link to explicitly: none -- Performing Test COMPILER_SUPPORT_WERROR -- Performing Test COMPILER_SUPPORT_WERROR - Success -- Performing Test COMPILER_SUPPORT_pedantic -- Performing Test COMPILER_SUPPORT_pedantic - Success -- Performing Test COMPILER_SUPPORT_Wall -- Performing Test COMPILER_SUPPORT_Wall - Success -- Performing Test COMPILER_SUPPORT_Wextra -- Performing Test COMPILER_SUPPORT_Wextra - Success -- Performing Test COMPILER_SUPPORT_Wundef -- Performing Test COMPILER_SUPPORT_Wundef - Success -- Performing Test COMPILER_SUPPORT_Wcastalign -- Performing Test COMPILER_SUPPORT_Wcastalign - Success -- Performing Test COMPILER_SUPPORT_Wcharsubscripts -- Performing Test COMPILER_SUPPORT_Wcharsubscripts - Success -- Performing Test COMPILER_SUPPORT_Wnonvirtualdtor -- Performing Test COMPILER_SUPPORT_Wnonvirtualdtor - Success -- Performing Test COMPILER_SUPPORT_Wunusedlocaltypedefs -- Performing Test COMPILER_SUPPORT_Wunusedlocaltypedefs - Success -- Performing Test COMPILER_SUPPORT_Wpointerarith -- Performing Test COMPILER_SUPPORT_Wpointerarith - Success -- Performing Test COMPILER_SUPPORT_Wwritestrings -- Performing Test COMPILER_SUPPORT_Wwritestrings - Success -- Performing Test COMPILER_SUPPORT_Wformatsecurity -- Performing Test COMPILER_SUPPORT_Wformatsecurity - Success -- Performing Test COMPILER_SUPPORT_Wshorten64to32 -- Performing Test COMPILER_SUPPORT_Wshorten64to32 - Failed -- Performing Test COMPILER_SUPPORT_Wlogicalop -- Performing Test COMPILER_SUPPORT_Wlogicalop - Success -- Performing Test COMPILER_SUPPORT_Wenumconversion -- Performing Test COMPILER_SUPPORT_Wenumconversion - Success -- Performing Test COMPILER_SUPPORT_Wcpp11extensions -- Performing Test COMPILER_SUPPORT_Wcpp11extensions - Failed -- Performing Test COMPILER_SUPPORT_Wdoublepromotion -- Performing Test COMPILER_SUPPORT_Wdoublepromotion - Success -- Performing Test COMPILER_SUPPORT_Wshadow -- Performing Test COMPILER_SUPPORT_Wshadow - Success -- Performing Test COMPILER_SUPPORT_Wnopsabi -- Performing Test COMPILER_SUPPORT_Wnopsabi - Success -- Performing Test COMPILER_SUPPORT_Wnovariadicmacros -- Performing Test COMPILER_SUPPORT_Wnovariadicmacros - Success -- Performing Test COMPILER_SUPPORT_Wnolonglong -- Performing Test COMPILER_SUPPORT_Wnolonglong - Success -- Performing Test COMPILER_SUPPORT_fnochecknew -- Performing Test COMPILER_SUPPORT_fnochecknew - Success -- Performing Test COMPILER_SUPPORT_fnocommon -- Performing Test COMPILER_SUPPORT_fnocommon - Success -- Performing Test COMPILER_SUPPORT_fstrictaliasing -- Performing Test COMPILER_SUPPORT_fstrictaliasing - Success -- Performing Test COMPILER_SUPPORT_wd981 -- Performing Test COMPILER_SUPPORT_wd981 - Failed -- Performing Test COMPILER_SUPPORT_wd2304 -- Performing Test COMPILER_SUPPORT_wd2304 - Failed -- Performing Test COMPILER_SUPPORT_OPENMP -- Performing Test COMPILER_SUPPORT_OPENMP - Success -- The Fortran compiler identification is unknown -- Found unsuitable Qt version "" from NOTFOUND -- Qt4 not found, so disabling the mandelbrot and opengl demos -- Could NOT find CLANG_FORMAT: Found unsuitable version "0.0", but required is exact version "9" (found CLANG_FORMAT_EXECUTABLE-NOTFOUND) -- -- Configured Eigen 3.4.90 -- CMake Deprecation Warning at 3rdparty/prestosynth/3rdparty/gcem/CMakeLists.txt:21 (cmake_minimum_required): Compatibility with CMake < 3.5 will be removed from a future version of CMake. Update the VERSION argument value or use a ... suffix to tell CMake that the project does not need compatibility with older versions. -- GCE-Math version 1.17.0 -- Performing Test COMPILER_SUPPORTS_CXX11 -- Performing Test COMPILER_SUPPORTS_CXX11 - Success -- The compiler /usr/bin/c++ has C++11 support. -- prestosynth_src: /tmp/symusic/3rdparty/prestosynth/src/envelope.cpp -- prestosynth_src: /tmp/symusic/3rdparty/prestosynth/src/soundfont.cpp -- prestosynth_src: /tmp/symusic/3rdparty/prestosynth/src/soundfont_internal.cpp -- prestosynth_src: /tmp/symusic/3rdparty/prestosynth/src/stb_vorbis.c -- prestosynth_src: /tmp/symusic/3rdparty/prestosynth/src/synthesizer.cpp -- symusic_src: /tmp/symusic/src/conversion.cpp -- symusic_src: /tmp/symusic/src/event.cpp -- symusic_src: /tmp/symusic/src/io/common.cpp -- symusic_src: /tmp/symusic/src/io/midi.cpp -- symusic_src: /tmp/symusic/src/io/zpp.cpp -- symusic_src: /tmp/symusic/src/pianoroll.cpp -- symusic_src: /tmp/symusic/src/repr.cpp -- symusic_src: /tmp/symusic/src/score.cpp -- symusic_src: /tmp/symusic/src/synth.cpp -- symusic_src: /tmp/symusic/src/track.cpp -- symusic_src: /tmp/symusic/src/utils.cpp Building python binding. -- Found Python: /tmp/symusic_env/bin/python (found version "3.11.7") found components: Interpreter Development.Module -- Link-time optimization (LTO) enabled -- Configuring done (2.9s) -- Generating done (0.0s) -- Build files have been written to: /tmp/tmp0aq2j5ja/build *** Building project with Ninja... [1/75] Building CXX object CMakeFiles/symusic.dir/src/event.cpp.o [2/75] Building CXX object CMakeFiles/symusic.dir/src/pianoroll.cpp.o [3/75] Building CXX object CMakeFiles/symusic.dir/src/utils.cpp.o [4/75] Building CXX object CMakeFiles/symusic.dir/src/synth.cpp.o FAILED: CMakeFiles/symusic.dir/src/synth.cpp.o /usr/bin/c++ -DFMT_HEADER_ONLY=1 -I/tmp/symusic/include -I/tmp/symusic/3rdparty/pdqsort -I/tmp/symusic/3rdparty/zpp_bits -I/tmp/symusic/3rdparty/fmt/include -I/tmp/symusic/3rdparty/minimidi/include -I/tmp/symusic/3rdparty/prestosynth/include -I/tmp/symusic/3rdparty/prestosynth/3rdparty/mio/include -I/tmp/symusic/3rdparty/prestosynth/3rdparty/libsamplerate/include -I/tmp/symusic/3rdparty/prestosynth/3rdparty/eigen -I/tmp/symusic/3rdparty/prestosynth/3rdparty/MPMCQueue/include -I/tmp/symusic/3rdparty/prestosynth/3rdparty/gcem/include -O3 -DNDEBUG -std=gnu++20 -flto=auto -fno-fat-lto-objects -fPIC -MD -MT CMakeFiles/symusic.dir/src/synth.cpp.o -MF CMakeFiles/symusic.dir/src/synth.cpp.o.d -o CMakeFiles/symusic.dir/src/synth.cpp.o -c /tmp/symusic/src/synth.cpp In file included from /tmp/symusic/3rdparty/prestosynth/include/prestosynth/wav.h:8, from /tmp/symusic/3rdparty/prestosynth/include/prestosynth/util/audio_util.h:9, from /tmp/symusic/3rdparty/prestosynth/include/prestosynth/soundfont_internal.h:11, from /tmp/symusic/3rdparty/prestosynth/include/prestosynth/soundfont.h:9, from /tmp/symusic/3rdparty/prestosynth/include/prestosynth/synthesizer.h:8, from /tmp/symusic/include/symusic/synth.h:8, from /tmp/symusic/src/synth.cpp:4: /tmp/symusic/3rdparty/prestosynth/include/prestosynth/util/io_util.h: In function ‘FILE* psynth::open_file(const string&, const string&)’: /tmp/symusic/3rdparty/prestosynth/include/prestosynth/util/io_util.h:34:22: error: ‘path’ was not declared in this scope 34 | FILE* fp = fopen(path.c_str(), mode.c_str()); | ^~~~ /tmp/symusic/3rdparty/prestosynth/include/prestosynth/util/io_util.h:36:34: error: ‘fmt’ was not declared in this scope; did you mean ‘fma’? 36 | throw std::runtime_error(fmt::format("File not found file: {}", path)); | ^~~ | fma [5/75] Building CXX object CMakeFiles/symusic.dir/src/io/common.cpp.o /tmp/symusic/src/io/common.cpp: In function ‘symusic::vec symusic::read_file(const string&)’: /tmp/symusic/src/io/common.cpp:49:10: warning: ignoring return value of ‘size_t fread(void*, size_t, size_t, FILE*)’ declared with attribute ‘warn_unused_result’ [-Wunused-result] 49 | fread(buffer.data(), 1, size, fp); | ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~ [6/75] Building CXX object CMakeFiles/symusic.dir/src/repr.cpp.o [7/75] Building CXX object CMakeFiles/symusic.dir/src/track.cpp.o [8/75] Building CXX object CMakeFiles/nanobind-static.dir/tmp/pip-build-env-vs80vy_8/overlay/lib/python3.11/site-packages/nanobind/src/nb_internals.cpp.o [9/75] Building CXX object CMakeFiles/symusic.dir/src/score.cpp.o [10/75] Building CXX object CMakeFiles/symusic.dir/src/io/midi.cpp.o [11/75] Building CXX object CMakeFiles/symusic.dir/src/io/zpp.cpp.o [12/75] Building CXX object CMakeFiles/symusic.dir/src/conversion.cpp.o [13/75] Building CXX object CMakeFiles/core.dir/py_src/core.cpp.o FAILED: CMakeFiles/core.dir/py_src/core.cpp.o /usr/bin/c++ -DFMT_HEADER_ONLY=1 -Dcore_EXPORTS -I/tmp/symusic/include -I/tmp/symusic/3rdparty/pdqsort -I/tmp/symusic/3rdparty/zpp_bits -I/tmp/symusic_env/include/python3.11 -I/tmp/pip-build-env-vs80vy_8/overlay/lib/python3.11/site-packages/nanobind/include -I/tmp/symusic/3rdparty/fmt/include -I/tmp/symusic/3rdparty/minimidi/include -I/tmp/symusic/3rdparty/prestosynth/include -I/tmp/symusic/3rdparty/prestosynth/3rdparty/mio/include -I/tmp/symusic/3rdparty/prestosynth/3rdparty/libsamplerate/include -I/tmp/symusic/3rdparty/prestosynth/3rdparty/eigen -I/tmp/symusic/3rdparty/prestosynth/3rdparty/MPMCQueue/include -I/tmp/symusic/3rdparty/prestosynth/3rdparty/gcem/include -O3 -DNDEBUG -std=gnu++20 -flto=auto -fno-fat-lto-objects -fPIC -fvisibility=hidden -fno-stack-protector -Os -ffunction-sections -fdata-sections -MD -MT CMakeFiles/core.dir/py_src/core.cpp.o -MF CMakeFiles/core.dir/py_src/core.cpp.o.d -o CMakeFiles/core.dir/py_src/core.cpp.o -c /tmp/symusic/py_src/core.cpp In file included from /tmp/symusic/3rdparty/prestosynth/include/prestosynth/wav.h:8, from /tmp/symusic/3rdparty/prestosynth/include/prestosynth/util/audio_util.h:9, from /tmp/symusic/3rdparty/prestosynth/include/prestosynth/soundfont_internal.h:11, from /tmp/symusic/3rdparty/prestosynth/include/prestosynth/soundfont.h:9, from /tmp/symusic/3rdparty/prestosynth/include/prestosynth/synthesizer.h:8, from /tmp/symusic/include/symusic/synth.h:8, from /tmp/symusic/include/symusic.h:21, from /tmp/symusic/py_src/core.cpp:14: /tmp/symusic/3rdparty/prestosynth/include/prestosynth/util/io_util.h: In function ‘FILE* psynth::open_file(const string&, const string&)’: /tmp/symusic/3rdparty/prestosynth/include/prestosynth/util/io_util.h:34:22: error: ‘path’ was not declared in this scope; did you mean ‘std::filesystem::__cxx11::path’? 34 | FILE* fp = fopen(path.c_str(), mode.c_str()); | ^~~~ | std::filesystem::__cxx11::path In file included from /usr/include/c++/11/filesystem:45, from /tmp/pip-build-env-vs80vy_8/overlay/lib/python3.11/site-packages/nanobind/include/nanobind/stl/filesystem.h:13, from /tmp/symusic/py_src/core.cpp:12: /usr/include/c++/11/bits/fs_path.h:248:9: note: ‘std::filesystem::__cxx11::path’ declared here 248 | class path | ^~~~ ninja: build stopped: subcommand failed. *** CMake build failed [end of output] note: This error originates from a subprocess, and is likely not a problem with pip. ERROR: Failed building wheel for symusic Failed to build symusic ERROR: Could not build wheels for symusic, which is required to install pyproject.toml-based projects ```

[Yikai-Liao]

Could you offer me your OS and compiler versions? The new version havn't passed the compiling tests for all the platforms on github actions. We are fixing it.

[leleogere]

• OS: Pop OS 22.04
• g++: 11.4.0
• Python: 3.11 though anaconda

$ cat /etc/os-release
NAME="Pop!_OS"
VERSION="22.04 LTS"
ID=pop
ID_LIKE="ubuntu debian"
PRETTY_NAME="Pop!_OS 22.04 LTS"
VERSION_ID="22.04"
HOME_URL="https://pop.system76.com"
SUPPORT_URL="https://support.system76.com"
BUG_REPORT_URL="https://github.com/pop-os/pop/issues"
PRIVACY_POLICY_URL="https://system76.com/privacy"
VERSION_CODENAME=jammy
UBUNTU_CODENAME=jammy
LOGO=distributor-logo-pop-os

$ hostnamectl
 Static hostname: pop-os
       Icon name: computer-laptop
         Chassis: laptop
      Machine ID: b82d41eb6ed641d34eabfdf363c14f6c
         Boot ID: 32b71e27a6e542da99e80e8d50810d52
Operating System: Pop!_OS 22.04 LTS               
          Kernel: Linux 6.6.10-76060610-generic
    Architecture: x86-64
 Hardware Vendor: Dell Inc.
  Hardware Model: Latitude 5510

$ uname -r
6.6.10-76060610-generic

$ g++ --version
g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[Yikai-Liao]

@leleogere Now the compilation passed on Linux. You could re-clone the resopitory (or git pull and git submodule update --recursive) and install it.

[leleogere]

Perfect, I managed to build it!

I tried a bit to play with it, and it works well on your example. However, I might have found a bug while doing some to try to compare the output to PrettyMIDI:

path = "1_funk_80_beat_4-4.mid"

import symusic
s = symusic.Score(path)
print("s.end() in ticks", s.end())
s = s.to(symusic.TimeUnit.second)  # I was trying to convert it to seconds to compare with PrettyMIDI (as they use seconds)
print("s.end() in seconds", s.end())
end = s.end()
old_times = [0, 10, 30, end]
new_times = [0, 20, 25, end // 2]

s2 = s.adjust_time(old_times, new_times, is_sorted=True)
print("s2.end() in seconds", s2.end())
s2 = s2.to(symusic.TimeUnit.tick)
print("s2.end() in ticks", s2.end())
s2.dump_midi("/tmp/test_symusic.mid")

# Reread score
s3 = symusic.Score("/tmp/test_symusic.mid")
print("s3.end() in ticks", s3.end())

When I run this file multiple times, the printed output is always the same (like the end printed are always the same). However, when opening the produced MIDI file, it sometimes output a 43 second-long file, and sometimes a 116 hour-long file :

When exploring the timing of the events, here is what I got:

It seems that sometimes, some very far events are added to the file.

It is very weird as it happens when running the exact same code, seemingly random.

You can find here the initial file, the "correct" version, and the "corrupted" version: https://we.tl/t-FUpyFstrXE

[Yikai-Liao]

Quite strange. In MuseScore, both of them get 43 seconds.

[Yikai-Liao]

@leleogere The bug is fixed now.

I wrote an inequality incorrectly, resulting in two control changes outside of end not being filtered out, which in turn resulted in an out-of-bounds.

While this bug has been fixed, it also hints at another interface semantics issue, which is whether we should consider all events when calculating end or start, or just note as before. @Natooz What's your opinion?

[Natooz]

👋 I think it makes sens to also adapt the times (.time, .duration) of all the events within the provided ranges. Btw I can add tests for this feature when it's merged in main

[Yikai-Liao]

The new version is almost ready. There is still one thing that, should we add an inplace argument in the adjust_time methods?

Although not implemented with any inplace operations (no performance benefits to add this argument), this might make the interface more consistent with sort and filter (they both get inplace argument`)

[Natooz]

If it's a low effort addition, I see no reason to not add it for consistency reasons :)

[Natooz]

Also @leleogere thank you for the figures above, they are very well made and self-explanatory. The docs could greatly benefit to have them included.

[Yikai-Liao]

@Natooz I have merged the branch into main. Shall I release 0.4.0 directly or do you want to test adjust_time further?

[Natooz]

It's best if I add the tests before releasing anything :) I'll do it when I'll have some time in the day (in a few hours)

[leleogere]

It seems to be working as it should!

While this bug has been fixed, it also hints at another interface semantics issue, which is whether we should consider all events when calculating end or start, or just note as before. @Natooz What's your opinion?

I agree that considering all events makes more sense.

The new version is almost ready. There is still one thing that, should we add an inplace argument in the adjust_time methods?

The inplace argument makes sense as it is already in other functions. More generally, a consistent inplace argument in all methods that modify a score is nice.

Also @leleogere thank you for the figures above, they are very well made and self-explanatory. The docs could greatly benefit to have them included.

Thank you! You can use them in the doc if you want (or use them as inspiration to do something more fancy). I can even provide the SVG file if you want to modify them.

[Yikai-Liao]

Nice, SVG is better. I'll add them to the document later.

[leleogere]

Here is the SVG file.

[Natooz]

Following #30 , I'll continue the discussion of .adjust_time's implementation. From my understanding, the method "shifts" points in time, which effectively shrinks or stretch portions of times. When giving original_times=[0, 4] and new_times=[0, 2], the method should effectively shrink by two the portion of time from tick 0 to 4. It should be equivalent to providing original_times=[4] and new_times=[2] as it is the only reference time adjusted. It should also be equivalent to providing original_times=[4, 8] and new_times=[2, 6] as the time at tick 8 is already "shifted" to tick 6 by the first time portion adjustment (0 to 4). Now pretty_midi only covers the time portions within the ranges provided in original_times. I personally thinks the method should do what its name suggests, that is "shrinking"/"stretching" portions of times given some reference times.

What do you think of this way of treating the time information? I actually find it clearer, especially when visualising what's actually happening on the figures from @leleogere

[Yikai-Liao]

@leleogere Any suggestions？

[leleogere]

I'm not sure that there is a right or wrong answer. I feel like an implementation choice has to be made. If you want ot=[0,4]/nt=[0,2] to be equivalent to ot=[4]/nt=[2], then you assume that behind the scenes, a virtual 0 is added at the start of both arrays if ot does not start with zero. But the question is, should we add a virtual "end" when the last element of ot isn't the end of the track? Or consider that we only "shift" the remaining elements to the left?

The two possibilities are the following (assuming the track ends at time 8):

In the first case, all those parameters are equivalents :

• ot=[4]/nt=[2]
• ot=[0,4]/nt=[0,2]
• ot=[4,8]/nt=[2,6]
• ot=[0,4,8]/nt=[0,2,6] (most complete formula)

In the second case, we have :

• ot=[4]/nt=[2]
• ot=[0,4]/nt=[0,2]
• ot=[4,8]/nt=[2,8]
• ot=[0,4,8]/nt=[0,2,8] (most complete formula)

In this second case, we consider that the start and the end of the track are fixed if not specified, and only move points in the interval, meaning that moving a point to the left will speed up the part before it, and slow down the part after it (and vice-verse when moved to the right).

I'm not sure about which solution makes the more sense.

[Yikai-Liao]

I think pretty midi is right, it gives the user a clear idea of the behavior of their code, instead of being added a couple of parameters that implicitly have two possibilities @Natooz

[Natooz]

Alright, I'll just adapt the tests accordingly then :)

[Yikai-Liao]

@leleogere new version has been released with adjust_time. If there is no other prolems, I'll close this issue.

[Natooz]

I quickly found a good usage for this method in MidiTok for a method splitting a MIDI in chunks of n beats https://github.com/Natooz/MidiTok/pull/148/files#diff-fd4bd5bf18bd048b3958c0bf7a2c8603e60dedec33aea84518aca8d0ab8f74b2R741

The method itself works, the only thing left to handle for a proper usage in this case would be to include all the notes starting in the original_times range. Here is a figure of what adjust_time selects when using (0, 12) beats (2.5 bars):

The original is:

adjust_times currently discard the last 3 notes as they end outside of the original_times range. Do you think an adding an argument allowing to keep such notes could be added? No big deal if you don't think it's a something worth to be added.

[Yikai-Liao]

@Natooz You could use the clip method for Track and Score. It will clip all the events in the object according to the given time range, and returns a new one (not an inplace operation).

Here is one of the function signature:

    def clip(
        self, start: int, end: int, clip_end: bool = False
    ) -> symusic.core.ScoreTick:
        """
        Clip events a given time range
        """

As you can see, there is a clip_end parameter that meet your needs. Sorry for the lack of documentation.

[leleogere]

@Natooz You could use the clip method for Track and Score. It will clip all the events in the object according to the given time range, and returns a new one (not an inplace operation).

Not the point, but wouldn't an in_place parameter for clip make sense too?

[Natooz]

@Yikai-Liao Awesome! That will do, thank you for the advice! :)

[Yikai-Liao]

@leleogere following #31 , I implemented an order preserving version of adjust_time using a similar algorithm. This makes the is_sorted parameter in the original adjust_time function meaningless, since we don't need to sort the data before adjust_time.

I think I can just remove this parameter now, since the difference in interface this brings is still acceptable. What's your idea?

[Yikai-Liao]

Not the point, but wouldn't an in_place parameter for clip make sense too?

@leleogere Yeap, I think there is indeed a need to systematize the interfaces that are currently available right now. I'll take the time to categorize and sort out the existing interfaces and open a new issue for discussion. Thanks for your advice.

[leleogere]

@leleogere following https://github.com/Yikai-Liao/symusic/issues/31 , I implemented an order preserving version of adjust_time using a similar algorithm. This makes the is_sorted parameter in the original adjust_time function meaningless, since we don't need to sort the data before adjust_time.

I think I can just remove this parameter now, since the difference in interface this brings is still acceptable. What's your idea?

Yeah, I don't see any issue with removing the is_sorted parameter, it seemed a bit odd regardless. It is better to have an order preserving algorithm as any way the adjust_time shouldn't cause any change in the order of the notes (except if the times are not strictly increasing, do you have a check for that?)

[Yikai-Liao]

Now the function checks if the original times and bew times are ascending. But i don't check if whether or not there are equal before and after. I will add the check.