search

LMMS / lmms

Cross-platform music production software
https://lmms.io
GNU General Public License v2.0
8k stars 994 forks source link

Time Signature in Song Editor #3734

Closed gi0e5b06 closed 6 years ago

gi0e5b06 commented 7 years ago

Time signature was not displayed in the song editor. Always assumed 4/4. I fixed it for the numerator. song_editor_timesig

PhysSong commented 7 years ago

Could you clarify what you mean?

gi0e5b06 commented 7 years ago

The black and grey backgrounds. Also the bar numbers were inappropriate.

PhysSong commented 7 years ago

I think something looks inconsistent in current LMMS, but I don't think it is wrong. @Umcaruje Do you have any idea about this?

zonkmachine commented 7 years ago

I'm not sure what you're trying to achieve here. A bar is still one bar under a different time signature. Putting bars in groups of three, as in the example above, just because the time signature is 3/4, doesn't make much sense musically.

PhysSong commented 7 years ago

Background looks okay, but I think text positions are somewhere weird. For 5/3 signature and 100% zoom, labels are: 1, 8, 15, 22, ...

gi0e5b06 commented 7 years ago

@zonkmachine Short answer: you're right and I reverted to 4. (one line of code) Long answer: It's not uncommon to have the length of the hyperbars matching the time signature in some way, but it can not be generalized. Goal: I would like a better representation of the hyperbars and/or the structure. If possible, automatically.

zonkmachine commented 7 years ago

Long answer: It's not uncommon to have the length of the hyperbars matching the time signature in some way, but it can not be generalized.

Right. I actually only have deeper experience with lmms so I should maybe be a bit careful to express myself in absolutes. : )

Umcaruje commented 7 years ago

@gi0e5b06 please, open pull requests from different branches for each of your proposals, I can't see what's wrong with the current implementation, or what problem you're describing. Code would tremendously help.

From a musical standpoint it makes sense to always use groups of 4 for bar shadings, because most usually musical sentences are either 4 or 8 bars long, and that's not dependent on the time signature. the time signature affects metering inside the bar, and that is a piano roll problem, not a song editor one.

gi0e5b06 commented 7 years ago 
const int tactsPerBar = 4; // most common case
//const int tactsPerBar=Engine::getSong()->getTimeSigModel().getNumerator();

@Umcaruje I already agreed that it is not possible to assume the length of the hyperbars using only the timesig. But I disagree with you on 4 being the best choice, however I don't have the data to prove it. I still think that, statiscally, the length of the phrases is loosely correlated to the timesig. Anyway, it doesn't matter, because it should be user-defined.

Thanks to this thread, I have refined a lot my idea (to show the structure of the song) and how to implement it.

gi0e5b06 commented 7 years ago

Starting to look good. Default to 4. Structure is: intro+ABABABB+outro (typical pop song) hyperbars1 hyperbars2 WIP. Labels and Dialog are missing.

musikBear commented 7 years ago

im confused

tresf commented 7 years ago

@gi0e5b06 please use the default theme when possible and also, we would prefer if working from a pull request.

At a glance, it appears this bug report (Time Signature improvements) is being used to show mock-ups for custom loop points (right?).

gi0e5b06 commented 7 years ago

@tresf You can not switch between themes easily and the default theme is beautiful but painful for the eyes.

Not custom loop points but the song structure (these are two different things). And not mock-ups but real code (it's not finished but it's working).

In this example: green = intro red = verse (16 bars) blue = chorus (8 bars) there is also a one-bar break and a two-bar outro.

PS: multi-loops is a different issue and it's only 10% done. It works but nothing is saved, there is no postponed jump like for Ableton, it's not yet midi-controlled (so no launchpad). But the main problematic point in LMMS is that each track doesn't have an independant play position (it will take 6-8 hours to implement that, maybe next week).

tresf commented 7 years ago

@gi0e5b06 please read your original bug report and then realize how off-topic this all is.

Using your own theme is fine for mock-ups, but the code must use our theme and the PR should illustrate that.

The contributions are welcome, but we expect the code to be in a reviewable fashion. This means you should create a separate branch for each feature, a separate PR for discussion and use our built-in theme so that developers can approve this from a code quality, UI quality and UX quality perspective. We can help with these, but we do expect the standard theme to be used for the proof of concept.

@tresf You can not switch between themes easily and the default theme is beautiful but painful for the eyes.

Your development build should use a localized .lmmsrc.xml file, so you should be able to switch at least the one you're building easily.

musikBear commented 7 years ago

In this example:

@gi0e5b06 Oh, you are shading over spans of structure My eyes simply could not catch that (they are rather lousy.. 🦇 Coloured B&B segments in one B&B-track, can actually be used for that already now

BaraMGB commented 6 years ago

I close this because of not valid. I don't see what is the issue here.

gi0e5b06 commented 6 years ago

Seriously?