The orginal test case has an incorrect use on logResult() function, a corrected
version attached.

Also refer to attached screen-shot for reference

	ss100-endedevent.gif 80.0 KB   View   Download

video-currentTime-set.html 1.0 KB   View   Download

There are a number of issues relating to end of file.
The pipeline gets into a bad state.

One issue is the UI doesn't update. Videos in the test matrix are muxed with 500ms 
audio packets.  The time bar stops 500 ms short of the end.  This is especially evident 
on the 'still' frame which is 1 second of audio, so the time only goes half way.
A work around is to mux the audio at a finer level.
Once we fix the end of file issue, the UI should just work.

scherkus, if you could please assign.

(No comment was entered for this change.)

 Issue 13930  has been merged into this issue.

 issue 14702  is a bug, which sounds similar to this, but is related to the <audio> 
event. If it's not relevant, please delete this comment.

It is indeed related to this issue -- we're simply not firing the ended event at all.  
I'll mark your other bug as a duplicate.

For future reference if you add "Video" as a label a bunch of us will get notified and 
we can assign the bug, otherwise it can be harder to find.

Thanks again for the report!

 Issue 14702  has been merged into this issue.

I'm not sure I know how to label, so I'll look into this. I've raised a few <audio>
related tickets, mostly as I find them in my own project. I'll see if I can get them
flagged better, and hopefully get some fixes.

thank you for the advice.

(No comment was entered for this change.)

Word on the street is implementing a proper clock has fixed this.

Will need to verify.

...almost there :)

We do fire ended events, but there are still some bugs: we continuously fire "ended" 
and "seeked" events and then the player gets all confused.

Alpha I'm going to take this over since it's related to the changes I've been making 
recently.

ok, feel free to take it :)

(No comment was entered for this change.)

(No comment was entered for this change.)

 Issue 18860  has been merged into this issue.

Should be fixed as of r23255.

Test still fails, but due to other subtle differences between WebKit and Chrome 
implementations.  Closing.

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=23255 

------------------------------------------------------------------------
r23255 | scherkus@chromium.org | 2009-08-12 16:52:05 -0700 (Wed, 12 Aug 2009) | 12 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/filter_host.h?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/filters.h?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/mock_filter_host.h?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/mock_filters.h?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/pipeline_impl.cc?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/pipeline_impl.h?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/pipeline_impl_unittest.cc?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/video_frame_impl.cc?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/video_frame_impl.h?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/video_frame_impl_unittest.cc?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/audio_renderer_base.cc?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/audio_renderer_base.h?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/audio_renderer_base_unittest.cc?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/video_renderer_base.cc?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/video_renderer_base.h?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/video_renderer_base_unittest.cc?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/webmediaplayer_impl.cc?r1=23255&r2=23254
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/webmediaplayer_impl.h?r1=23255&r2=23254

Implemented end-of-stream callback for media::PipelineImpl.

A new method HasEnded() was added to renderer interfaces.  Renderers return true when they have both received and rendered an end-of-stream buffer.  For audio this translates to sending the very last buffer to the hardware.  For video this translates to displaying a black frame after the very last frame has been displayed.

Renderers can notify the pipeline that the value of HasEnded() has changed to true via FilterHost::NotifyEnded().  Instead of tracking which renderers have called NotifyEnded(), the pipeline uses the notification to poll every renderer.  The ended callback will only be executed once every renderer returns true for HasEnded().  This has a nice benefit of being able to ignore extra NotifyEnded() calls if we already determine the pipeline has ended.

With the changes to WebMediaPlayerImpl, we should now properly support both the ended event and looping.

BUG=16768,17970,18433,18846
TEST=media_unittests, media layout tests, ended event, timeupdate should stop firing, looping should work, seeking after video ends

Review URL: http://codereview.chromium.org/164403
------------------------------------------------------------------------

 Issue 19271  has been merged into this issue.

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=23336 

------------------------------------------------------------------------
r23336 | laforge@chromium.org | 2009-08-13 11:49:43 -0700 (Thu, 13 Aug 2009) | 16 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/base/filter_host.h?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/base/filters.h?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/base/mock_filter_host.h?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/base/mock_filters.h?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/base/pipeline_impl.cc?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/base/pipeline_impl.h?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/base/pipeline_impl_unittest.cc?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/base/video_frame_impl.cc?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/base/video_frame_impl.h?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/base/video_frame_impl_unittest.cc?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/filters/audio_renderer_base.cc?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/filters/audio_renderer_base.h?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/filters/audio_renderer_base_unittest.cc?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/filters/video_renderer_base.cc?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/filters/video_renderer_base.h?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/media/filters/video_renderer_base_unittest.cc?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/webkit/glue/webmediaplayer_impl.cc?r1=23336&r2=23335
   M http://src.chromium.org/viewvc/chrome/branches/195/src/webkit/glue/webmediaplayer_impl.h?r1=23336&r2=23335

Merge 23255 - Implemented endofstream callback for media::PipelineImpl.

