1831451 - Implement basic VideoDecoder on Linux

Status: RESOLVED FIXED

(Core :: Audio/Video: Web Codecs, task, P1)

Description

[C.M.Chang[:chunmin]]

This bug is a sub-task for bug 1749045. The task is to implement the basic VideoDecoder operations. The goal is to create an VideoDecoder that can pass the following wpts

• videoDecoder-codec-specific.https.any.js
• video-decoder.https.any.js
• video-decoder.crossOriginIsolated.https.any.js

On Linux platforms

Comment 1

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Add a blank EncodedVideoChunk interface

Comment 2

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Implement EncodedVideoChunk

Depends on D155894

Comment 3

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Add a blank VideoDecoder interface

Depends on D155892

Comment 4

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Add NS_WARN_IF to unlikely condition

Comment 5

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Implement VideoDecoder::IsConfigSupported

Comment 6

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Implement VideoDecoder::Constructor

Comment 7

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Implement VideoDecoder::Reset

Comment 8

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Implement VideoDecoder::Close

Comment 9

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Add a VideoColorSpace-to-ColorRange function

Comment 10

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Implement VideoDecoder::Configure

Comment 11

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Implement VideoDecoder::Decode

Comment 12

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Fire a dequeue event when data is being decoded

Comment 13

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Implement VideoDecoder::Flush

Comment 14

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Guess color space from VideoData

Comment 15

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Guess pixel format from VideoData

Comment 16

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Allow constructing VideoFrame without pixel format

Comment 17

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Enable VideoDecoder WPTs

Depends on D171518

Comment 18

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Unblock DecodeProcessor whenever possible

Depends on D171518

Comment 19

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Revise dequeue event wpt

let lastDequeueSize = Infinity;
decoder.ondequeue = () => {
  assert_greater_than(lastDequeueSize, 0, "Dequeue event after queue empty");
  assert_greater_than(lastDequeueSize, decoder.decodeQueueSize,
                      "Dequeue event without decreased queue size");
  lastDequeueSize = decoder.decodeQueueSize;
};

The above checkes doesn't really work since the VideoDecoder's
codec-work-queue runs in parallel with the VideoDecoder interface. The
dequeue events can arrive after the last dequeue size has been 0 if the
work queue decodes multiple samples while VideoDecoder is processing
just one dequeue event. Besides, decodeQueueSize can change between
the two decoder.decodeQueueSize calls since it can be mutated on both
VideoDecoder's owner thread and the codec-work-queue thread. As a
result, these two decodeQueueSize can be different, and the
lastDequeueSize is not guaranteed to be decreased one at a time per
event, even if the internal decodeQueueSize is decreased one at a time
(on codec-work-queue thread). The value can be decreased more than one
at a time.

The following example demonstrates one case when those checks can fail.

Suppose VideoDecoder D runs on thread T (main or worker) and
codec-work-queue runs on thread Q. In the test, by the time
VideoDecoder is configured successfully and decode request are sent,
the codec-work-queue is ready to decode the pending samples. Below
illustrates why the wpt checks in the begining don't work

Q) The underlying codec is configured, ready to decode sample #1.
Schedule a dequeue event. The current queue size is 9.
T) Get a dequeue event #1. Now D.decodeQueueSize is 9.
Q) Decode sample #1 succeeded, ready to decode sample #2. Schedule a
dequeue event. The current queue size is 8.
T) D.decode() for sample #1 succeeded. Get VideoFrame #1.
T) Get a dequeue event #2. Now D.decodeQueueSize is 8.
Q) Decode sample #2 succeeded, ready to decode sample #3. Schedule a
dequeue event. The current queue size is 7.
T) D.decode() for sample #2 succeeded. Get VideoFrame #2.
Q) Decode sample #3 succeeded, ready to decode sample #4. Schedule a
dequeue event. The current queue size is 6.
Q) Decode sample #4 succeeded, ready to decode sample #5. Schedule a
dequeue event. The current queue size is 5.
T) Get a dequeue event #3. Now D.decodeQueueSize is 5.
T) D.decode() for sample #3 succeeded. Get VideoFrame #3.
T) Get a dequeue event #4. Now D.decodeQueueSize is 5.
=> "Dequeue event without decreased queue size" fails here
Q) Decode sample #5 succeeded, ready to decode sample #6. Schedule a
dequeue event. The current queue size is 4.
Q) Decode sample #6 succeeded, ready to decode sample #7. Schedule a
dequeue event. The current queue size is 3.
Q) Decode sample #7 succeeded, ready to decode sample #8. Schedule a
dequeue event. The current queue size is 2.
T) D.decode() for sample #4 succeeded. Get VideoFrame #4.
T) Get a dequeue event #5. Now D.decodeQueueSize is 2.
T) D.decode() for sample #5 succeeded. Get VideoFrame #5.
T) Get a dequeue event #6. Now D.decodeQueueSize is 2.
=> "Dequeue event without decreased queue size" fails again here
Q) Decode sample #8 succeeded, ready to decode sample #9. Schedule a
dequeue event. The current queue size is 1.
T) D.decode() for sample #6 succeeded. Get VideoFrame #6.
Q) Decode sample #9 succeeded, ready to decode sample #10. Schedule a
dequeue event. The current queue size is 0.
T) Get a dequeue event #7. Now D.decodeQueueSize is 0.
T) D.decode() for sample #7 succeeded. Get VideoFrame #7.
T) Get a dequeue event #8. D.decodeQueueSize is still 0.
=> "Dequeue event after queue empty" fails here
=> "Dequeue event without decreased queue size" fails here as well

