WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
180258
[MSE] Add isValid() check before using trackBuffer.lastEnqueuedPresentationTime
https://bugs.webkit.org/show_bug.cgi?id=180258
Summary
[MSE] Add isValid() check before using trackBuffer.lastEnqueuedPresentationTime
Alicia Boya García
Reported
2017-12-01 10:14:40 PST
The current comparison in sourceBufferPrivateDidReceiveSample() does not take in account the possibility of lastEnqueuedPresentationTime not being set. MediaTime currentMediaTime = m_source->currentTime(); if (currentMediaTime < trackBuffer.lastEnqueuedPresentationTime) { PlatformTimeRanges possiblyEnqueuedRanges(currentMediaTime, trackBuffer.lastEnqueuedPresentationTime); possiblyEnqueuedRanges.intersectWith(erasedRanges); if (possiblyEnqueuedRanges.length()) trackBuffer.needsReenqueueing = true; } Is this intentional? When experimenting with a new frame removal algorithm I hit a case where it was unset and therefore it created an invalid range in possiblyEnqueuedRanges.
Attachments
Patch
(1.83 KB, patch)
2017-12-01 10:16 PST
,
Alicia Boya García
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Alicia Boya García
Comment 1
2017-12-01 10:16:41 PST
Created
attachment 328114
[details]
Patch
Jer Noble
Comment 2
2017-12-01 12:07:09 PST
Comment on
attachment 328114
[details]
Patch Is this change fix something, and if so, should it have a test?
Alicia Boya García
Comment 3
2017-12-04 04:26:33 PST
No.(In reply to Jer Noble from
comment #2
)
> Comment on
attachment 328114
[details]
> Patch > > Is this change fix something, and if so, should it have a test?
I'm not sure, that's why I'm asking if the assumption in the `if` above was intentional. As I see it, if is not intentional (i.e. it's possible that lastEnqueuedPresentationTime is MediaTime::invalidTime()) then we should check for that as in the patch. If it's intentional (i.e. it can never be invalid time), it's not obvious and should have an ASSERT() to make it more evident and avoid unexpected side effects should that ever cease being the case.
Alicia Boya García
Comment 4
2017-12-14 04:25:56 PST
Could we take a decision on this?
WebKit Commit Bot
Comment 5
2017-12-14 15:13:45 PST
Comment on
attachment 328114
[details]
Patch Clearing flags on attachment: 328114 Committed
r225936
: <
https://trac.webkit.org/changeset/225936
>
WebKit Commit Bot
Comment 6
2017-12-14 15:13:46 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7
2017-12-14 15:14:27 PST
<
rdar://problem/36059745
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug