1404992 - Rework VideoConduit

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, enhancement, P2)

Description

[Paul Adenot (:padenot)]

No need to reconfig on input resolution changes anymore, yet we still do.\nMultiple api surfaces: e.g., one for input frames and their scaling, one for the encoder and the rest of its configuration.\n Keeping state updated based on input from JSEP, incoming frames and encoder\nFeedback (like bandwidth, load) gets broken easily. Should be able to improve the inter-thread communication model.\nNeeds unit tests so we know given input signals causes the right calls into the encoder.

Comment 1

[Paul Adenot (:padenot)]

Andreas, can you rank this one, and/or file sub-bugs ?

Comment 2

[Andreas Pehrson [:pehrsons]]

I'll leave as is. Someone needs to do an analysis of paths in VideoConduit that could be simplified and file bugs then or do them together. It would make sense to do this while, or after, writing unittests, see bug 1406935, bug 1406936 and bug 1406937.

Comment 3

[Andreas Pehrson [:pehrsons]]

VideoConduit's time has come, to make it easier for bug 1253499 to land.

Comment 4

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1404992 - Remove unused/unnecessary code and do some cleanup. r?dminor

Comment 5

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1404992 - Use common code to convert images to I420. r?dminor

Comment 6

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1404992 - Initialize primitive VP8TrackEncoder members in declarations. r?dminor

Comment 7

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1404992 - Reduce bitrate selection paths. r?dminor

Comment 8

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1404992 - Initialize VideoConduit members in declarations. r?dminor

Comment 9

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1404992 - Refactor incoming frames' scaling path. r?dminor

Comment 10

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1404992 - Make VideoConduit scaling use a buffer pool. r?dminor

Comment 11

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1404992 - Audit, document, assert and fix threading policy of VideoConduit. r?dminor, r?bwc

Comment 12

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1404992 - Enable some upstream threading asserts. r?dminor

Comment 13

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1404992 - Don't rely on VideoAdapter in VideoConduit unittests. r?dminor

Comment 14

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1404992 - Recreate the VideoConduit's video adapter when setting the send codec config. r?dminor

This enables us to configure it for cropping to a certain resolution alignment in a future patch.
For instance with simulcast, so we don't have to skip a layer because it is impossible to scale
the highest layer any further without losing the aspect ratio.

Comment 15

[Andreas Pehrson [:pehrsons]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a93428f86871d11b6bec6ac8c06ff42397b01f2f

Comment 16

[Dan Minor [:dminor]]

Comment on attachment 9003532 [details]
Bug 1404992 - Remove unused/unnecessary code and do some cleanup. r?dminor

Dan Minor [:dminor] has approved the revision.

Comment 17

[Dan Minor [:dminor]]

Comment on attachment 9003533 [details]
Bug 1404992 - Use common code to convert images to I420. r?dminor

Dan Minor [:dminor] has approved the revision.

Comment 18

[Dan Minor [:dminor]]

Comment on attachment 9003534 [details]
Bug 1404992 - Initialize primitive VP8TrackEncoder members in declarations. r?dminor

Dan Minor [:dminor] has approved the revision.

Comment 19

[Dan Minor [:dminor]]

Comment on attachment 9003535 [details]
Bug 1404992 - Reduce bitrate selection paths. r?dminor

Dan Minor [:dminor] has approved the revision.

Comment 20

[Dan Minor [:dminor]]

Comment on attachment 9003536 [details]
Bug 1404992 - Initialize VideoConduit members in declarations. r?dminor

Dan Minor [:dminor] has approved the revision.

Comment 21

[Dan Minor [:dminor]]

Comment on attachment 9003537 [details]
Bug 1404992 - Refactor incoming frames' scaling path. r?dminor

Dan Minor [:dminor] has approved the revision.

Comment 22

[Dan Minor [:dminor]]

Comment on attachment 9003538 [details]
Bug 1404992 - Make VideoConduit scaling use a buffer pool. r?dminor

Dan Minor [:dminor] has approved the revision.

Comment 23

[Dan Minor [:dminor]]

Comment on attachment 9003541 [details]
Bug 1404992 - Don't rely on VideoAdapter in VideoConduit unittests. r?dminor

Dan Minor [:dminor] has approved the revision.

Comment 24

[Dan Minor [:dminor]]

Comment on attachment 9003542 [details]
Bug 1404992 - Recreate the VideoConduit's video adapter when setting the send codec config. r?dminor

Dan Minor [:dminor] has approved the revision.

Comment 25

[Andreas Pehrson [:pehrsons]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=673f87b5b4612754c30403c58dd26f44eca8f779

Comment 26

[Byron Campen [:bwc]]

Comment on attachment 9003539 [details]
Bug 1404992 - Audit, document, assert and fix threading policy of VideoConduit. r?dminor, r?bwc

Byron Campen [:bwc] has approved the revision.

Comment 27

[Dan Minor [:dminor]]

Comment on attachment 9003539 [details]
Bug 1404992 - Audit, document, assert and fix threading policy of VideoConduit. r?dminor, r?bwc

Dan Minor [:dminor] has approved the revision.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd4cdda124f5
Remove unused/unnecessary code and do some cleanup. r=dminor
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e9a6abc6007
Use common code to convert images to I420. r=dminor
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c4d50c4b2d3
Initialize primitive VP8TrackEncoder members in declarations. r=dminor
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d3035db6bbf
Reduce bitrate selection paths. r=dminor
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd00b044e6e8
Initialize VideoConduit members in declarations. r=dminor
https://hg.mozilla.org/integration/mozilla-inbound/rev/8609a269556a
Refactor incoming frames' scaling path. r=dminor
https://hg.mozilla.org/integration/mozilla-inbound/rev/dbd6b3384314
Make VideoConduit scaling use a buffer pool. r=dminor
https://hg.mozilla.org/integration/mozilla-inbound/rev/43ee5a35eaf1
Audit, document, assert and fix threading policy of VideoConduit. r=dminor, r=bwc
https://hg.mozilla.org/integration/mozilla-inbound/rev/b86237ac8855
Don't rely on VideoAdapter in VideoConduit unittests. r=dminor
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f177888ae65
Recreate the VideoConduit's video adapter when setting the send codec config. r=dminor

Comment 29

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/cd4cdda124f5
https://hg.mozilla.org/mozilla-central/rev/1e9a6abc6007
https://hg.mozilla.org/mozilla-central/rev/7c4d50c4b2d3
https://hg.mozilla.org/mozilla-central/rev/0d3035db6bbf
https://hg.mozilla.org/mozilla-central/rev/fd00b044e6e8
https://hg.mozilla.org/mozilla-central/rev/8609a269556a
https://hg.mozilla.org/mozilla-central/rev/dbd6b3384314
https://hg.mozilla.org/mozilla-central/rev/43ee5a35eaf1
https://hg.mozilla.org/mozilla-central/rev/b86237ac8855
https://hg.mozilla.org/mozilla-central/rev/9f177888ae65