Bug 220555 – [Mac] MediaSampleCursor::stepInOrderedMap can hang when stepping to the end of a track that hasn't been fully parsed

• Component: New Bugs
• Status: RESOLVED FIXED

[Andy Estes]

[Mac] MediaSampleCursor::stepInOrderedMap can hang when stepping to the end of a track that hasn't been fully parsed

[Andy Estes]

Created attachment 417474 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/73366041>

[Eric Carlson]

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

[Andy Estes]

Created attachment 418445 [details]
Patch

[EWS]

Committed r271939: <https://trac.webkit.org/changeset/271939>

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