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 sample track view in BB editor #3002

Merged
merged 1 commit into from
Sep 4, 2016
Merged

Conversation

jasp00
Copy link
Member

@jasp00 jasp00 commented Aug 26, 2016

The goal is not mentioned in #1471. A sample track can be added to the BB editor by copying the track from the song editor. The pattern works, but the display is neglected. This patch fixes the view and includes a button to add a sample track to the BB editor.

@jasp00 jasp00 merged commit 08d54d3 into LMMS:master Sep 4, 2016
@jasp00 jasp00 deleted the sample-track branch September 4, 2016 00:16
@grejppi
Copy link
Contributor

grejppi commented Sep 4, 2016

Well, that was quick. I found some issues with this:

  1. Samples don't affect the length of a B&B pattern
  2. No matter which B&B pattern is playing, the sample track always plays all TCOs separated by 1 bar
  3. Why would anyone ever want sample tracks in the B&B editor?

@jasp00
Copy link
Member Author

jasp00 commented Sep 5, 2016

  1. Samples don't affect the length of a B&B pattern

Are you sure? With 140 BPM, techno_bass01.ogg lasts one bar, while techno_bass02.ogg lasts two.

  1. No matter which B&B pattern is playing, the sample track always plays all TCOs separated by 1 bar

Right, another issue for #1471.

  1. Why would anyone ever want sample tracks in the B&B editor?

Bass loops. Example: bb_techno_synth01.mmp.zip

@@ -752,8 +752,6 @@ PatternView::~PatternView()

void PatternView::update()
{
m_pat->changeLength( m_pat->length() );
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit problematic. If you create a BBTrack with a pattern that goes beyond step 16 ( see #1105/#2964) and you enter this in a song and extend it to cover more than one bar it will fill with one bar sequences. On saving/reloading the project this will now be a sequence of two bar BBTracks, every second bar being empty.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do not know the relation to this line of code. Anyway, I cannot reproduce this one bar sequence that reloads as two bars. Is there a sample project that shows the sequence?

Copy link
Member

Choose a reason for hiding this comment

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

funkyEnding.mmp.zip

funkyending

funkyendingreloaded

Method to reproduce.

  1. In a new project make a beat with a note at step 16 and hit play.
  2. Right click and open sequence in the Piano Roll. Shift the note one 32nd step to the right.
    It now still plays back as a one bar beat if you play from the BBEditor but if you now stop and play from the Piano Roll it will play back two bars, the second silent.
  3. In the Song Editor, enter a BB track and drag it out. In my case this will make a stretch of one bar BB Tracks.
  4. Save and reload. Now the BB tracks in the Song Editor are two bars.

Copy link
Member Author

Choose a reason for hiding this comment

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

It now still plays back as a one bar beat if you play from the BBEditor

It plays back two bars from the BB editor; it is the error described in #1105. Should I apply #2964?

Copy link
Member

Choose a reason for hiding this comment

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

It plays back two bars from the BB editor; it is the error described in #1105.

Yes, that's the old bug. The new one is that now you don't see it in the BBTrack until you reload the project unless you play it from the Song Editor.

Should I apply #2964?

Not ready yet. But I can poke this one a bit and fix it in 2964? I need to rebase that one and try it with Sample Tracks any way.

Copy link
Member Author

Choose a reason for hiding this comment

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

About the new bug, the pattern length is not being updated. Nevertheless, this m_pat->changeLength( m_pat->length() ); line is not suitable, since it would be a model update from a view update. The fix should update the model from related model changes.

Copy link
Member

Choose a reason for hiding this comment

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

Right. I've poked this a bit and I don't think there is anything I can do to help fixing this particular issue.

@zonkmachine
Copy link
Member

  1. Why would anyone ever want sample tracks in the B&B editor?

I think keeping the consistency is good. You get the same behaviour from both editors as far as possible.

@grejppi
Copy link
Contributor

grejppi commented Sep 5, 2016

Except that sample tracks are broken and there's little point in keeping them alive

@zonkmachine
Copy link
Member

I have not worked with sample tracks before but I have two objections to this PR.
1 - There is no function to sync the track.
2 - We're supposed to release something so this really looks like something for 1.3 to me.

@grejppi
Copy link
Contributor

grejppi commented Sep 8, 2016

As @jasp00 said this is not mentioned in #1471. At this point adding a new feature would delay the 1.2 release indefinitely, so personally I'd suggest reverting this, disabling the ability to move sample tracks from song editor to b&b editor for the time being, and discuss the feature again after 1.2 is out.

@BaraMGB
Copy link
Contributor

BaraMGB commented Sep 8, 2016

Sample track in bb editor seems pointless to me.

@tresf
Copy link
Member

tresf commented Sep 8, 2016

Sample track is pointless 99.9% of the time regardless of its location because it is very broken (#1471). Its usefulness in BBEditor is hard to gauge without fixing even basic usage.

From a consistency and UI perspective, any tracks that can live in Song Editor should be available for BBEditor as well unless there is a very good argument otherwise. And @grejppi: agree, 1.3 milestone is a better home.

@jasp00
Copy link
Member Author

jasp00 commented Sep 8, 2016

Sample tracks are like audioFileProcessor beats, but with a different view; they do not require the plug-in and it is a different way to work. There is demand for sample tracks in #1471. The code already considered sample tracks in BB editor. This PR does not add a new feature, only fixes the view, and #3023 fixes the detected issue; the needed work is done, so there is no delay for 1.2.

There is no function to sync the track.

@zonkmachine, what do you mean by function to sync the track? The sample is synced with the start of the beat pattern. Are you talking about an enhancement?

Sample track in bb editor seems pointless to me.

As I said, what about bass loops? Is audioFileProcessor pointless in the BB editor?

@zonkmachine
Copy link
Member

Are you talking about an enhancement?

Yes. It's handy to be able to set start/stop points. Syncopated samples and anything with voices in them (which usually are more 'relaxed' with the concept 'one' in a beat) are the ones I'm thinking of.

@zonkmachine
Copy link
Member

bbtracksync.mmp.zip

Here is an example of a problematic sample that we ship that I've added to a BBTrack both as Sample Track and in an AFP.

sambler added a commit to sambler/lmms that referenced this pull request Sep 19, 2016
* master: (213 commits)
  Update Pattern and AutomationPattern length (LMMS#3037)
  Refresh i18n strings
  Hint text update
  Drop notes with length zero (LMMS#3031)
  Background tweak
  Background
  Update Flanger
  Exclude .ts files from the Github linguist
  Redesign Multitap echo (LMMS#3008)
  Update i18n source strings
  Extended arpeggiator functions (LMMS#2130)
  Fix sample track playback in BB tracks (LMMS#3023)
  Sort plug-in embedded resources (LMMS#3014)
  Implement version major.minor.release-stage.build (LMMS#3011)
  Fix regressions on loading broken projects (LMMS#3013)
  Improved file input validation. (LMMS#2523)
  Fix sample track view in BB editor (LMMS#3002)
  Request change in model when dropping a track (LMMS#3000)
  Add LocklessAllocator and use it in LocklessList (LMMS#2998)
  Drop forceStep in AutomatableModel (LMMS#3010)
  ...
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
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.

5 participants