Bug 190893 – [MSE][WTF][Media] Invalid MediaTime should be falsy

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alicia Boya García]

Currently MediaTime::invalidTime() is evaluated as true in the context of a boolean evaluation. This behavior is surprising and I've already seen some code fall on it. In particular, in SourceBuffer:

} else if (trackBuffer.roundedTimestampOffset) {
    // Reflect the timestamp offset into the sample.
    sample.offsetTimestampsBy(trackBuffer.roundedTimestampOffset);
}

There, the intention would be to offset the timestamps of the sample if and only if a valid non-zero timestamp offset exists. But with the current definition of MediaTime::operator bool, an invalid time is true. I definitively did not expect that, did you?

This patch includes:

1. A mock MSE test case replicating a bug caused by that assumption.

2. A change in the definition of MediaTime::operator bool() and MediaTime::operator!() so that invalid times are falsy.

3. Additional API tests for conversions of MediaTime to bool.

[Alicia Boya García]

Created attachment 353066 [details]
Patch

[Jer Noble]

Comment on attachment 353066 [details]
Patch

Originally, I decided not to make isInvalid() false because I thought it would be weird to have !isInvalid() == !zero.  But I see that floating point NaN is also falsy, so this LGTM.

[WebKit Commit Bot]

Comment on attachment 353066 [details]
Patch

Clearing flags on attachment: 353066

Committed r237450: <https://trac.webkit.org/changeset/237450>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.