1919384 - Optimize latency of getting encoded result in AppleVTEncoder

Status: RESOLVED FIXED

(Core :: Audio/Video, enhancement, P1)

Description

[C.M.Chang[:chunmin]]

Currently, the AppleVTEncoder's encoded result will delay at least one encoding iteration since the encoded output is yielded when Encode is called, not when ProcessOutput is executed.

In some cases (e.g., Zoom's web conferencing), getting the encoded result as soon as possible is critical. For now, the sample encoded in the Nth round will be dispatched by the (N+1) th round's output callback (That is, users need to call Encode again to get the previous Encode() results). Some real-time usage requires getting the encoded result ASAP (not in the next encoding iteration). Additionally, doing this aligns with the encoding behaviors in other codecs like VPX and AV1.

Comment 1

[C.M.Chang[:chunmin]]

Attachment: videoencoder.html

This behavior can be tested by:

1. Select a codec type (for H264, it requires to select AVC or ANNEXB as well)
2. Check the Wait per frame option
3. Open the console to watch the encoder outputs

Expected result: For all codecs, we can get one output data after one encode call
Actual result: For H264, no output is yielded after the first encode (other codecs work as expected)

Comment 2

[C.M.Chang[:chunmin]]

Attachment: WIP: Bug 1919384 - Add debugging logs

Comment 3

[C.M.Chang[:chunmin]]

Attachment: WIP: Bug 1919384 - Remove an unused function

Depends on D222554

Comment 4

[C.M.Chang[:chunmin]]

Attachment: WIP: Bug 1919384 - Correct SetFrameRate setting

Depends on D222555

Comment 5

[C.M.Chang[:chunmin]]

Attachment: WIP: Bug 1919384 - Release CFNumberRef for SVC setting automatically

Depends on D222556

Comment 6

[C.M.Chang[:chunmin]]

Attachment: Bug 1919384 - Add MaxFrameDelayCount setting

Depends on D222557

Comment 7

[C.M.Chang[:chunmin]]

Attachment: Bug 1919384 - Improve realtime encoding in AppleVTEncoder

Depends on D222558

Comment 8

[C.M.Chang[:chunmin]]

Attachment: WIP: Bug 1919384 - Simplify realtime setting

Depends on D222559

Comment 9

[C.M.Chang[:chunmin]]

Attachment: WIP: Bug 1919384 - Remove empty comment

Comment 10

[C.M.Chang[:chunmin]]

Attachment: videoencoder.html

Comment 11

[Phabricator Automation]

Comment on attachment 9426036 [details]
WIP: Bug 1919384 - Remove empty comment

Revision D222883 was moved to bug 1920740. Setting attachment 9426036 [details] to obsolete.

Comment 12

[Phabricator Automation]

Comment on attachment 9425428 [details]
WIP: Bug 1919384 - Remove an unused function

Revision D222555 was moved to bug 1920740. Setting attachment 9425428 [details] to obsolete.

Comment 13

[Phabricator Automation]

Comment on attachment 9425427 [details]
WIP: Bug 1919384 - Add debugging logs

Revision D222554 was moved to bug 1920740. Setting attachment 9425427 [details] to obsolete.

Comment 14

[Phabricator Automation]

Comment on attachment 9425429 [details]
WIP: Bug 1919384 - Correct SetFrameRate setting

Revision D222556 was moved to bug 1920740. Setting attachment 9425429 [details] to obsolete.

Comment 15

[Phabricator Automation]

Comment on attachment 9425433 [details]
WIP: Bug 1919384 - Simplify realtime setting

Revision D222560 was moved to bug 1920740. Setting attachment 9425433 [details] to obsolete.

Comment 16

[C.M.Chang[:chunmin]]

Attachment: Bug 1919384 - Add PrioritizeEncodingSpeedOverQuality setting

Depends on D222558

Comment 17

[Phabricator Automation]

Comment on attachment 9425430 [details]
WIP: Bug 1919384 - Release CFNumberRef for SVC setting automatically

Revision D222557 was moved to bug 1920740. Setting attachment 9425430 [details] to obsolete.

Comment 18

[C.M.Chang[:chunmin]]

Attachment: Bug 1919384 - Add number of outputs in drain log

Comment 19

[C.M.Chang[:chunmin]]

Attachment: Bug 1919384 - Sort include headers

Comment 20

[C.M.Chang[:chunmin]]

Attachment: Bug 1919384 - Apply `AutoCFRelease` to `isUsingHW`

Depends on D224052

Comment 21

[C.M.Chang[:chunmin]]

Attachment: Bug 1919384 - Handle encode-error in AppleVTEncoder for real-time encoding

Previously, the AppleVTEncoder did not adequately handle situations
where encoding errors occured, frames were dropped, or the encoder
produced empty outputs. In such cases, the output processing callback
was not invoked. This issue is particularly problematic in real-time
encoding modification introduced in the prior patch, where the encode
promse is managed within the output processing callback, causing the
encode promise to remain pending indefinitely.

This patch introduces an enum that encompasses the following cases:
encode error, frame dropped, empty output, and success. When the system
callback is invoked, the encode result is labeled with one of these enum
valus. This result is then passed to the output processing callback,
allowing it to appropriately resolve or reject the encode promise based
on the specific usage.

By explicitly handling these scenarios, the encoder ensures that encode
promises are properly resolved or rejected in all the cases. This
improvement prevent promises from remaining unresolved and avoiding
potential application hangs.

Depends on D222559

Comment 22

[C.M.Chang[:chunmin]]

Attachment: videoencoder.html

Comment 23

[C.M.Chang[:chunmin]]

Attachment: Bug 1919384 - Update comment for mEncodedData

Depends on D224054

Comment 24

[C.M.Chang[:chunmin]]

Attachment: Bug 1919384 - Use gfx::IntSize to create encoder directly in MediaDataEncoderTest

Depends on D224054

Comment 25

[C.M.Chang[:chunmin]]

Attachment: Bug 1919384 - Add 4K image encoding tests in MediaDataEncoderTest

This patch introduces 4K image encoding tests to evaluate the encoder's
behavior, specifically frame dropping, under different scenarios. By
incorporating 4K images into our test suite, we can thoroughly assess
how the encoder handles frame dropping policies in both Realtime and
Record usages.

In the Realtime usage scenario, it allows frames to be dropped if
necessary, preventing delays when the encoder cannot keep up with the
desired frame rate. In contrast, the Record usage scenario demands
that all frames be preserved to maintain the highest possible quality.

By adding these 4K tests, we can verify that the encoder correctly
handles frame dropping policies in each usage mode. In addition, it's
easier to assess the performance by adding some timestamps if needed.

Depends on D224436

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e3ae2c4c30ec
Update comment for mEncodedData r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/fe3e648e6469
Sort include headers r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/c9459d9e194d
Apply `AutoCFRelease` to `isUsingHW` r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/de5d38923b03
Add number of outputs in drain log r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/56dc8b01a4ed
Add MaxFrameDelayCount setting r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/863a05b29d03
Add PrioritizeEncodingSpeedOverQuality setting r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/8bb80f43f2d7
Improve realtime encoding in AppleVTEncoder r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/9f1f74ca5453
Handle encode-error in AppleVTEncoder for real-time encoding r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/f1fe75359c00
Use gfx::IntSize to create encoder directly in MediaDataEncoderTest r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/c8826ab7eda8
Add 4K image encoding tests in MediaDataEncoderTest r=media-playback-reviewers,alwu

Comment 27

[Atila Butkovits]

Backed out for causing failures at GeckoProfiler.FeatureCombinations.

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

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=c8826ab7eda8418f81227ddf87eff2a8f6537a84

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

Comment 28

[C.M.Chang[:chunmin]]

(In reply to Atila Butkovits from comment #27)

Backed out for causing failures at GeckoProfiler.FeatureCombinations.

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

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=c8826ab7eda8418f81227ddf87eff2a8f6537a84

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

This is weird. Those patches are unrelated to GeckoProfiler. Besides, most of the patches are Mac only, and GeckoProfiler.FeatureCombinations only fails on Windows 11 x86 22H2 WebRender opt.

Comment 29

[C.M.Chang[:chunmin]]

From my test, applying D224436 on top of the mc has a high chance of making GeckoProfiler.AllThreads fail: https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=bgBVvCGUQx2x2qqa0NubPg.0&revision=4969a482320da92e4b08f265ba9fd986ac2982a9
If the D224437 is applied as well on top of D224436, then GeckoProfiler.FeatureCombinations will fail: https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=VQZO1QOxQsaeRK-0J0Av8Q.0&revision=6f74401fcbc9778192d65bdc2ac2fa5693521f68

Bug 1925510 seems to be dealing with a similar issue. Nazim, do you happen to know what the best approach here is? Is it ok to disable GeckoProfiler.FeatureCombinations on 32-bit Windows as Bug 1925510 did?

Comment 30

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

It looks strange. But yeah, I think your patches are unrelated to the profiler failures. The best way forward is to disable these tests on 32-bit platforms for now so it'll unblock you. I will also ask the infra folks if they changed anything related to the 32-bit machines.

Comment 31

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

The patch that disables these tests on 32-bit platforms has landed to mozilla-central.

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7fcec415d328
Update comment for mEncodedData r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/bde8330f6a1c
Sort include headers r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/0bb01cdfe814
Apply `AutoCFRelease` to `isUsingHW` r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/7c3e23262156
Add number of outputs in drain log r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/738094007591
Add MaxFrameDelayCount setting r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/e6f664d69472
Add PrioritizeEncodingSpeedOverQuality setting r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/906fbb518c6c
Improve realtime encoding in AppleVTEncoder r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/466bf0fbcc2c
Handle encode-error in AppleVTEncoder for real-time encoding r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/f07f2e98c66e
Use gfx::IntSize to create encoder directly in MediaDataEncoderTest r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/7efff3e68a28
Add 4K image encoding tests in MediaDataEncoderTest r=media-playback-reviewers,alwu

Comment 33

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/7fcec415d328
https://hg.mozilla.org/mozilla-central/rev/bde8330f6a1c
https://hg.mozilla.org/mozilla-central/rev/0bb01cdfe814
https://hg.mozilla.org/mozilla-central/rev/7c3e23262156
https://hg.mozilla.org/mozilla-central/rev/738094007591
https://hg.mozilla.org/mozilla-central/rev/e6f664d69472
https://hg.mozilla.org/mozilla-central/rev/906fbb518c6c
https://hg.mozilla.org/mozilla-central/rev/466bf0fbcc2c
https://hg.mozilla.org/mozilla-central/rev/f07f2e98c66e
https://hg.mozilla.org/mozilla-central/rev/7efff3e68a28

Comment 34

[Andrej (:aglavic)]

(In reply to Norisz Fay [:noriszfay] from comment #33)

https://hg.mozilla.org/mozilla-central/rev/7fcec415d328
https://hg.mozilla.org/mozilla-central/rev/bde8330f6a1c
https://hg.mozilla.org/mozilla-central/rev/0bb01cdfe814
https://hg.mozilla.org/mozilla-central/rev/7c3e23262156
https://hg.mozilla.org/mozilla-central/rev/738094007591
https://hg.mozilla.org/mozilla-central/rev/e6f664d69472
https://hg.mozilla.org/mozilla-central/rev/906fbb518c6c
https://hg.mozilla.org/mozilla-central/rev/466bf0fbcc2c
https://hg.mozilla.org/mozilla-central/rev/f07f2e98c66e
https://hg.mozilla.org/mozilla-central/rev/7efff3e68a28

We're seeing improvements in mac performance here!
Thank you for all your work! :)
Perfherder has detected a mozperftest performance change from push 7efff3e68a28f3bc3685ef0b650327960bf34c5f.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
52%	test_encode_from_canvas.html realtime - frame-to-frame stddev (key)	macosx1015-64-shippable-qr		61.82 -> 29.66
50%	test_encode_from_canvas.html realtime - frame-to-frame mean (non key)	macosx1015-64-shippable-qr		163.94 -> 81.64
48%	test_encode_from_canvas.html realtime - frame-to-frame mean (non key)	macosx1015-64-shippable-qr		175.04 -> 90.82
46%	test_encode_from_canvas.html realtime - frame-to-frame mean (key)	macosx1015-64-shippable-qr		157.03 -> 84.61

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 42692

For more information on performance sheriffing please see our FAQ.