Bug 222260

Summary: [MSE] Media segment is incorrectly dropped when using negative timestampOffset
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: calvaris, cgarcia, eric.carlson, ews-watchlist, glenn, gustavo, jean-yves.avenard, jer.noble, menard, philipj, pnormand, sergio, vjaquez, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://jyavenard.github.io/htmltests/tests/mse_mp4/test_audiooffset.html
See Also: https://bugs.webkit.org/show_bug.cgi?id=224067
Attachments:
Description Flags
Patch
none
Patch
eric.carlson: review+
Patch none

Jean-Yves Avenard [:jya]
Reported 2021-02-21 20:33:16 PST
Here is a small test: https://jyavenard.github.io/htmltests/tests/mse_mp4/test_audiooffset.html This adds roughly 1s of audio (975ms), the media segment is made of 21 samples ranging from [0.9752380952380952, 1.9504761904761905); each sample is 46.44ms long. When adding them to an audio source buffer with timestampOffset set to -1; you would expect to see [0, 0.9504761904761905] (as both Firefox and Chrome do) now being buffered, but you get instead: [0.4860770975056689, 0.9504761904761905) Per spec: https://w3c.github.io/media-source/#sourcebuffer-coded-frame-processing "8. If presentation timestamp is less than appendWindowStart, then set the need random access point flag to true, drop the coded frame, and jump to the top of the loop to start processing the next coded frame." "Some implementations MAY choose to collect coded frames with presentation timestamp less than appendWindowEnd and frame end timestamp greater than appendWindowEnd and use them to generate a splice across the portion of the collected coded frames within the append window at time of collection, and the beginning portion of later processed frames which only partially overlap the end of the collected coded frames. Supporting this requires multiple decoders or faster than real-time decoding so for now this behavior will not be a normative requirement. In conjunction with collecting coded frames that span appendWindowStart, implementations MAY thus support gapless audio splicing. " Now, even if audio splicing isn't supported; the only audio frame that should be dropped per the description above is the one with time [-0.024762,0.021678], but webkit drops 10 samples from 0.021678s to 0.486077s that shouldn't be.
Attachments
Patch (15.08 KB, patch)
2021-02-22 23:09 PST, Jean-Yves Avenard [:jya]
no flags
Patch (22.95 KB, patch)
2021-02-23 04:17 PST, Jean-Yves Avenard [:jya]
eric.carlson: review+
Patch (22.98 KB, patch)
2021-02-24 16:02 PST, Jean-Yves Avenard [:jya]
no flags
Jean-Yves Avenard [:jya]
Comment 1 2021-02-21 23:08:20 PST
SourceBufferPrivateAVFObjC seems to pack the samples in block of 10 samples. So the first block of 10 samples is dropped.
Jean-Yves Avenard [:jya]
Comment 2 2021-02-22 23:09:06 PST
Jean-Yves Avenard [:jya]
Comment 3 2021-02-23 04:17:29 PST
Eric Carlson
Comment 4 2021-02-24 08:26:24 PST
Comment on attachment 421300 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=421300&action=review > Source/WebCore/ChangeLog:11 > + CoreMedia packs multiple audio frames together into a single CMSampleBuffer, this allows for faster processing and easier insertion into the track buffer tree. > + However, per mediasoure spec [1], a frame is to be dropped according to its start time and duration. So if only the beginning of the MediaSample was to be dropped, we would have incorrectly dropped the lot. Since there is a typo to fix, please wrap these overly long lines people won't need to scroll to read the ChangeLog. > Source/WebCore/ChangeLog:13 > + Audio splicing isn't done yet, but this gets us closeer to it. s/closeer/closer/
Jean-Yves Avenard [:jya]
Comment 5 2021-02-24 16:02:05 PST
EWS
Comment 6 2021-02-24 16:57:59 PST
Committed r273461: <https://commits.webkit.org/r273461> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421477 [details].
Radar WebKit Bug Importer
Comment 7 2021-02-24 16:59:21 PST
Note You need to log in before you can comment on or make changes to this bug.