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.
<rdar://problem/89960307>
Created attachment 454096 [details] Patch for review
Created attachment 454098 [details] Patch for EWS
Created attachment 454106 [details] Patch for review
Created attachment 454107 [details] Patch for review
Created attachment 454108 [details] Patch for EWS
Created attachment 454173 [details] Patch Simplify flags handling
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.
Created attachment 454193 [details] Patch apply comments
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].