reaper-oss / sws

The SWS extension is a collection of features that seamlessly integrate into REAPER, the Digital Audio Workstation (DAW) software by Cockos, Inc
https://www.sws-extension.org/
MIT License
448 stars 85 forks source link

FR (API): NF_GetMediaItemMaxPeak should return position #953

Closed X-Raym closed 5 years ago

X-Raym commented 6 years ago

Hi !

Could reaper.NF_GetMediaItemMaxPeak( item )also return the absolute pos (relative to item left edges) of this peaks ?

NF_AnalyzeTakeLoudness can do that, but when we don't need Loudness, it is a bit overkill (because it display the SWS analyse window and is less performant that this simple peak function).

Cheers !

nofishonfriday commented 6 years ago

Version for testing:

https://www.dropbox.com/s/szk73cd4wb6tzcl/reaper_sws64.dll?dl=1

I didn't want to break scripts already using NF_GetMediaItemMaxPeak() by changing functionality of this function, so I've added NF_GetMediaItemMaxPeakAndMaxPeakPos()

Test code:

reaper.ShowConsoleMsg("")

function msg(m)
  return reaper.ShowConsoleMsg(tostring(m) .. "\n")
end

item = reaper.GetSelectedMediaItem(0, 0)
maxPeak, maxPeakPos = reaper.NF_GetMediaItemMaxPeakAndMaxPeakPos(item)

msg("max peak: " .. maxPeak)
msg("max peak pos: " .. maxPeakPos)

-- set edit cursor to max peak pos,
-- should match with "SWS: Move cursor to item peak sample"
reaper.SetEditCurPos(maxPeakPos, true, false);

Btw.

because it display the SWS analyse window

NF_GetMediaItemMaxPeak() also always did display the analyze window. :P (Sorry, nothing I can do about it.) But it should at least be quite faster getting the peak pos. with it now than using the NF_AnalyzeTakeLoudness() for it. :)

Let me know how it goes with testing.

X-Raym commented 6 years ago

@nofishonfriday thanks for taking a look !

Well, It doesn't seems to give the same results as the Loudness API function:

demo

NF_GetMediaItemMaxPeak() also always did display the analyze window.

How come ? reaper.NF_GetMediaItemMaxPeak( item ) doesn't return if I remember correctly ?

Anyway, it is several time faster than the Loudness function indeed !

nofishonfriday commented 6 years ago

I think the difference in results comes because the Loudness function uses true peaks measurement while GetMediaItemMaxPeak doesn't. You can read about true peak measurement here: https://tech.ebu.ch/docs/tech/tech3341.pdf

