Closed Bug 1112875 Opened 5 years ago Closed 5 years ago

Land web-audio-automation-timeline library

Categories

(DevTools Graveyard :: Web Audio Editor, defect)

37 Branch
x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 37

People

(Reporter: jsantell, Assigned: jsantell)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Assignee: nobody → jsantell
Blocks: 1056458
Status: NEW → ASSIGNED
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.
Flags: needinfo?(gerv)
Attachment #8538139 - Flags: review?(padenot)
Paul, for the original source and tests, etc., view the original repo: https://github.com/jsantell/web-audio-automation-timeline
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/
Attachment #8538139 - Flags: review?(padenot)
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.
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.
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
Attachment #8538139 - Attachment is obsolete: true
Attachment #8538495 - Flags: review?(padenot)
(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
Flags: needinfo?(gerv)
Attachment #8538495 - Flags: review?(padenot) → review+
The browserify bundle that packages the file is also released via MIT: https://github.com/substack/browser-pack/blob/master/prelude.js
https://hg.mozilla.org/integration/fx-team/rev/60ae30c56e26

Tests? :)
Flags: in-testsuite?
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
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
https://hg.mozilla.org/mozilla-central/rev/60ae30c56e26
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 37
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.