Implement suspend() and resume() for OfflineAudioContext

Source/modules/webaudio/OfflineAudioContext.cpp

 /*
  * Copyright (C) 2012, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1.  Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 13 matching lines @@
 
 #include "config.h"
 #if ENABLE(WEB_AUDIO)
 #include "modules/webaudio/OfflineAudioContext.h"
 
 #include "bindings/core/v8/ExceptionMessages.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/ExecutionContext.h"
+#include "modules/webaudio/OfflineAudioCompletionEvent.h"
+#include "modules/webaudio/OfflineAudioDestinationNode.h"
+#include "platform/ThreadSafeFunctional.h"
 #include "platform/audio/AudioUtilities.h"
+#include "public/platform/Platform.h"
 
 namespace blink {
 
 OfflineAudioContext* OfflineAudioContext::create(ExecutionContext* context, unsigned numberOfChannels, size_t numberOfFrames, float sampleRate, ExceptionState& exceptionState)
 {
     // FIXME: add support for workers.
     if (!context || !context->isDocument()) {
         exceptionState.throwDOMException(
             NotSupportedError,
             "Workers are not supported.");
@@ skipping 40 matching lines @@
             + ", " + String::number(sampleRate)
             + ")");
     }
 
     audioContext->suspendIfNeeded();
     return audioContext;
 }
 
 OfflineAudioContext::OfflineAudioContext(Document* document, unsigned numberOfChannels, size_t numberOfFrames, float sampleRate)
     : AudioContext(document, numberOfChannels, numberOfFrames, sampleRate)
+    , m_isRenderingStarted(false)
+    , m_totalRenderFrames(numberOfFrames)
 {
 }
 
 OfflineAudioContext::~OfflineAudioContext()
 {
 }
 
+// FIXME: What should be done to trace members in OfflineAudioContext?

[haraken, 2015/06/17 06:46:08]

This trace method looks correct. You can just trace all members.

[hongchan, 2015/06/17 20:10:39]

It produces error during the compilation. Probably it is because the oilpan
option was not enabled.

+// DEFINE_TRACE(OfflineAudioContext)
+// {
+//     visitor->trace(m_scheduledSuspends);
+//     visitor->trace(m_completeResolver);
+// }
+
+OfflineAudioContext::ScheduledSuspendContainer::ScheduledSuspendContainer(
+    size_t suspendFrame, PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
+    : m_suspendFrame(suspendFrame)
+    , m_resolver(resolver)
+    , m_isPending(false)
+{
+}
+
+OfflineAudioContext::ScheduledSuspendContainer::~ScheduledSuspendContainer()
+{
+}
+
+DEFINE_TRACE(OfflineAudioContext::ScheduledSuspendContainer)
+{
+    visitor->trace(m_resolver);
+}
+
+PassOwnPtr<OfflineAudioContext::ScheduledSuspendContainer>
+OfflineAudioContext::ScheduledSuspendContainer::create(
+    size_t suspendFrame, PassRefPtrWillBeRawPtr<ScriptPromiseResolver> resolver)
+{
+    return adoptPtr(new ScheduledSuspendContainer(suspendFrame, resolver));
+}
+
+bool OfflineAudioContext::ScheduledSuspendContainer::shouldSuspendAt(size_t whenFrame) const
+{
+    if (m_suspendFrame != whenFrame)
+        return false;
+
+    return true;
+}
+
+bool OfflineAudioContext::ScheduledSuspendContainer::isPending() const
+{
+    return m_isPending;
+}
+
+void OfflineAudioContext::ScheduledSuspendContainer::markAsPending()
+{
+    m_isPending = true;
+}
+
+bool OfflineAudioContext::shouldSuspendNow()
+{
+    ASSERT(!isMainThread());
+
+    // Suspend if necessary and mark the associated promise as pending. Marked
+    // promises will be resolved later. Note that duplicate entries in the
+    // suspend list are prohibited so it returns immediately when a valid
+    // suspend is found. This duplicate check is done by
+    // |suspendOfflineRendering|.
+    size_t nowFrame = currentSampleFrame();
+    for (unsigned index = 0; index < m_scheduledSuspends.size(); ++index) {
+        if (m_scheduledSuspends.at(index)->shouldSuspendAt(nowFrame)) {
+            m_scheduledSuspends.at(index)->markAsPending();
+            return true;
+        }
+    }
+
+    return false;
+}
+
+void OfflineAudioContext::resolvePendingSuspendPromises()
+{
+    ASSERT(!isMainThread());
+
+    // Resolve promises marked as 'pending'.
+    if (m_scheduledSuspends.size() > 0) {
+        Platform::current()->mainThread()->postTask(FROM_HERE,
+            threadSafeBind(&OfflineAudioContext::resolvePendingSuspendPromisesOnMainThread, this));
+    }
+}
+
+void OfflineAudioContext::fireCompletionEvent()
+{
+    ASSERT(isMainThread());
+    if (!isMainThread())

[haraken, 2015/06/17 06:46:08]

Basically it is not a good idea to add both ASSERT(x()) and if (!x()). If !x()
shouldn't happen, we should only have ASSERT(x()). The same comment for other
places in this CL.

[hongchan, 2015/06/17 20:10:39]

Done.

+        return;
+
+    // We set the state to closed here so that the oncomplete event handler sees
+    // that the context has been closed.
+    setContextState(Closed);
+
+    AudioBuffer* renderedBuffer = renderTarget().get();
+
+    ASSERT(renderedBuffer);
+    if (!renderedBuffer)
+        return;
+
+    // Avoid firing the event if the document has already gone away.
+    if (executionContext()) {
+        // Call the offline rendering completion event listener and resolve the
+        // promise too.
+        dispatchEvent(OfflineAudioCompletionEvent::create(renderedBuffer));
+        m_completeResolver->resolve(renderedBuffer);
+    }
+}
+
 ScriptPromise OfflineAudioContext::startOfflineRendering(ScriptState* scriptState)
 {
+    ASSERT(isMainThread());
+
     // Calling close() on an OfflineAudioContext is not supported/allowed,
     // but it might well have been stopped by its execution context.
     if (isContextClosed()) {
         return ScriptPromise::rejectWithDOMException(
             scriptState,
             DOMException::create(
                 InvalidStateError,
                 "cannot call startRendering on an OfflineAudioContext in a stopped state."));
     }
 
-    if (m_offlineResolver) {
+    if (m_completeResolver) {
         // Can't call startRendering more than once.  Return a rejected promise now.
         return ScriptPromise::rejectWithDOMException(
             scriptState,
             DOMException::create(
                 InvalidStateError,
                 "cannot call startRendering more than once"));
     }
 
-    m_offlineResolver = ScriptPromiseResolver::create(scriptState);
-    startRendering();
-    return m_offlineResolver->promise();
+    m_completeResolver = ScriptPromiseResolver::create(scriptState);

[yhirano, 2015/06/17 01:07:50]

Shouldn't this statement be placed after the L239-L245 block?

[hongchan, 2015/06/17 20:10:39]

Done.

+
+    // If the context is not in the suspended state, reject the promise.
+    if (contextState() != AudioContextState::Suspended) {
+        return ScriptPromise::rejectWithDOMException(
+            scriptState,
+            DOMException::create(
+                InvalidStateError,
+                "cannot startRendering when an OfflineAudioContext is not in a suspended state"));
+    }
+
+    // Start rendering and return the promise.
+    m_isRenderingStarted = true;
+    setContextState(Running);
+    destination()->audioDestinationHandler().startRendering();
+    return m_completeResolver->promise();
+}
+
+ScriptPromise OfflineAudioContext::suspendOfflineRendering(ScriptState* scriptState, double when)
+{
+    ASSERT(isMainThread());

[yhirano, 2015/06/17 01:07:50]

if (state == closed) { return Promise.reject(...) } is needed? How about when it
is already suspended?

[hongchan, 2015/06/17 20:10:39]

1) OfflineAudioContext does not have an explicit close() method. I think it is
not possible to be a 'closed' state unless it reaches the end of the rendering.

2) Calling suspend(t2) when the context is suspended by suspend(t1) is a
legitimate pattern. However, we check the time argument of suspend() call so we
can avoid duplicates in the scheduled suspension. In other words, we allow to
call suspend(t2) when (t1 != t2).

I can share a mini design doc for the usage pattern if that's helpful to you.

[yhirano, 2015/06/18 13:40:52]

I see, thanks!

+
+    // The specified suspend time is negative, reject the promise.
+    if (when < 0) {
+        return ScriptPromise::rejectWithDOMException(
+        scriptState,
+        DOMException::create(
+            InvalidStateError,
+            "negative suspend time (" + String::number(when) + ") is not allowed"));
+    }
+
+    // Quantize the suspend time to the rendering block boundary.
+    size_t quantizedFrame = destination()->audioDestinationHandler().quantizeTimeToRenderQuantum(when);
+
+    // The specified suspend time is in the past, reject the promise.
+    if (quantizedFrame < currentSampleFrame()) {
+        return ScriptPromise::rejectWithDOMException(
+        scriptState,
+        DOMException::create(
+            InvalidStateError,
+            "cannot schedule a suspend at frame " + String::number(quantizedFrame) +
+            " (" + String::number(when) + " seconds) because it is earlier than the current frame of " +
+            String::number(currentSampleFrame())));
+    }
+
+    // The suspend time should be earlier than the total render frame. If the
+    // requested suspension time is equal to the total render frame, the promise
+    // will be rejected.
+    if (m_totalRenderFrames <= quantizedFrame) {
+        return ScriptPromise::rejectWithDOMException(
+        scriptState,
+        DOMException::create(
+            InvalidStateError,
+            "cannot schedule a suspend at frame " + String::number(quantizedFrame) +
+            " (" + String::number(when) + " seconds) because it is greater than or equal to the total render duration of " +
+            String::number(m_totalRenderFrames) + " frames"));
+    }
+
+    // If there is a duplicate suspension at the same quantize frame, reject the
+    // promise.
+    for (unsigned index = 0; index < m_scheduledSuspends.size(); ++index) {
+        if (m_scheduledSuspends.at(index)->shouldSuspendAt(quantizedFrame)) {
+            return ScriptPromise::rejectWithDOMException(
+            scriptState,
+            DOMException::create(
+                InvalidStateError,
+                "cannot schedule more than one suspend at frame " + String::number(quantizedFrame) +
+                " (" + String::number(when) + " seconds)"));
+        }
+    }
+
+    RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
+    ScriptPromise promise = resolver->promise();
+    m_scheduledSuspends.append(ScheduledSuspendContainer::create(quantizedFrame, resolver));
+
+    return promise;
+}
+
+ScriptPromise OfflineAudioContext::resumeOfflineRendering(ScriptState* scriptState)
+{
+    ASSERT(isMainThread());
+
+    AutoLocker locker(this);

[Raymond Toy, 2015/06/17 15:56:10]

destination()->audioDestinationHandler().startRendering() is run while this lock
is held, right?  Won't this prevent any thing on the main thread from happening
until startRendering() returns?  If so, this is probably not what we want.

[hongchan, 2015/06/17 20:10:39]

Yes. I agree. However, it didn't matter because startRendering() actually jumps
to the other thread. So the lock will be released when this scope is over.

Will remove this.

+
+    // If the context is not in a suspended state, reject the promise.
+    if (contextState() != AudioContextState::Suspended) {
+        return ScriptPromise::rejectWithDOMException(
+            scriptState,
+            DOMException::create(
+                InvalidStateError,
+                "cannot resume a context that is not suspended"));
+    }
+
+    // If the rendering has not started, reject the promise.
+    if (!m_isRenderingStarted) {
+        return ScriptPromise::rejectWithDOMException(
+            scriptState,
+            DOMException::create(
+                InvalidStateError,
+                "cannot resume a context that has not started"));
+    }
+
+    // If the context is suspended, resume rendering by calling startRendering()
+    // and set the state to "Running." Note that resuming is possible only after
+    // the rendering started.
+    setContextState(Running);
+
+    destination()->audioDestinationHandler().startRendering();
+
+    RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
+    ScriptPromise promise = resolver->promise();
+
+    // Resolve the promise immediately.
+    resolver->resolve();
+
+    return promise;
+}
+
+void OfflineAudioContext::resolvePendingSuspendPromisesOnMainThread()
+{
+    ASSERT(isMainThread());
+    AutoLocker locker(this);

[haraken, 2015/06/17 06:46:09]

Why do you need this lock?

[hongchan, 2015/06/17 20:10:39]

When we resolve the suspend promise, the script can change the graph. So I
wanted to make sure the graph is not touched by the other thread while being
manipulated.

+
+    // Suspend the context first. This will fire onstatechange event.
+    setContextState(Suspended);
+
+    // FIXME: is removing elements efficient? What if there are 10K suspends?

[haraken, 2015/06/17 06:46:08]

FIXME => TODO

[hongchan, 2015/06/17 20:10:39]

Done.

+    // Resolve any pending suspend and remove it from the list.
+    bool pendingPromiseResolved = false;
+    for (unsigned index = 0; index < m_scheduledSuspends.size();) {
+        if (m_scheduledSuspends.at(index)->isPending()) {
+
+            // We should never have more than one pending suspend at any time.
+            ASSERT(!pendingPromiseResolved);
+
+            m_scheduledSuspends.at(index)->resolver()->resolve();
+            m_scheduledSuspends.remove(index);
+
+            pendingPromiseResolved = true;
+        } else {
+            ++index;
+        }
+    }
 }
 
 } // namespace blink
 
 #endif // ENABLE(WEB_AUDIO)