Bug 237594 – Have MediaFormatReader plugin use WebMParser directly

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jean-Yves Avenard [:jya]]

Sub-task of bug 236754.

Currently, the MediaFormatReader uses a SourceBufferParserWebM to demux the webm file and build a samples table.
This unnecessarily creates MediaSampleAVFObjC object which will be immediately deleted after the needed information has been read (presentation time, decode time, flags and byte range).

This is inefficient and requires to read the entire webm file into memory before we can play it.

By using the WebM parser introduced in bug 236754, we can only export the required ByteRange and build the sample table from that without needed an intermediary MediaSampleAVFObjC.

[Radar WebKit Bug Importer]

<rdar://problem/89960307>

[Jean-Yves Avenard [:jya]]

Created attachment 454096 [details]
Patch for review

[Jean-Yves Avenard [:jya]]

Created attachment 454098 [details]
Patch for EWS

[Jean-Yves Avenard [:jya]]

Created attachment 454106 [details]
Patch for review

[Jean-Yves Avenard [:jya]]

Created attachment 454107 [details]
Patch for review

[Jean-Yves Avenard [:jya]]

Created attachment 454108 [details]
Patch for EWS

[Jean-Yves Avenard [:jya]]

Created attachment 454173 [details]
Patch

Simplify flags handling

[Eric Carlson]

Comment on attachment 454173 [details]
Patch

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

> Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp:1097
> +    if (m_useByteRange)
> +        m_completeMediaSamples.append({ presentationTime, presentationTime, MediaTime(duration, presentationTime.timeScale()), MediaSample::ByteRange { metadata.position, metadata.size }, isKey ? MediaSample::SampleFlags::IsSync : MediaSample::SampleFlags::None });
> +    else
> +        m_completeMediaSamples.append({ presentationTime, presentationTime, MediaTime(duration, presentationTime.timeScale()), m_completeBlockBuffer.releaseNonNull(), isKey ? MediaSample::SampleFlags::IsSync : MediaSample::SampleFlags::None });

This would be easier to read, and less error prone if/when it is edited, if you rewrote it to set a local variable based on `m_useByteRange` and passed that to `m_completeMediaSamples.append({...`. 

Is that possible?

> Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp:1188
> +    if (m_useByteRange)
> +        m_completeMediaSamples.append({ presentationTime, MediaTime::invalidTime(), m_packetDuration, MediaSample::ByteRange { metadata.position, metadata.size }, MediaSample::SampleFlags::IsSync });
> +    else
> +        m_completeMediaSamples.append({ presentationTime, MediaTime::invalidTime(), m_packetDuration, m_completeBlockBuffer.releaseNonNull(), MediaSample::SampleFlags::IsSync });

Ditto.

> Source/WebKit/Shared/mac/MediaFormatReader/MediaFormatReader.cpp:148
> +    // TODO: why do we need a storage queue different to reader's queue?

"FIXME:" is the more idiomatic way to leave a note for future WebKit work.

[Jean-Yves Avenard [:jya]]

Created attachment 454193 [details]
Patch

apply comments

[EWS]

Committed r291033 (248207@main): <https://commits.webkit.org/248207@main>

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