A new method HasEnded() was added to renderer interfaces.  Renderers return true when they have both received and rendered an endofstream buffer.  For audio this translates to sending the very last buffer to the hardware.  For video this translates to displaying a black frame after the very last frame has been displayed.

Renderers can notify the pipeline that the value of HasEnded() has changed to true via FilterHost::NotifyEnded().  Instead of tracking which renderers have called NotifyEnded(), the pipeline uses the notification to poll every renderer.  The ended callback will only be executed once every renderer returns true for HasEnded().  This has a nice benefit of being able to ignore extra NotifyEnded() calls if we already determine the pipeline has ended.

With the changes to WebMediaPlayerImpl, we should now properly support both the ended event and looping.

BUG=16768,17970,18433,18846
TEST=media_unittests, media layout tests, ended event, timeupdate should stop firing, looping should work, seeking after video ends

Review URL: http://codereview.chromium.org/164403

TBR=scherkus@chromium.org

Review URL: http://codereview.chromium.org/165472
------------------------------------------------------------------------

hello. I'm testing the new win32 (windows XP) build of Chrome (4.0.202.0) and the 
ended event appears to be firing sometimes, sometimes not. In a nutshell, I have an 
array of ids, which are used to locate track locations in a db. On the ended event, 
the next id in the array is used and the src in the audio object adjusted, firing off 
a play event. This seems to work fine for a max of about two songs or so, before the 
old issue happens and the scrubber stops at the end of the stream. 

I can show this happening in the app I've built (which works elsewhere, Firefox and 
Safari), but I can't make the app location public, for various legal and practical 
reasons. I'm happy for a dev to use it as an example of the issue, so long as it 
remains out of the public gaze. 
 

Thanks for the report.  Where you using playback rate?  It turns out we did have an 
issue where ended might not be fired when playback rate != 1.0f

I don't touch the playback rate, certainly I have nothing in the script using that. 
However, I do fiddle with the volume.

by default, the script sets the volume to 0.7. I noticed that at the end of each 
audio stream, despite me setting the volume manually to something else (lower|higher) 
the volume would reset itself to 1.0 (I think, or 0.7, definitely it kept putting the 
volume back up when I turned it down), however that was something I wanted to look at 
later, could have been my code.

I've also been testing Chrome dev 4.0.202.2 in Ubuntu Linux, and I can happily say it 
works a charm. I've been leaving it playing through extended playlists for hours at a 
time and it's handled it all. Many, many thanks.

Strange that it works for you on Ubuntu. I'm using Chromium daily build (currently
23955) on Ubuntu, and it also doesn't fire the ended event, just as you described in
comment #22.

I fib, I'm getting exactly the same error with ended as in comment #22. I _thought_ 
it was working but, for a reason I don't quite understand yet, one method of giving 
the audio object things to play is fine, the other isn't. I haven't quite figured out 
a reliable error with this yet. It could be my code, but the same code works fine in 
Firefox and Safari, so I'm inclined to think this is still a Chromium issue. I'm man 
enough to admit my code could be wrong however...

I'll see if I can pinpoint why one approach works and the other doesn't and hopefully 
it might offer some insight into why ended is still problematic.

If you could provide a tiny snippet of code that'd be great.  I'll try reproducing
this on my side in the meantime.

Re-opening and bumping priority as I'd like to get this fixed ASAP.

I'm trying to pinpoint the problem. I couldn't reproduce the problem on a small test 
page, but I'll search further tonight and the following days.

I've got it.

