Closed
Bug 1064113
Opened 10 years ago
Closed 10 years ago
MediaSource::EndOfStream occurring during seek causes assertion
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: cajbir, Assigned: cajbir)
References
(Blocks 1 open bug)
Details
Attachments
(4 obsolete files)
If we are in this loop in MediaSourceReader::Seek:
while (!TrackBuffersContainTime(target) && !IsShutdown() && !IsEnded()) {
MSE_DEBUG("MediaSourceReader(%p)::Seek waiting for target=%f", this, target);
static_cast<MediaSourceDecoder*>(mDecoder)->WaitForData();
}
And MediaSource::EndOfStream happens such that IsEnded() returns true, we hit the following assertion laster in the seek call:
MOZ_ASSERT(ok && static_cast<SourceBufferDecoder*>(mAudioReader->GetDecoder())->ContainsTime(target));
Because the loop exited due to IsEnded() we do not necessarily have the target time in the reader.
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → cajbir.bugzilla
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
When a seek is in progress and MediaSource::EndOfStream() is called the behaviour should be to keep the seek request in progress. If data is appended later that is for the range the seek is waiting then the seek resumes.
Assignee | ||
Comment 3•10 years ago
|
||
When a seek is waiting for data and new data is appended the notiftifcation of this data occurs before the decoder is registered as an initialized decoder in the track buffer. As a result the seek continues waiting for data as it never gets a further notification.
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
Bug 1064160 has the same effective fix for comment 3 so the fix here will need to be removed in the attached patch.
Comment 6•10 years ago
|
||
Ah sorry, didn't see you had made similar changes to the wait/notify stuff.
(In reply to cajbir (:cajbir) from comment #2)
> When a seek is in progress and MediaSource::EndOfStream() is called the
> behaviour should be to keep the seek request in progress. If data is
> appended later that is for the range the seek is waiting then the seek
> resumes.
If I understand correctly, we can't really ever use IsEnded() because it's always possible to reopen the MediaSource by appending new data. That may be problematic for the use in On{Audio,Video}EOS too.
There's a bit of other work necessary to support reopening MediaSource objects too, so a complete fix will require modifications to other areas of the code.
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Matthew Gregan [:kinetik] from comment #6)
>
> If I understand correctly, we can't really ever use IsEnded() because it's
> always possible to reopen the MediaSource by appending new data. That may
> be problematic for the use in On{Audio,Video}EOS too.
>
> There's a bit of other work necessary to support reopening MediaSource
> objects too, so a complete fix will require modifications to other areas of
> the code.
Right, that's what I'm currently doing. Just returning the IsEnded check in the seek loop prevents the assertion but there's other IsEnded usage as you note.
Assignee | ||
Comment 8•10 years ago
|
||
Adds a test for the simple case of an appendBuffer after an endOfStream. This triggers bug 1065221 but the test still passes beyond that as noted in that bug report.
Assignee | ||
Comment 9•10 years ago
|
||
This refactors the previous test case to work with recent test changes. An endOfStream() occurs during a seek.
This currently triggers the assertion in this bug.
It should result in the seek continuing and playing through to the end of the file.
With a fix for the asssertion that test case still fails as the playback does not resume after the seek.
Attachment #8485560 -
Attachment is obsolete: true
Assignee | ||
Comment 10•10 years ago
|
||
Bug 1065218 removes the assertions that are triggered by testcase in attachment 8486963 [details] [diff] [review]. The remaining errors occur as outlined in comment 9. I'll rename this bug if that one lands.
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8486937 [details] [diff] [review]
p1: Testing appendData after an endOfStream
Spinning this test case out into bug 1068483 as it now fails.
Attachment #8486937 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8485583 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8486963 -
Attachment is obsolete: true
Assignee | ||
Comment 12•10 years ago
|
||
Assertion was fixed in other patch landing/refactoring. Dealing with the problem mentioned in comment 6 is moved to bug 1068483.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•