In brief, what VideoDecoder can guarantee are the order of dequeue
event, decode() result, and the flush() result:

1. For sample #N, its dequeue event arrives before it's decode()
   callback
2. The results/callbacks of decode() and flush() are arrived in FIFO
   order

There is no guarantee that decodeQueueSize read on the VideoDecoder's
owner thread only decrease one per dequeue event. If the
codec-work-queue decodes sample #1, #2, ... , #K while the dequeue event
for sample #1 is being dispatched to or handled on the VideoDecoder's
owner thread, the last-dequeueQueueSize can drop more than one compared
to it's previous value, and for the same reason, the dequeue event can
arrive even if the last-dequeueQueueSize has been set to 0.

Depends on D177212

Comment 20

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Schedule to report error in Close()

The WPT assumes the error callback invoked via the decode() failure will
arrive before flush()'s promise, but it's not guaranteed. The example
below demonstrates why this assumption isn't held.

Suppose VideoDecoder D runs on thread T (main or worker) and
codec-work-queue runs on thread Q. In the test, by the time
VideoDecoder is configured successfully and decode request are sent,
the codec-work-queue is ready to decode the pending samples. Below
illustrates that error callback can arrive after flush()'s promise.

Q) The underlying codec is configured, ready to decode sample #1.
Q) Decode sample #1 succeeded, ready to decode sample #2.
T) D.decode() for sample #1 succeeded. Get VideoFrame #1.
Q) Decode sample #1 failed, ready to flush the decoder.
T) D.decode() for sample #2 failed. Close D via close() with an
EncodingError. The close queues a task to invoke the error
callback.
Q) Flush has been done.
T) D.flush() has done (but D has been closed) and its promise get
rejected
=> error callback hasn't arrive yet!
T) The error callback scheduled by close above is called

Depends on D179514

Comment 21

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Downgrade BitReader::ReadBits's assertion to warning

BitReader::ReadBits will call a NS_ASSERTION when the request is
unreasonable. However, in WebCodecs' VedioDecoder wpt, one test will
intentionally decode a corrupted sample and it makes the av1 decoder to
reach that assertion and cause a wpt failure. Since that assertion
doesn't really do any meaningful work, downgrade it to NS_WARNING
to make wpt work seems fine.

Depends on D179515

Comment 22

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Match display ratio to aspect ratio

Depends on D171518

Comment 23

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Reject flush with EncodingError if decode fails

If decoding results in an error, queue a task to run the Close
VideoDecoder algorithm with EncodingError and return

By the spec, in step 4-2 when running a control message for decode, the
flush queued after decode should be rejected with a EncodingError, via
Close algorithm, instead of AbortError.

Depends on D179583

Comment 24

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Avoid MOZ_CAN_RUN_SCRIPT warnings in runnables

This patch addes work-arounds to using MOZ_CAN_RUN_SCRIPT function in
nsIEventTarget::Dispatch and runnables. The MOZ_CAN_RUN_SCRIPT functions
should be executed onlt within another MOZ_CAN_RUN_SCRIPT function.
However, Dispatch functions have no MOZ_CAN_RUN_SCRIPT marks so this
patch addes work-arounds to avoid those kind of warnings.

Depends on D179515

Comment 25

[Phabricator Automation]