The first song will always fire the ended event. The second will only fire the event if it's 
'currentPosition' is not changed by javascript (i.e., the user hasn't seeked). The first song 
can be seeked, though.

A test site can be found at http://www.xs4all.nl/~pineau/test_audio/

I'm sorry for the long songs, but this is the only free music I've gotten.

PS. The length of the second song is 236 seconds. It is not properly updated as is described in 
 issue 20152 .

This is great!  I've got it reproducing and I'm debugging it right now.

Fixes are in and tested as working on Windows and Linux.  Any nightly building r24331 
should have the fix.

Thanks again for the report!!

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=24251 

------------------------------------------------------------------------
r24251 | scherkus@chromium.org | 2009-08-25 11:08:16 -0700 (Tue, 25 Aug 2009) | 9 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/webmediaplayer_impl.cc?r1=24251&r2=24250

Fixes bug where changing the src of an audio/video element would stop firing events.

HTMLMediaElement keeps track of network/ready state separately.  When re-using an element by setting the src and calling load(), HTMLMediaElement's network/ready states became out of sync with the actual values inside WebMediaPlayerImpl.  This led to the second media never firing 'loadedmetadata' and other events.

BUG=16768,20152
TEST=change src of an audio/video, call load(), should see durationchange and loadedmetadata events

Review URL: http://codereview.chromium.org/174385

------------------------------------------------------------------------

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=24331 

------------------------------------------------------------------------
r24331 | scherkus@chromium.org | 2009-08-25 14:27:56 -0700 (Tue, 25 Aug 2009) | 9 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/webmediaplayer_impl.cc?r1=24331&r2=24330

Fix for audio/video not firing ended if you change src and call load().

Another instance of HTMLMediaElement becoming out of sync with WebMediaPlayerImpl.  Most of these bugs are a result of HTMLMediaElement re-creating WebMediaPlayerImpl whenever src changes but not completely resetting internal state, causing inconsistent behaviour.

BUG=16768,20152
TEST=change src and call load(), wait until the end of the clip -- ended should fire

Review URL: http://codereview.chromium.org/173388

------------------------------------------------------------------------

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=24401 

------------------------------------------------------------------------
r24401 | laforge@chromium.org | 2009-08-25 18:29:16 -0700 (Tue, 25 Aug 2009) | 13 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/195/src/webkit/glue/webmediaplayer_impl.cc?r1=24401&r2=24400

Merge 24251 - Fixes bug where changing the src of an audio/video element would stop firing events.

HTMLMediaElement keeps track of network/ready state separately.  When reusing an element by setting the src and calling load(), HTMLMediaElement's network/ready states became out of sync with the actual values inside WebMediaPlayerImpl.  This led to the second media never firing 'loadedmetadata' and other events.

BUG=16768,20152
TEST=change src of an audio/video, call load(), should see durationchange and loadedmetadata events

Review URL: http://codereview.chromium.org/174385


TBR=scherkus@chromium.org

Review URL: http://codereview.chromium.org/173447
------------------------------------------------------------------------

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=24403 

------------------------------------------------------------------------
r24403 | laforge@chromium.org | 2009-08-25 18:31:19 -0700 (Tue, 25 Aug 2009) | 13 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/195/src/webkit/glue/webmediaplayer_impl.cc?r1=24403&r2=24402

Merge 24331 - Fix for audio/video not firing ended if you change src and call load().

Another instance of HTMLMediaElement becoming out of sync with WebMediaPlayerImpl.  Most of these bugs are a result of HTMLMediaElement recreating WebMediaPlayerImpl whenever src changes but not completely resetting internal state, causing inconsistent behaviour.

BUG=16768,20152
TEST=change src and call load(), wait until the end of the clip  ended should fire

Review URL: http://codereview.chromium.org/173388


TBR=scherkus@chromium.org

Review URL: http://codereview.chromium.org/173449
------------------------------------------------------------------------

Sorry, I'm still having issues playing more than two audio files in a row, second 
audio seems to stop a second before duration. Perhaps what I'm experiencing isn't 
related to this bug, but something else, so I'll try and get that test written to 
figure out what's happening. 

Can you confirm that the issue you describe does not happen on
http://www.xs4all.nl/~pineau/test_audio/. If you do experience the issue, can you
describe what actions to take to reproduce it, as it does not occur at my machine.

hello, yes, this issue happens on your test also. Track 2, the NIN song (sounds like, 
anyway) gets to the end, then stops. The output on the screen says,

"Currently playing: 2. 1,000,000

Played: 235.4388885498047 seconds
Length: 85.56024932861328 seconds
Emptied: 1 times"

I've noticed I might have been perhaps a bit daft, I assumed this fix made it to the 
latest dev release (4.0.203.2) but perhaps it hasn't, which would explain a lot. I'm 
gonna kick myself if that's the case...

I'm using version 4.0.204.0 (Devoloper Build Ubuntu build 24734), provided by the
chromium daily ubuntu ppa (https://launchpad.net/~chromium-daily/+archive/ppa). I
don't know which build is 4.0.203.2, but I guess it is lower than 24403.

Oh, and don't kick yourself to hard. The more people on the lookout for bugs in the
<audio> element, the better :p

I have just installed chrome on a windows installation and I also get version
2.0.203.2 (build 24690). I can also confirm that the issue is still present in that
version. It seems to be an older version than the Ubuntu daily, though, as it has V8
version 1.3.8, while the daily build has version 1.3.8.1.

Still strange though that the issue is present, as the build is higher that 24403.

Thanks for the updates!  2.0.203.2 was branched at r24234 and wouldn't contain the 
fixes.  The 24690 was the revision of a quick fix to update V8 to 1.3.8:
http://src.chromium.org/viewvc/chrome?view=rev&revision=24690

This week's dev channel should contain the fixes.

hugwi...: could you confirm whether the ubuntu build works?  I've tested with a local 
build, but I don't run the ubuntu daily builds

I can confirm that this issue is solved in the Ubuntu builds. Thanks for the fix!

Sweet!  Thanks for the checking :)

the new word on the street is that as of the latest dev-channel update, this works a 
charm on Ubuntu linux, and I now have a working web based jukebox. I'll test WinXP 
when I'm next in the office. Sweet! Scherkus, if you're in west London (UK), allow me 
to buy you a coffee, or stronger. Many thanks also to hugwi... for writing the test. 

Awesome!

Afraid I'm nowhere near London... but fortunately being in Seattle there's plenty of 
coffee :D

(No comment was entered for this change.)

(No comment was entered for this change.)

(No comment was entered for this change.)

(No comment was entered for this change.)

This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.

(No comment was entered for this change.)

(No comment was entered for this change.)

(No comment was entered for this change.)