634747 - ASSERTION: Seek target should lie inside the first audio block after seek: '!audio || (audio->mTime <= seekTime && seekTime <= audio->mTime + audio->mDuration)'

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Mats Palmgren (inactive)]

STEPS TO REPRODUCE
1. load the URL
2. while the video is playing: drag the thumb back and forth and
   click on the progress bar repeatedly, as much as possible

ACTUAL RESULTS
The following three assertions are easily reproducible, the first one
being the most frequent.

###!!! ASSERTION: Seek target should lie inside the first audio block after seek: '!audio || (audio->mTime <= seekTime && seekTime <= audio->mTime + audio->mDuration)', file content/media/nsBuiltinDecoderStateMachine.cpp, line 1131
###!!! ASSERTION: Start time must be before end time: 'startTime < endTime', file content/media/nsBuiltinDecoderReader.cpp, line 264
###!!! ASSERTION: Seek target should lie inside the first frame after seek: 'video->mTime <= seekTime && seekTime <= video->mEndTime', file content/media/nsBuiltinDecoderStateMachine.cpp, line 1139

PLATFORMS AND BUILDS TESTED
Local Firefox debug build mozilla-central (rev 52246c1b1799) on Linux x86-64

Comment 1

[Mats Palmgren (inactive)]

Updated my local build to see if bug 634787 fixed it.  It seems the 2nd
assertion above is now fixed.  But I can still reproduce the other two,
and a new one:

###!!! ASSERTION: Seek target should lie inside the first frame after seek: 'video->mTime <= seekTime && seekTime <= video->mEndTime', file content/media/nsBuiltinDecoderStateMachine.cpp, line 1109
###!!! ASSERTION: Seek target should lie inside the first audio block after seek: '!audio || (audio->mTime <= seekTime && seekTime <= audio->mTime + audio->mDuration)', file content/media/nsBuiltinDecoderStateMachine.cpp, line 1101
###!!! ASSERTION: Page read cursor should be inside range: 'mPageOffset <= endOffset', file content/media/ogg/nsOggReader.cpp, line 1510
WARNING: Hit audio event queue max.: file content/media/nsAudioAvailableEventManager.cpp, line 169

Comment 2

[Mats Palmgren (inactive)]

Additionally, quite often when I click on the progress bar nothing happens,
the video just continues to play where it is.  I expect it to start playing
where I clicked.  This worked fine when I reported this bug so the new
behavior appears to be a regression. (This is on x86-64 Linux)

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to comment #2)
> Additionally, quite often when I click on the progress bar nothing happens,
> the video just continues to play where it is.

Are you clicking close to the current playback position label? You could be seeing bug 488082.

Comment 4

[Mats Palmgren (inactive)]

Yes, it looks like it's bug 488082, I probably just didn't notice it earlier.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to comment #0)
> ###!!! ASSERTION: Seek target should lie inside the first audio block after
> seek: '!audio || (audio->mTime <= seekTime && seekTime <= audio->mTime +
> audio->mDuration)', file content/media/nsBuiltinDecoderStateMachine.cpp, line
> 1131

I caught this in a debugger. It looks like this is caused by rounding off when we convert samples numbers to ms. We end up timestamping a SoundData with mTime=n mDuration=d, and the next SoundData gets mTime=n+d+1 (it should be mTime=n+d), and the assertion fires when the seek target is n+d and when DecodeToTarget() finishes with the mTime=n+d+1 SoundData at the front of the audio queue.

Perhaps we should store SoundData start times as sample offsets, not ms, and when comparing SoundData sample-offsets with a ms value, convert the ms to samples and compare them as samples?

I haven't investigated the other assertions Mats reported yet.

Comment 6

[Paul Adenot (:padenot)]

Attachment: Expect some assertions on windows on test_seek.html while we investigate.

I've removed the assertion expectations from test_seek.html in bug 845676, but
this assertions failed on Windows, so it went orange:

  ASSERTION: Seek target should lie inside the first frame after seek: 'video->mTime <= seekTime && seekTime <= video->mEndTime'

This assertions seems to occur only on Windows. This patch makes test go green
tbpl for now, but I'll keep in on my list of assertion failures to investigate.

Comment 7

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/396454d30d27

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/396454d30d27

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

Another one bites the dust.
https://tbpl.mozilla.org/?tree=Try&rev=b47218e78209

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/63efb2b87c92

Comment 12

[Mats Palmgren (inactive)]

Please don't needinfo me without a specific question.

If you just want to test Bugzilla features, use landfill instead:
https://landfill.bugzilla.org