search

justinsalamon / scaper

A library for soundscape synthesis and augmentation
BSD 3-Clause "New" or "Revised" License
384 stars 56 forks source link

Add SNR parameter to background events #48

Closed beasteers closed 4 years ago

beasteers commented 5 years ago

This change is Reviewable

beasteers commented 5 years ago

All of those commits except for the last should go away once you merge #40

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling d4a22ea41709bf08f9df8be7cd5ce82b816aab4f on bensteers:bgsnr into 18e679f4ae4f9b0fa4e646eabbde363f81d06f48 on justinsalamon:master.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling d4a22ea41709bf08f9df8be7cd5ce82b816aab4f on bensteers:bgsnr into 18e679f4ae4f9b0fa4e646eabbde363f81d06f48 on justinsalamon:master.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling a3955f8816883f1afdf02a7f05419be4041608fd on bensteers:bgsnr into 3c83bba96c146025119516b4e1a6f3ef7ff5bfad on justinsalamon:master.

beasteers commented 5 years ago

This is in reference to #47

justinsalamon commented 5 years ago

As noted on #47, SNR is relative so it doesn't make sense for a background track to have an SNR value. Super appreciate the contribution but perhaps best to first discuss in an issue, as I don't think I can merge this PR.

justinsalamon commented 4 years ago

Supporting this functionality, if we decide to, requires the solution to be carefully spec'd out prior to implementation. cf. discussion on #47.

Since this PR has diverged quite significantly from the master branch at this point (and since I think we should first converge on a design and only then implement it), I'm going to close this PR out.

Thanks again, hopefully we can get to this feature in the future!