1112875 - Land web-audio-automation-timeline library

Status: RESOLVED FIXED

(DevTools Graveyard :: Web Audio Editor, defect)

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1112875-add-automation-timeline.patch

Paul, here's my JS lib for implementing the automation timeline in JS for rendering the curves in the devtools.

Gerv, is there any security/legal reviews needed for an external lib? MIT License, and I wrote it as well.

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

Paul, for the original source and tests, etc., view the original repo: https://github.com/jsantell/web-audio-automation-timeline

Comment 3

[Paul Adenot (:padenot)]

Comment on attachment 8538139 [details] [diff] [review]
1112875-add-automation-timeline.patch

Review of attachment 8538139 [details] [diff] [review]:
-----------------------------------------------------------------

It could be nice to test this against the C++ implementation somehow to make sure they stay in sync, but I'm not too sure how to do it.

::: toolkit/devtools/server/actors/utils/automation-timeline.js
@@ +3,5 @@
> + * https://github.com/jsantell/web-audio-automation-timeline
> + * MIT License, copyright (c) 2014 Jordan Santell
> + */
> +!function(e){if("object"==typeof exports&&"undefined"!=typeof module)module.exports=e();else if("function"==typeof define&&define.amd)define([],e);else{var f;"undefined"!=typeof window?f=window:"undefined"!=typeof global?f=global:"undefined"!=typeof self&&(f=self),f.Timeline=e()}}(function(){var define,module,exports;return (function e(t,n,r){function s(o,u){if(!n[o]){if(!t[o]){var a=typeof require=="function"&&require;if(!u&&a)return a(o,!0);if(i)return i(o,!0);var f=new Error("Cannot find module '"+o+"'");throw f.code="MODULE_NOT_FOUND",f}var l=n[o]={exports:{}};t[o][0].call(l.exports,function(e){var n=t[o][1][e];return s(n?n:e)},l,l.exports,e,t,n,r)}return n[o].exports}var i=typeof require=="function"&&require;for(var o=0;o<r.length;o++)s(r[o]);return s})({1:[function(require,module,exports){
> +module.exports = require("./lib/timeline").Timeline;

What is that? Also is it the same license as the rest?

@@ +62,5 @@
> +  return v1 + (v0 - v1) * Math.exp(-(t - t0) / timeConstant);
> +};
> +
> +// Since we are going to accumulate error by adding 0.01 multiple times
> +// in a loop, we want to fuzz teh equality check in `getValueAtTime`

nit: s/teh/the/

@@ +271,5 @@
> +      if (ev.type === events[i].type) {
> +        // If times and types are equal, replace the event;
> +        events[i] = ev;
> +      } else {
> +        // Otherwise, place the elemetn after the last event of another type

nit: s/elemetn/element/

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8538139 [details] [diff] [review]
1112875-add-automation-timeline.patch

Review of attachment 8538139 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/devtools/server/actors/utils/automation-timeline.js
@@ +3,5 @@
> + * https://github.com/jsantell/web-audio-automation-timeline
> + * MIT License, copyright (c) 2014 Jordan Santell
> + */
> +!function(e){if("object"==typeof exports&&"undefined"!=typeof module)module.exports=e();else if("function"==typeof define&&define.amd)define([],e);else{var f;"undefined"!=typeof window?f=window:"undefined"!=typeof global?f=global:"undefined"!=typeof self&&(f=self),f.Timeline=e()}}(function(){var define,module,exports;return (function e(t,n,r){function s(o,u){if(!n[o]){if(!t[o]){var a=typeof require=="function"&&require;if(!u&&a)return a(o,!0);if(i)return i(o,!0);var f=new Error("Cannot find module '"+o+"'");throw f.code="MODULE_NOT_FOUND",f}var l=n[o]={exports:{}};t[o][0].call(l.exports,function(e){var n=t[o][1][e];return s(n?n:e)},l,l.exports,e,t,n,r)}return n[o].exports}var i=typeof require=="function"&&require;for(var o=0;o<r.length;o++)s(r[o]);return s})({1:[function(require,module,exports){
> +module.exports = require("./lib/timeline").Timeline;

Browserified header -- MIT license, but confirming with authors that the generated boilerplate for this is also MIT licensed.

Comment 5

[Jordan Santell [:jsantell] [@jsantell]]

Updated nits in source file and rebuilt in the patch. Not sure how they can stay in sync outside of a common test suite (which all the tests were ported over, hence finding bug 1110344. I'll regularly will poll AudioEventTimeline.h for changes, but barring any spec changes, there shouldn't be any big changes that will make this no longer work. It's purpose is to represent the timeline curves in web audio, and they are approximated in some areas for speed, and can and will differ from the underlying implementation (nature of JS's number system, and for instance in bug 1110344 -- in this one case, this library follows spec, but there's still a bug in the c++ implementation).

As long as the curve can be rendered rather accurately, it'd still be a win for debugging tool.

Comment 6

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1112875-add-automation-timeline.patch

Other than confirming licensing, anything else you'd like to see for this? The server using it is implemented in bug 1056458 if you'd like to see the usage

Comment 7

[Gervase Markham [:gerv]]

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #1)
> Gerv, is there any security/legal reviews needed for an external lib? MIT
> License, and I wrote it as well.

Well, the MIT license requires reproduction of the license text in about:license - but you haven't actually included the license text which contains that requirement in the file, so it's not clear which text to reproduce! Which leads to a rather odd situation.

If what you mean is "do what you like with this, just don't sue me", then fine :-) No problems.

Gerv

Comment 8

[Jordan Santell [:jsantell] [@jsantell]]

The browserify bundle that packages the file is also released via MIT: https://github.com/substack/browser-pack/blob/master/prelude.js

Comment 9

[Jordan Santell [:jsantell] [@jsantell]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c630b1791edb

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/60ae30c56e26

Tests? :)

Comment 11

[Jordan Santell [:jsantell] [@jsantell]]

This is an external lib, and tested separately: https://travis-ci.org/jsantell/web-audio-automation-timeline

We can copy the tests for this within m-c, but bug 1056458 will use this lib directly as well and has complete tests already

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/60ae30c56e26