Bug 220555

Summary: [Mac] MediaSampleCursor::stepInOrderedMap can hang when stepping to the end of a track that hasn't been fully parsed
Product: WebKit Reporter: Andy Estes <aestes>
Component: New BugsAssignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Andy Estes 2021-01-12 11:41:40 PST 
[Mac] MediaSampleCursor::stepInOrderedMap can hang when stepping to the end of a track that hasn't been fully parsed
Comment 1 Andy Estes 2021-01-12 11:42:28 PST 
Created attachment 417474 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2021-01-19 11:42:13 PST 
<rdar://problem/73366041>
Comment 3 Eric Carlson 2021-01-25 14:13:41 PST 
Comment on attachment 417474 [details]
Patch

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

Some of the ALWAYS_LOG should be changed to DEBUG_LOG.

r=me

> Source/WebCore/platform/Logging.h:81
> +    M(MediaFormatReader) \

Having a dedicated log channel has some benefits, but using Media would allow us to configure the log level from the WebInspector.

> Source/WebKit/Shared/mac/MediaFormatReader/MediaSampleCursor.cpp:54
> +String LogArgument<CFComparisonResult>::toString(CFComparisonResult comparisonResult)

It would be more efficient to return a `ASCIILiteral` here and below where we return literals.

> Source/WebKit/Shared/mac/MediaFormatReader/MediaSampleCursor.cpp:245
> +static MediaTime makeDuration(typename OrderedMap::iterator iterator, OrderedMap& samples, const MediaTime& trackDuration)

Nit: "makeDuration" implies, to me, that this will set the duration of something. Maybe `calculateDuration` instead?

> Source/WebKit/Shared/mac/MediaFormatReader/MediaSampleCursor.cpp:270
> +    PresentationTime presentationTime { time };

Is there a reason to create a local variable here?

> Source/WebKit/Shared/mac/MediaFormatReader/MediaSampleCursor.cpp:276
> +    DecodeTime decodeTime { std::make_pair(time, time) };

Ditto
Comment 4 Andy Estes 2021-01-26 10:40:03 PST 
Created attachment 418445 [details]
Patch
Comment 5 EWS 2021-01-27 01:07:41 PST 
Committed r271939: <https://trac.webkit.org/changeset/271939>

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