Closed Bug 938022 Opened 11 years ago Closed 11 years ago

context.createMediaElementSource() breaks audio element currentTime/ontimeupdate event


(Core :: Web Audio, defect)

25 Branch
Not set





(Reporter: lapsio3, Assigned: roc)




(7 files, 2 obsolete files)

12.04 KB, patch
: review+
Details | Diff | Splinter Review
8.15 KB, patch
: review+
Details | Diff | Splinter Review
2.12 KB, patch
: review+
Details | Diff | Splinter Review
938 bytes, patch
: review+
Details | Diff | Splinter Review
5.26 KB, patch
: review+
Details | Diff | Splinter Review
14.03 KB, patch
: review+
Details | Diff | Splinter Review
799 bytes, text/html
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.101 Safari/537.36

Steps to reproduce:

create AudioContext
use <audio> element as source for context with MediaElementAudioSourceNode
check <audio> playback progress (currentTime)


Actual results:

After attaching to context timeupdate doesn't fire anymore, currentTime is also not changing. However manual setting value of currentTime does change playback progress as well as currenTime property itself, even though meassuring current progress is still impossible.

Expected results:

Audio element should still fire timeupdate/change currentTime value properly
This is pretty much the same issue as bug 934100, but has a simpler test case.
Blocks: 934100
Ever confirmed: true
This is different from what we talked about. Instead of redefining mAudioEndTime, this roughly keeps the current definition ("data pushed to output stream").
Assignee: nobody → roc
Attachment #8335070 - Flags: review?(cpearce)
This fixes a pretty major bug in the capture code. When the MediaDecoderStateMachine stops playback due to buffering, we were still feeding data into the MediaStream (and playing it) as we decoded whatever we had. That's bad.
Attachment #8335072 - Flags: review?(cpearce)
Attachment #8335070 - Flags: review?(cpearce) → review+
Attachment #8335072 - Flags: review?(cpearce) → review+
To get tests to work I had to fix more things. Additional patches coming.
Attachment #8337926 - Flags: review?(paul) → review+
Attachment #8337923 - Flags: review?(cpearce) → review+
Attachment #8337925 - Flags: review?(cpearce) → review+
Comment on attachment 8337927 [details] [diff] [review]
Part 6: Have MediaDecoder/MediaDecoderStateMachine that's producing a MediaStream use that stream's current time as the media clock

Review of attachment 8337927 [details] [diff] [review]:

::: content/media/MediaDecoder.cpp
@@ +206,5 @@
>      mHaveBlockedForStateMachineNotPlaying(false)
>  {
>    mStream->AddMainThreadListener(this);
> +  mListener = new DecodedStreamGraphListener(mStream);
> +  aStream->AddListener(mListener);

s/aStream/mStream/ for consistency.

::: content/media/MediaDecoderStateMachine.cpp
@@ +2447,5 @@
>    AssertCurrentThreadInMonitor();
>    // Determine the clock time. If we've got audio, and we've not reached
>    // the end of the audio, use the audio clock. However if we've finished
>    // audio, or don't have audio, use the system clock.

Update this comment to reflect the fact that we can now also use a clock coming from a MediaStreamGraph.

::: content/media/MediaDecoderStateMachine.h
@@ +327,5 @@
>      }
>      mDecoder = nullptr;
>    }
> +  void SetSyncPointForMediaStream();

Can you add a short comment on this one?
Attachment #8337927 - Flags: review?(paul) → review+
Blocks: 944924
Depends on: 950509
Flags: in-testsuite?
Attached file test_bug938022.html (obsolete) —
Here's a test case for this bug. When the "playing" event is fired, the currentTime is set to the duration of the audio, if the audio element is connected to the AudioContext's graph via a MediaElementSourceNode. This checks if the currentTime is 0 at the beginning of playback.
Depends on: 973782
Removed the test due to bug 973782
Flags: in-testsuite+ → in-testsuite?
Attached file test_bug938022.html (obsolete) —
Apparently, the "playing" event can fire after the media starts playing, i.e. when the playing event fires the currentTime may have already advanced past zero. This replacement test case instead adds a listener on the "loadeddata" event which should be fired when the currentTime is 0 (in versions where the bug has been fixed), and also asserts that currentTime is not the duration of the <audio> element.
Attachment #8376912 - Attachment is obsolete: true
Attached file test_bug938022.html
This is just a small amendment to verify currentTime == 0 rather than currentTime != duration, as it is a more accurate check.
Attachment #8377972 - Attachment is obsolete: true
Reverted the test.

roc tells me (though not speaker in the precise context of this test) that the state of media elements can change before events are processed.
Flags: in-testsuite+ → in-testsuite?
Depends on: 1113600
You need to log in before you can comment on or make changes to this bug.