Bug 237473

Summary: MediaTime::invalidTime() isn't convertible without loss into CMTime
Product: WebKit Reporter: Jean-Yves Avenard [:jya] <jean-yves.avenard>
Component: MediaAssignee: Jean-Yves Avenard [:jya] <jean-yves.avenard>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 237472    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch none

Description Jean-Yves Avenard [:jya] 2022-03-04 07:49:17 PST
Consider the following code:
```
        auto invalid = MediaTime::invalidTime();
        auto cminvalid = PAL::toCMTime(invalid);
        auto invalid2 = PAL::toMediaTime(cminvalid);
        ASSERT(invalid == invalid2);
```

the assertion will fail.

The reason is that PAL::toCMTime(MediaTime::invalidTime()) will generate a CMTime that has the flag kCMTimeFlags_HasBeenRounded set.

This causes failure here 
https://webkit-search.igalia.com/webkit/rev/ceb5e25a803df6cd6ea1c45859e4e03bbf659f75/Source/WebKit/Shared/mac/MediaFormatReader/MediaSampleByteRange.cpp#49
Comment 1 Jean-Yves Avenard [:jya] 2022-03-04 07:56:59 PST
Actually, earlier description is incorrect:

the code of toCMTime is as follow:
```
    CMTime time;

    if (mediaTime.hasDoubleValue())
        time = CMTimeMakeWithSeconds(mediaTime.toDouble(), mediaTime.timeScale());
    else
        time = CMTimeMake(mediaTime.timeValue(), mediaTime.timeScale());

    if (mediaTime.isValid())
        time.flags |= kCMTimeFlags_Valid;
    if (mediaTime.hasBeenRounded())
        time.flags |= kCMTimeFlags_HasBeenRounded;
    if (mediaTime.isPositiveInfinite())
        time.flags |= kCMTimeFlags_PositiveInfinity;
    if (mediaTime.isNegativeInfinite())
        time.flags |= kCMTimeFlags_NegativeInfinity;

    return time;
```

what converting MediaTime::invalidTime() : `CMTimeMake(mediaTime.timeValue(), mediaTime.timeScale());` will return a valid time and that flag isn't reset.
Comment 2 Radar WebKit Bug Importer 2022-03-04 08:23:52 PST
<rdar://problem/89814921>
Comment 3 Jean-Yves Avenard [:jya] 2022-03-04 08:37:13 PST
Created attachment 453845 [details]
Patch
Comment 4 Jean-Yves Avenard [:jya] 2022-03-04 19:31:35 PST
Created attachment 453886 [details]
Patch
Comment 5 Jean-Yves Avenard [:jya] 2022-03-04 19:33:03 PST
Created attachment 453887 [details]
Patch
Comment 6 Jean-Yves Avenard [:jya] 2022-03-04 19:47:20 PST
Created attachment 453888 [details]
Patch

Fix iOS compilation
Comment 7 Jer Noble 2022-03-07 13:53:18 PST
Comment on attachment 453888 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=453888&action=review

> Source/WebCore/PAL/ChangeLog:15
> +2022-03-04  Jean-Yves Avenard  <jya@apple.com>

Nit: This ChangeLog entry should get merged with the one above it.
Comment 8 Jean-Yves Avenard [:jya] 2022-03-07 13:58:23 PST
Created attachment 454028 [details]
Patch
Comment 9 Jean-Yves Avenard [:jya] 2022-03-07 14:00:27 PST
Created attachment 454031 [details]
Patch
Comment 10 EWS 2022-03-07 16:01:42 PST
Committed r290964 (248144@main): <https://commits.webkit.org/248144@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454031 [details].