celtera / libremidi

A modern C++ MIDI 1 / MIDI 2 real-time & file I/O library. Supports Windows, macOS, Linux and WebMIDI.
Other
463 stars 52 forks source link

[writer] Change ticksPerQuarterNote to be uint16_t #119

Closed alyssais closed 1 month ago

alyssais commented 1 month ago

This is passed to util::write_uint16_be(), which could cause a truncation if it was set to a value that didn't fit into a 16-bit unsigned integer.

This produced the following compiler warning for me:

/build/source/libremidi/include/libremidi/writer.cpp: In member function 'void libremidi::writer::write(std::ostream&) const':
/build/source/libremidi/include/libremidi/writer.cpp:205:30: error: conversion from 'int' to 'uint16_t' {aka short unsigned int'} may change value [-Werror=conversion]
  205 |   util::write_uint16_be(out, ticksPerQuarterNote);
      |                              ^~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
jcelerier commented 1 month ago

thanks for noticing this ! most likely at some point in the future it will change again to have an error being brought up if someone wants to use an invalid ppq value, as right now someone may just do ticksPerQuarterNote = 1e14; and things will still compile & run

jcelerier commented 1 month ago

also spent some time improving the big endian code with C++20 while I was checking this :)