Comment on attachment 9331698 [details]
Bug 1831451 - Add a blank EncodedVideoChunk interface

Revision D155892 was moved to bug 1838958. Setting attachment 9331698 [details] to obsolete.

Comment 26

[Phabricator Automation]

Comment on attachment 9331699 [details]
Bug 1831451 - Implement EncodedVideoChunk

Revision D158238 was moved to bug 1838958. Setting attachment 9331699 [details] to obsolete.

Comment 27

[Phabricator Automation]

Comment on attachment 9331701 [details]
Bug 1831451 - Add NS_WARN_IF to unlikely condition

Revision D158815 was moved to bug 1838958. Setting attachment 9331701 [details] to obsolete.

Comment 28

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Enable VideoDecoder in idlharness wpt pages

Depends on D180982

Comment 29

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Expose VideoDecoder in test_(worker_)interfaces.html

Depends on D181327

Comment 30

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Move VideoColorSpace-to-gfx-values functions to utils

Depends on D166402

Comment 31

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Introduce DecoderAgent

DecoderAgent is a class operating MediaDataDecoder. It combines multiple
MediaDataDecoder APIs into one to serve WebCodecs' needs.

Depends on D181818

Comment 32

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Add Decode() into DecoderAgent

Depends on D161356

Comment 33

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Add DrainAndFlush() into DecoderAgent

Depends on D171809

Comment 34

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Enable VideoDecoder on Linux only

H264-annexb doesn't work on non-linux for now. This will be sovled in
Bug 1840192 later.

Depends on D179515

Comment 35

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Create a class for ControlMessage

This patch replaces the using ControlMessage = Variant<...> by a
class ControlMessage, which wraps the common interfaces and variables
of the classes within the above Variant into a class.

Depends on D179515

Comment 36

[C.M.Chang[:chunmin]]

Attachment: Bug 1831451 - Generate a unique Id for both ConfigureMessage and DecoderAgent

To batter tracking the ControlMessage logs, ConfigureMessage should
generate a unique Id for itself and its following DecodeMessages and
FlushMessages. Besides, that unique id can be assigned to DecoderAgent
as well. As a result, it's easier to know what ConfigureMessage and
DecoderAgent the DecodeMessage/FlushMessage belongs.

This patch moves the id generation from DecoderAgent to ConfigureMessage
so the id can be used for both of DecoderAgent and ConfigureMessage.

Depends on D182124

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb17a0f3efdf
Add a blank VideoDecoder interface r=webidl,padenot,edgar,smaug,saschanaz
https://hg.mozilla.org/integration/autoland/rev/8f3c10480da7
Implement VideoDecoder::IsConfigSupported r=padenot
https://hg.mozilla.org/integration/autoland/rev/4e7051264cb3
Implement VideoDecoder::Constructor r=padenot
https://hg.mozilla.org/integration/autoland/rev/f615134b7349
Implement VideoDecoder::Reset r=padenot
https://hg.mozilla.org/integration/autoland/rev/94ad056419ea
Implement VideoDecoder::Close r=padenot
https://hg.mozilla.org/integration/autoland/rev/8a3af1118791
Add a VideoColorSpace-to-ColorRange function r=padenot
https://hg.mozilla.org/integration/autoland/rev/05ae55dd0cec
Move VideoColorSpace-to-gfx-values functions to utils r=padenot
https://hg.mozilla.org/integration/autoland/rev/0fca9e6c2ef6
Introduce DecoderAgent r=alwu
https://hg.mozilla.org/integration/autoland/rev/68fae9bd7d92
Implement VideoDecoder::Configure r=padenot
https://hg.mozilla.org/integration/autoland/rev/06f1008127bb
Add Decode() into DecoderAgent r=padenot
https://hg.mozilla.org/integration/autoland/rev/c8fdb24dd953
Implement VideoDecoder::Decode r=padenot
https://hg.mozilla.org/integration/autoland/rev/00bf4fed8bcb
Fire a dequeue event when data is being decoded r=padenot
https://hg.mozilla.org/integration/autoland/rev/89cf8d2548a8
Add DrainAndFlush() into DecoderAgent r=padenot
https://hg.mozilla.org/integration/autoland/rev/61be9a6b9cd4
Implement VideoDecoder::Flush r=padenot
https://hg.mozilla.org/integration/autoland/rev/d7f55df2c2e4
Guess color space from VideoData r=aosmond
https://hg.mozilla.org/integration/autoland/rev/1cf50ce36a73
Guess pixel format from VideoData r=aosmond
https://hg.mozilla.org/integration/autoland/rev/f19cfef38d63
Allow constructing VideoFrame without pixel format r=padenot
https://hg.mozilla.org/integration/autoland/rev/342a32c598fe
Match display ratio to aspect ratio r=aosmond
https://hg.mozilla.org/integration/autoland/rev/e2272bf547e1
Enable VideoDecoder WPTs r=padenot
https://hg.mozilla.org/integration/autoland/rev/81c7ebd2aeb3
Downgrade BitReader::ReadBits's assertion to warning r=padenot
https://hg.mozilla.org/integration/autoland/rev/f1b3b6d3186b
Reject flush with EncodingError if decode fails r=padenot
https://hg.mozilla.org/integration/autoland/rev/ecb08baa7f0e
Schedule to report error in Close() r=padenot
https://hg.mozilla.org/integration/autoland/rev/6892b88cb4ee
Create a class for ControlMessage r=alwu
https://hg.mozilla.org/integration/autoland/rev/cd30b650aeed
Generate a unique Id for both ConfigureMessage and DecoderAgent r=alwu
https://hg.mozilla.org/integration/autoland/rev/200c174fc2d1
Enable VideoDecoder on Linux only r=padenot
https://hg.mozilla.org/integration/autoland/rev/91c2c0f7e9ab
Enable VideoDecoder in idlharness wpt pages r=webidl,saschanaz
https://hg.mozilla.org/integration/autoland/rev/4fb457340ffe
Expose VideoDecoder in test_(worker_)interfaces.html r=webidl,saschanaz,padenot

