1441260 - Unify interface for setting RTP headers for Audio and Video conduits

Status: RESOLVED FIXED

(Core :: WebRTC, enhancement, P3)

Description

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Audio and Video conduits use different interfaces for setting the RTP extmaps.  This should be unified ahead of the webrtc 64 merge to reduce merge pain.

Comment 1

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Attachment: Bug 1441260 - unify API for setting extmaps in AudioConduit

Review commit: https://reviewboard.mozilla.org/r/223412/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223412/

Comment 2

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8954266 [details]
Bug 1441260 - unify API for setting extmaps in AudioConduit

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223412/diff/1-2/

Comment 3

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8954266 [details]
Bug 1441260 - unify API for setting extmaps in AudioConduit

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223412/diff/2-3/

Comment 4

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8954266 [details]
Bug 1441260 - unify API for setting extmaps in AudioConduit

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223412/diff/3-4/

Comment 5

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8954266 [details]
Bug 1441260 - unify API for setting extmaps in AudioConduit

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223412/diff/4-5/

Comment 6

[Dan Minor [:dminor]]

Comment on attachment 8954266 [details]
Bug 1441260 - unify API for setting extmaps in AudioConduit

https://reviewboard.mozilla.org/r/223412/#review230022

lgtm, thanks for cleaning this up.

::: media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h:37
(Diff revision 5)
>  class VideoFrame;
>  }
>  
>  namespace mozilla {
>  
> +enum class MediaSessionConduitLocalDirection : int {

nit: I don't think it's necessary to specify the type here.

Comment 7

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8954266 [details]
Bug 1441260 - unify API for setting extmaps in AudioConduit

https://reviewboard.mozilla.org/r/223412/#review230022

> nit: I don't think it's necessary to specify the type here.

This has to be done to allow forward declaration unfortunately.

Comment 8

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8954266 [details]
Bug 1441260 - unify API for setting extmaps in AudioConduit

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/223412/diff/5-6/

Comment 9

[Michael Froman [:mjf]]

Comment on attachment 8954266 [details]
Bug 1441260 - unify API for setting extmaps in AudioConduit

https://reviewboard.mozilla.org/r/223412/#review230678

Like a breath of fresh air!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa1869480dd5
unify API for setting extmaps in AudioConduit r=dminor,mjf

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/fa1869480dd5