REOPENED 254222
[mac][ios][win] CodecDelay webm field isn't being honored
https://bugs.webkit.org/show_bug.cgi?id=254222
Summary [mac][ios][win] CodecDelay webm field isn't being honored
Enrique Ocaña
Reported 2023-03-21 11:16:38 PDT
While debugging https://bugs.webkit.org/show_bug.cgi?id=228820 and, more specifically, the failure in media/media-source/media-webm-opus-partial-abort.html for glib (GStreamer) platforms, I found out that the test results in that platform were actually correct, and the incorrection was in the expectations (it the test results for the rest of the platforms). The test-opus.webm[1] test file used for that test contains a CodecDelay[2] field in its Track element with value 0x632ea0 = 6500000ns = 0.065s. This can be seen in the tree structure dump produced by the fq[3] binary analysis tool: $ fq d LayoutTests/media/media-source/content/test-opus.webm |00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21|0123456789abcdef0123456789abcdef01|.{}: LayoutTests/media/media-source/content/test-opus-codecdelay.webm (matroska) | | | elements[0:2]: | | | [0]{}: 0x00000|1a 45 df a3 |.E.. | id: "EBML" (0x1a45dfa3) | | | type: "master" (7) 0x00000| 9f | . | size: 31 | | | elements[0:7]: | | | [0]{}: 0x00000| 42 86 | B. | id: "EBMLVersion" (0x4286) | | | type: "uinteger" (1) 0x00000| 81 | . | size: 1 0x00000| 01 | . | value: 1 ... | | | [3]{}: 0x000aa| 16 54 ae 6b | .T.k | id: "Tracks" (0x1654ae6b) (A Top-Level Element of information with many tracks described.) | | | type: "master" (7) 0x000aa| c8 | . | size: 72 | | | elements[0:1]: | | | [0]{}: 0x000aa| ae | . | id: "TrackEntry" (0xae) (Describes a track with all Elements.) | | | type: "master" (7) 0x000aa| c6 | . | size: 70 | | | elements[0:8]: | | | [0]{}: 0x000aa| d7 | . | id: "TrackNumber" (0xd7) (The track number as used in the Block Header (using more than 127 tracks is not encouraged, though the design allows an unlimited number).) | | | type: "uinteger" (1) 0x000aa| 81| .| size: 1 0x000cc|01 |. | value: 1 | | | [1]{}: 0x000cc| 73 c5 | s. | id: "TrackUID" (0x73c5) (A unique ID to identify the Track. This SHOULD be kept the same when making a direct stream copy of the Track to another file.) | | | type: "uinteger" (1) 0x000cc| 87 | . | size: 7 0x000cc| 5c b8 df 0a de a9 72 | \.....r | value: 26098965956962674 | | | [2]{}: 0x000cc| 83 | . | id: "TrackType" (0x83) (A set of track types coded on 8 bits.) | | | type: "uinteger" (1) 0x000cc| 81 | . | size: 1 0x000cc| 02 | . | value: "audio" (2) | | | [3]{}: 0x000cc| 56 aa | V. | id: "CodecDelay" (0x56aa) (CodecDelay is The codec-built-in delay in nanoseconds. This value MUST be subtracted from each block timestamp in order to get the actual timestamp. The value SHOULD be small so the muxing of tracks with the same actual timestamp are in the same Cluster.) | | | type: "uinteger" (1) 0x000cc| 83 | . | size: 3 0x000cc| 63 2e a0 | c.. | value: 6500000 ... The test results in glib platforms were showing exactly a 0.065s difference compared to the expectations, which made me suspect about CodecDelay not being applied in other platforms (including Apple ones and Chrome): EXPECTED (sourceBuffer.buffered.length == '2') OK EXPECTED (sourceBuffer.buffered.end(0) == '6.041'), OBSERVED '6.0345' FAIL EXPECTED (sourceBuffer.buffered.end(1) == '82.981'), OBSERVED '82.9745' FAIL Actually, when I neutralized the code honoring CodecDelay in matroska-demux.c[4], I got the layout test "passing". I undid that change right after that. When I edited the webm file using hexedit and changed bytes 0x000cc11-0x000cc13 from 63 2e a0 to 00 00 00 to force a CodecDelay of 0, I also got the test "passing". This makes me think that the layout test should be "corrected" to have a CodecDelay of zero, and a new test should be created to raise awareness of this lack of CodecDelay support. [1] https://github.com/WebKit/WebKit/blob/main/LayoutTests/media/media-source/content/test-opus.webm [2] https://www.matroska.org/technical/elements.html [3] https://github.com/wader/fq [4] https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/gst-plugins-good/gst/matroska/matroska-demux.c#L4988
Attachments
Enrique Ocaña
Comment 1 2023-03-21 12:41:04 PDT
The file structure dump corresponds to test-opus.webm from before https://commits.webkit.org/260822@main. I was going a submit a patch today changing test-opus.webm to a version with CodecDelay=0, so that media/media-source/media-webm-opus-partial-abort.html behaves the same in all the platforms, as well as an additional test to only highlight the differences on CodecDelay processing, but this was using the old file. I have to rework it to use the new test-opus.webm as base before submitting.
Enrique Ocaña
Comment 2 2023-03-22 07:29:47 PDT
EWS
Comment 4 2023-03-27 03:19:41 PDT
Committed 262143@main (e9c9b4fd0c9c): <https://commits.webkit.org/262143@main> Reviewed commits have been landed. Closing PR #11807 and removing active labels.
Radar WebKit Bug Importer
Comment 5 2023-03-27 03:20:15 PDT
Enrique Ocaña
Comment 6 2023-03-27 04:51:39 PDT
Reopening so that the affected ports are aware, now that the test that triggers the bug and its corresponding expectation have been added.
Note You need to log in before you can comment on or make changes to this bug.