Comment 38

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/40805 for changes under testing/web-platform/tests

Comment 39

[Cosmin Sabou [:CosminS]]

Backed out for causing hazard failures on VideoDecoder.cpp

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&fromchange=4fb457340ffedd7d6e3233c120f578621eb8c2c3&tochange=b0bf1e68a80c2a34bfb859b939005f93b9d39b5d&selectedTaskRun=IZo6LtkVR0qxNX47J130XA.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=421052163&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/b0bf1e68a80c2a34bfb859b939005f93b9d39b5d

Comment 40

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 41

[Paul Adenot (:padenot)]

Attachment: Bug 1831451 - Don't use DOM objects internally in Web Codecs implementation. r?chunmin

Comment 42

[C.M.Chang[:chunmin]]

I've added additional fixes on top of D182476 and make it pass the hazard test:
https://treeherder.mozilla.org/jobs?repo=try&revision=3186420fde92bb56c5c288855de3e84f425c365b&selectedTaskRun=IBj231ftTZK5_tL-Xvk-rQ.0

I am going to merge my fixes into D182476 so we can land it.

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77b0f5226ed8
Add a blank VideoDecoder interface r=webidl,padenot,edgar,smaug,saschanaz
https://hg.mozilla.org/integration/autoland/rev/02cb31bfdc0f
Implement VideoDecoder::IsConfigSupported r=padenot
https://hg.mozilla.org/integration/autoland/rev/23bc44bf238c
Implement VideoDecoder::Constructor r=padenot
https://hg.mozilla.org/integration/autoland/rev/58a0e543998b
Implement VideoDecoder::Reset r=padenot
https://hg.mozilla.org/integration/autoland/rev/e06fd5d60970
Implement VideoDecoder::Close r=padenot
https://hg.mozilla.org/integration/autoland/rev/981e50f618d0
Add a VideoColorSpace-to-ColorRange function r=padenot
https://hg.mozilla.org/integration/autoland/rev/d62002584cf3
Move VideoColorSpace-to-gfx-values functions to utils r=padenot
https://hg.mozilla.org/integration/autoland/rev/13f8ed81dca8
Introduce DecoderAgent r=alwu
https://hg.mozilla.org/integration/autoland/rev/aea093a94985
Implement VideoDecoder::Configure r=padenot
https://hg.mozilla.org/integration/autoland/rev/43451f9834e7
Add Decode() into DecoderAgent r=padenot
https://hg.mozilla.org/integration/autoland/rev/226dda36eb9e
Implement VideoDecoder::Decode r=padenot
https://hg.mozilla.org/integration/autoland/rev/a86ecdaa1716
Fire a dequeue event when data is being decoded r=padenot
https://hg.mozilla.org/integration/autoland/rev/57df271c6d6c
Add DrainAndFlush() into DecoderAgent r=padenot
https://hg.mozilla.org/integration/autoland/rev/290fcbe92f0a
Implement VideoDecoder::Flush r=padenot
https://hg.mozilla.org/integration/autoland/rev/0899b9a2539e
Guess color space from VideoData r=aosmond
https://hg.mozilla.org/integration/autoland/rev/b49ae6bc356a
Guess pixel format from VideoData r=aosmond
https://hg.mozilla.org/integration/autoland/rev/1ec020b42625
Allow constructing VideoFrame without pixel format r=padenot
https://hg.mozilla.org/integration/autoland/rev/bdfa5574beda
Match display ratio to aspect ratio r=aosmond
https://hg.mozilla.org/integration/autoland/rev/20228a5c6504
Enable VideoDecoder WPTs r=padenot
https://hg.mozilla.org/integration/autoland/rev/ad5a8d4e40e6
Downgrade BitReader::ReadBits's assertion to warning r=padenot
https://hg.mozilla.org/integration/autoland/rev/30eaff96a933
Reject flush with EncodingError if decode fails r=padenot
https://hg.mozilla.org/integration/autoland/rev/049d2679df84
Schedule to report error in Close() r=padenot
https://hg.mozilla.org/integration/autoland/rev/b3c2b04d4a36
Create a class for ControlMessage r=alwu
https://hg.mozilla.org/integration/autoland/rev/8fe486442687
Generate a unique Id for both ConfigureMessage and DecoderAgent r=alwu
https://hg.mozilla.org/integration/autoland/rev/ff0876ea84c9
Enable VideoDecoder on Linux only r=padenot
https://hg.mozilla.org/integration/autoland/rev/0ac162e771bb
Enable VideoDecoder in idlharness wpt pages r=webidl,saschanaz
https://hg.mozilla.org/integration/autoland/rev/7dd351fce5ad
Expose VideoDecoder in test_(worker_)interfaces.html r=webidl,saschanaz,padenot
https://hg.mozilla.org/integration/autoland/rev/6c61c412fe0b
Don't use DOM objects internally in Web Codecs implementation. r=chunmin