In short: True peak also takes waveform 'overshoots' into account, so it's more exact with the drawback that it's also slower (since it uses oversampling). If you compare the return values of both functions, the Loudness function should always return a true peak value >= the GetMediaItemMaxPeak function (edit: Actually I think it's always > in (almost ?) all cases), which means, positions of peak and true peak can also be different.

I just tried with a random audio item:

max peak: 0.15545237637443
max peak pos: 1670.6489488957
true peak: 0.19827933103314
true peak pos.: 301.60463718821

How come ? reaper.NF_GetMediaItemMaxPeak( item ) doesn't return if I remember correctly ?

If by 'doesn't return' you mean doesn't open the analyze window, here's the discussion we had about it. :dagger:

https://github.com/reaper-oss/sws/pull/856

edit: Hold on, there may be a bug in the truePeakPos, need to check...

X-Raym commented 6 years ago

@nofishonfriday I understand true peaks and max peak isn't the same, but don't you think the Max Peak Pos results I get in my screenshot above looks a bit off ?

In short: True peak also takes waveform 'overshoots' into account, so it's more exact with the drawback that it's also slower (since it uses oversampling).

In that case, it appears that True Peaks will be more useful than max peak then 🐍

nofishonfriday commented 6 years ago

You're right, as said above, there's definitely a bug currently, sorry. New version coming in some minutes...

nofishonfriday commented 6 years ago

New version: https://www.dropbox.com/s/szk73cd4wb6tzcl/reaper_sws64.dll?dl=1

TruePeakPos returned wrong results when item start not at 0.0, as in your gif. This version should now give more sensible results.

X-Raym commented 6 years ago

@nofishonfriday this new version has the same problem with the TruePeaks and MaxPeaks function: the value isn't the same (not in absolute or not even in relative) if items position change:

bug

EDIT: I confirm NF_AnalyzeTakeLoudness is broken in last build (from your comment), previous behavior was consistent results no matter the item the position.

nofishonfriday commented 6 years ago

Which version you mean worked correctly ? The official v2.9.7, and the first build I posted here broke it ? Or did I misunderstand ?

X-Raym commented 6 years ago

https://github.com/reaper-oss/sws/issues/953#issuecomment-370274541 this one is broken.

I think the working version I restored is the actual public 2.9.7 (I'm not extra sure, I have a lots of SWS dll files now ^^)

nofishonfriday commented 6 years ago

Just to be clear, truePeakPos and maxPeakPos are supposed to give the absolute time counting from project start. So I'm currently wondering is that a valid test what you're doing with setting snap offset = truePeakPos in your above gif, as snap offset is defined relative from item start, no ? But maybe I'm confused now...

My test procedure is as follows:

item = reaper.GetSelectedMediaItem(0, 0)
take = reaper.GetActiveTake(item)
_, _, _, truePeak, truePeakPos, shortTermMax, momentaryMax, shortTermMaxPos, momentaryMaxPos = reaper.NF_AnalyzeTakeLoudness2(take, true)
maxPeak, maxPeakPos = reaper.NF_GetMediaItemMaxPeakAndMaxPeakPos(item)
reaper.SetEditCurPos(truePeakPos, true, false)
-- reaper.SetEditCurPos(maxPeakPos, true, false)
-- insert a stretch marker to mark this position
reaper.Main_OnCommand(41842, 0) -- Item: Add stretch marker at cursor

Now move item somewhere else and run script again, cursor should land on same position in item This works here for both maxPeakPos and truePeakPos with the latest build I posted here, so I'm wondering where the flaw is ?

X-Raym commented 6 years ago

snap offset is defined relative from item start, no ?

Indeed.

truePeakPos [is] supposed to give the absolute time counting from project start.

Actually, it is not the behavior I see in public 2.9.7. It works relative there. And I like it this way, I think it make more sense as it is a extrapolated property of the item.

maxPeakPos should also be relative.

It seems it breaks right from https://github.com/reaper-oss/sws/issues/953#issuecomment-370258332 test release.

nofishonfriday commented 6 years ago

Ah now we're getting somewhere. :D You're abslolutely right, I actually never intended it to work relative and was unaware that it did in the public 2.9.7, so when I discovered it shortly ago I regarded it as bug and thought I fix immediately to work absolute haha. Ok, so I think the conclusion is the new GetMediaItemMaxPeakAndMaxPeakPos (which currently works absolute) should also work relative for consistency (and Loudness reverted to relative)?

nofishonfriday commented 6 years ago

Next try, both should work relative (again) now: https://www.dropbox.com/s/szk73cd4wb6tzcl/reaper_sws64.dll?dl=1

Code I'm using for testing:

reaper.ShowConsoleMsg("")

function msg(m)
  return reaper.ShowConsoleMsg(tostring(m) .. "\n")
end

item = reaper.GetSelectedMediaItem(0, 0)
take = reaper.GetActiveTake(item)

_, _, _, truePeak, truePeakPos, _, _, _, _ = reaper.NF_AnalyzeTakeLoudness2(take, true)
maxPeak, maxPeakPos = reaper.NF_GetMediaItemMaxPeakAndMaxPeakPos(item)

msg("max peak: " .. maxPeak)
msg("rel. max peak pos: " .. maxPeakPos)
msg("true peak: " .. truePeak)
msg("rel. true peak pos: " .. truePeakPos)

reaper.SetMediaItemInfo_Value(item, "D_SNAPOFFSET", maxPeakPos) -- snapoffset should match with "SWS: Move cursor to item peak sample"
-- reaper.SetMediaItemInfo_Value(item, "D_SNAPOFFSET", truePeakPos) -- snapoffset should match with SWS/BR: Analyze loudness... -> Go to true peak

reaper.UpdateArrange()
X-Raym commented 6 years ago

@nofishonfriday Ahh that is good ! It works faster than the loudness function, and does works with complex scenarios (rate, stretch markers etc).

demo

Note that this MaxPeaks function is post fades and post envelopes, contrary to the true peaks one.

Fades/Envelopes

This is a significant difference between the two.

Maybe at some point a post fade/ post envelopes NF_AnalyzeTakeLoudness request ticket could be proposed, but this is another subject 😄 (though, it may be good to add specify this behavior in the API doc, at it is not said, contrary to the max peak function for which it is written that it is post fades / post gain / post envelopes etc...).

nofishonfriday commented 6 years ago

Thanks for testing (and the patience to get it right. :D) Good point btw. seeing the true/max peak positions as property of the item, it makes sense, so having this as relative also makes more sense.

Good point also about the API doc,, I'll add this, thanks. Looking at here http://wiki.cockos.com/wiki/index.php/Measure_and_normalize_loudness_with_SWS it seems it should actually be possible to at least have post envelopes included in the Loudness analyzis as Breeder seems to do it, I may have a look at some point...

X-Raym commented 6 years ago

@nofishonfriday thansks for your time and determination to get it right :P

I may have a look at some point...

Do you want me to open a new ticket about that ? ^^

nofishonfriday commented 6 years ago

Yes, probably better so it doesn't get lost. (As implemented issues such as this one will get auto-closed when merged with Master branch as Tim told me.)

nofishonfriday commented 5 years ago

addid in v2.10.0