Comment 44

[C.M.Chang[:chunmin]]

Just found a unused-variable in the D182476. Here is the simple fix: https://phabricator.services.mozilla.com/D182533

It will be done with other issues in bug 1841193

Comment 45

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/77b0f5226ed8
https://hg.mozilla.org/mozilla-central/rev/02cb31bfdc0f
https://hg.mozilla.org/mozilla-central/rev/23bc44bf238c
https://hg.mozilla.org/mozilla-central/rev/58a0e543998b
https://hg.mozilla.org/mozilla-central/rev/e06fd5d60970
https://hg.mozilla.org/mozilla-central/rev/981e50f618d0
https://hg.mozilla.org/mozilla-central/rev/d62002584cf3
https://hg.mozilla.org/mozilla-central/rev/13f8ed81dca8
https://hg.mozilla.org/mozilla-central/rev/aea093a94985
https://hg.mozilla.org/mozilla-central/rev/43451f9834e7
https://hg.mozilla.org/mozilla-central/rev/226dda36eb9e
https://hg.mozilla.org/mozilla-central/rev/a86ecdaa1716
https://hg.mozilla.org/mozilla-central/rev/57df271c6d6c
https://hg.mozilla.org/mozilla-central/rev/290fcbe92f0a
https://hg.mozilla.org/mozilla-central/rev/0899b9a2539e
https://hg.mozilla.org/mozilla-central/rev/b49ae6bc356a
https://hg.mozilla.org/mozilla-central/rev/1ec020b42625
https://hg.mozilla.org/mozilla-central/rev/bdfa5574beda
https://hg.mozilla.org/mozilla-central/rev/20228a5c6504
https://hg.mozilla.org/mozilla-central/rev/ad5a8d4e40e6
https://hg.mozilla.org/mozilla-central/rev/30eaff96a933
https://hg.mozilla.org/mozilla-central/rev/049d2679df84
https://hg.mozilla.org/mozilla-central/rev/b3c2b04d4a36
https://hg.mozilla.org/mozilla-central/rev/8fe486442687
https://hg.mozilla.org/mozilla-central/rev/ff0876ea84c9
https://hg.mozilla.org/mozilla-central/rev/0ac162e771bb
https://hg.mozilla.org/mozilla-central/rev/7dd351fce5ad
https://hg.mozilla.org/mozilla-central/rev/6c61c412fe0b

Comment 46

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot