784515 - Allow Audio without Video and unidirection Audio or Video in SIPCC

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Ethan Hugg [:ehugg]]

Need to implement:

Audio without Video
Video without Audio
unidirectional streams.

Comment 1

[Suhas]

Seeking more clarification ...

Does this mean ,  we need to support the following
1. SDP Offer Audio only and follow up with VCM/Media Conduit support 
2. SDP Offer Video only and follow up with VCM/Media Conduit support
3. SDP Offer or Answer with SendOnly, RecvOnly 

I am not sure how can this be configured or told via JavaScript. say , for example, SendOnly Audio Only Stream. Are we assuming something like we modify the SDP obtained on createOffer() and inserting it back in setLocalDesc()   ...

Comment 2

[Eric Rescorla (:ekr)]

Huh?

Audio w/o video:
1. You indicate which media streams you want via a combination of addStream() and hints.
2. It's already possible to have audio w/o video just by adding only an audio stream.
3. You should be able to have video w/o audio by adding only a video stream, but there is at least 1 and possibly many checks inside SIPCC to prevent it. Those checks need to be removed.

Unidirectional anything:
1. There is a hint to indicate that you want to receive media even if you didn't offer that type of media.
2. That hint needs to be wired up.

You should do the first set of tasks (video w/o audio) first. 

None of this should have any interaction with VCM/MediaConduit, since there are separate start recv and start send calls and we use MediaConduits only as one-way objects.

Comment 3

[enda mannion (:emannion)]

Attachment: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

One area here I may need to revisit is then obth options can only be true or false and there is no option to make the constraint undefined.  If I have to do this then I can make each option an enum and pass that into SIPCC.

Comment 4

[enda mannion (:emannion)]

Attachment: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

Added a revided patch that builds on alder tip.

Comment 5

[Eric Rescorla (:ekr)]

Comment on attachment 663484 [details] [diff] [review]
OfferWithoutAudio and OfferWithoutVideo unidirectional streams

Review of attachment 663484 [details] [diff] [review]:
-----------------------------------------------------------------

Partial review. This needs fixing to properly reflect the spec and also to rework the structure and the structure passing.

::: media/webrtc/signaling/include/CC_Call.h
@@ +302,5 @@
>             @return true or false.
>           */
>          virtual bool originateP2PCall (cc_sdp_direction_t video_pref, const std::string & digits, const std::string & ip) = 0;
>          
> +        virtual int createOffer (cc_media_constraints_t constraints) = 0;

Do you really want to pass a struct and not a pointer/ref to a struct?

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ +149,5 @@
>                else
>                {
>                  stream = remoteStream->GetMediaStream();
>  
> +                constraint = stream->GetHintContents();

See previous comments about hints vs constraints.

@@ +476,5 @@
>    return NS_OK;
>  }
>  
>  NS_IMETHODIMP
> +PeerConnectionImpl::CreateFakeMediaStream(PRUint32 constraint, nsIDOMMediaStream** retval)

Please revert all these inappropriate hints->constraints changes.

@@ +770,5 @@
>    CSFLogDebug(logTag, "AddStream");
>  
>    // TODO([e-mail]): Remove these asserts?
>    // Adding tracks here based on nsDOMMediaStream expectation settings
> +  PRUint32 constraints = stream->GetHintContents();

hints not constraints.

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
@@ +48,5 @@
>  #endif
>  
>  namespace sipcc {
>  
> +class MediaConstraints {

Why are you inventing a class here that needs to be tweaked for every new constraint.

Either simply use std::map<std::string, std::string> (feel fry to hide this behind a typedef) or have a class which wraps a map and has methods like isTrue(), getValue(), etc.

@@ +305,5 @@
>    static PeerConnectionImpl* CreatePeerConnection();
>    static void Shutdown();
>  
>    Role GetRole() const { return mRole; }
> +  void MakeMediaStream(PRUint32 constraint, nsIDOMMediaStream** stream);

Why did this change? This is not a constraint, it's a service function to create a representation of a remote stream. hints here is an output.

@@ +310,2 @@
>    void MakeRemoteSource(nsDOMMediaStream* stream, RemoteSourceStreamInfo** info);
> +  void CreateRemoteSourceStreamInfo(PRUint32 constraint, RemoteSourceStreamInfo** info);

Same here.

@@ +379,5 @@
>  
>    // Get the DTLS identity
>    mozilla::RefPtr<DtlsIdentity> const GetIdentity() { return mIdentity; }
>  
> +  NS_IMETHODIMP CreateOffer(MediaConstraints constraints);

Please do not pass structs as arguments.

Also, this should be const.

@@ +381,5 @@
>    mozilla::RefPtr<DtlsIdentity> const GetIdentity() { return mIdentity; }
>  
> +  NS_IMETHODIMP CreateOffer(MediaConstraints constraints);
> +
> +  NS_IMETHODIMP CreateAnswer(MediaConstraints constraints, const char* offer);

Same.

::: media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
@@ +200,5 @@
>  
>      return (dcb_p->media_cap_tbl);
>  }
>  
> +void gsmsdp_set_constraints(fsmdef_dcb_t *dcb, boolean offerToReceiveAudio, boolean offerToReceiveVideo) {

This does not seem like it is going to scale well to new constraints.

@@ +203,5 @@
>  
> +void gsmsdp_set_constraints(fsmdef_dcb_t *dcb, boolean offerToReceiveAudio, boolean offerToReceiveVideo) {
> +
> +    if (FALSE == offerToReceiveAudio) {
> +        dcb->media_cap_tbl->cap[CC_AUDIO_1].support_direction = SDP_DIRECTION_SENDONLY;

I don't follow this logic. What if there is no audio stream at all and no constraint set.

::: media/webrtc/signaling/src/sipcc/include/cc_call_feature.h
@@ +185,5 @@
>   * @return SUCCESS or FAILURE
>   */
>  cc_return_t CC_CallFeature_dial(cc_call_handle_t call_handle, cc_sdp_direction_t video_pref, const cc_string_t numbers);
>  
> +cc_return_t CC_CallFeature_CreateOffer(cc_call_handle_t call_handle, cc_media_constraints_t constraints);

Please don't pass structs.

::: media/webrtc/signaling/src/sipcc/include/cc_constants.h
@@ +581,5 @@
>  } cc_media_type_t;
>  
> +typedef struct cc_media_constraints_t_ {
> +    cc_boolean    offerToReceiveVideo;
> +    cc_boolean    offerToReceiveAudio;

Maybe use sipcc naming conventions here?

This isn't going to work because there are three values: true, false, and no constraint provided. See my email to the lsit.

::: media/webrtc/signaling/src/sipcc/include/ccapi_call.h
@@ +82,5 @@
>   */
>  cc_return_t CCAPI_Call_originateCall(cc_call_handle_t handle, cc_sdp_direction_t video_pref, cc_string_t digits);
>  
>  
> +cc_return_t CCAPI_CreateOffer(cc_call_handle_t handle, cc_media_constraints_t constraints);

Pass a const * here.

::: media/webrtc/signaling/src/softphonewrapper/CC_SIPCCCall.cpp
@@ +570,2 @@
>   */
> +int CC_SIPCCCall::createAnswer (cc_media_constraints_t constraints, const std::string & offersdp) {

Ref or pointer, please.

::: media/webrtc/signaling/src/softphonewrapper/CC_SIPCCCall.h
@@ +135,5 @@
>          virtual void removeStream(int streamId);
>          virtual bool setVolume(int volume);
>          virtual bool originateP2PCall (cc_sdp_direction_t video_pref, const std::string & digits, const std::string & ip);
> +        virtual int createOffer(cc_media_constraints_t constraints);
> +        virtual int createAnswer(cc_media_constraints_t constraints, const std::string & offersdp);

Pass a ref or a pointer.

Comment 6

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

I think is closer to what you may want but I not sure how we can avoid having some code in PCImpl that will not need updating when new constraints are added.

Comment 7

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

A few items to note.

1.  Not sure how to use the mandatory flag I pass with each constraint into SIPCC yet. Or if I need to as of yet.

2.  In cc_invokeFeatureSDPMode to add data to the hash you need a key, I am hardcoding this in CONSTRAINT_SESSION_ID,  it works but maybe I need to generate this.

3. I think the memory I allocate in buildArray is getting released in SIPCC but not totally sure. See CreateOffer new code, cpr_free.

4. Do I need to add locking to buildArray and setBooleanConstraint?

5. I do not add the constraints to the DCB I can easily if needed.

Comment 8

[Ethan Hugg [:ehugg]]

Comment on attachment 664993 [details] [diff] [review]
Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

Review of attachment 664993 [details] [diff] [review]:
-----------------------------------------------------------------

I will also give it a go on Linux and see if the unittests are happy there.

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ +53,5 @@
> +
> +  ConstraintInfo booleanconstraint;
> +  booleanconstraint.mandatory = mandatory;
> +
> +  if (true == enabled)

This should be if (enabled) according to the moz style guide.  There are a couple others in this method.

@@ +796,5 @@
>   * CC_SDP_DIRECTION_SENDRECV will not be used when Constraints are implemented
>   */
>  NS_IMETHODIMP
> +PeerConnectionImpl::CreateOffer(const char* constraints) {
> +  MOZ_ASSERT(constraints);

I understand that this one is going away.  It should be commented as such so it doesn't look broken.  Also the comment above is no longer useful.

@@ +818,5 @@
> +}
> +
> +NS_IMETHODIMP
> +PeerConnectionImpl::CreateAnswer(const char* constraints, const char* offer) {
> +  MOZ_ASSERT(constraints);

Some here if this one is going away as well.

::: media/webrtc/signaling/src/sipcc/core/ccapp/cc_call_feature.c
@@ +145,5 @@
>      callFeature.featData.ccData.level = level;
>  
> +
> +    /* If constraints exist add to session hash */
> +    if (NULL != constraints) {

if (constraints)

::: media/webrtc/signaling/src/sipcc/include/cc_constants.h
@@ +110,5 @@
>  #define GET_CALL_ID(call_handle) (cc_callid_t)(call_handle & 0xFFFF)
>  #define CREATE_CALL_HANDLE(line, callid) (cc_call_handle_t)(((line & 0xFFF) << CC_SID_LINE_SHIFT) + (callid & 0xFFFF))
>  #define CREATE_CALL_HANDLE_FROM_SESSION_ID(session_id) (session_id & 0xFFFFFFF)
>  #define CREATE_SESSION_ID_FROM_CALL_HANDLE(call_handle) ((CC_SESSIONTYPE_CALLCONTROL << CC_SID_TYPE_SHIFT) + call_handle)
> +#define CONSTRAINT_SESSION_ID 12345

I see where we set the sessionID to this, but I didn't find anywhere that it's checked.  Is it not really used, or planned or later?  Also do we know that it won't match some other sessionID?

@@ +588,5 @@
> +} cc_media_constraint_t;
> +
> +typedef struct {
> +  cc_media_constraint_t** constraints;
> +  cc_int16_t  constraintcnt;

I would consider changing this name to constraint_count and its type to unsigned unless there's a reason it could be negative.

Comment 9

[Ethan Hugg [:ehugg]]

Comment on attachment 664993 [details] [diff] [review]
Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

Review of attachment 664993 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/signaling/src/sipcc/core/ccapp/cc_call_feature.c
@@ +149,5 @@
> +    if (NULL != constraints) {
> +        if (constraints->constraintcnt > 0 &&
> +            (CC_FEATURE_CREATEOFFER == featureId || CC_FEATURE_CREATEANSWER == featureId )) {
> +            sessionItem.sessionID = CONSTRAINT_SESSION_ID;
> +            session_data_t * sessionData = (session_data_t *)findhash(sessionItem.sessionID);

This line produces this error on Linux
_feature.c:153:13: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]

Need to declare it at the top

Comment 10

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

The patch has been extended to handle negotiation of single stream media. Can now negotiate with just Audio or Video.

Comment 11

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

Comment 12

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

Added a new re-negotiation test.

Comment 13

[Ethan Hugg [:ehugg]]

Comment on attachment 665864 [details] [diff] [review]
Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams


These new tests don't check packets send/received.  We're going to probably need a fake video generator to properly test these.

Comment 14

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

All tests passing now except I get this strange crash on shutdown on Mac. I must remove my patch and test next. 

[       OK ] SignalingTest.FullCallTrickle (13536 ms)
[----------] 22 tests from SignalingTest (83771 ms total)

[----------] Global test environment tear-down
[==========] 22 tests from 1 test case ran. (83974 ms total)
[  PASSED  ] 22 tests.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00007fff5f3fff68
0x000000010b92d919 in arena_malloc (arena=0x10bd00040, size=296, zero=true) at /Users/Enda/code/mozilla/mozilla-central/memory/mozjemalloc/jemalloc.c:4205
4205			return (arena_malloc_small(arena, size, zero));
(gdb) bt
#0  0x000000010b92d919 in arena_malloc (arena=0x10bd00040, size=296, zero=true) at /Users/Enda/code/mozilla/mozilla-central/memory/mozjemalloc/jemalloc.c:4205
#1  0x000000010b9197a0 in icalloc (size=296) at /Users/Enda/code/mozilla/mozilla-central/memory/mozjemalloc/jemalloc.c:4227
#2  0x000000010b9196c2 in je_calloc (num=1, size=296) at /Users/Enda/code/mozilla/mozilla-central/memory/mozjemalloc/jemalloc.c:6476
#3  0x000000010b91d381 in zone_calloc (zone=0x10bade000, num=1, size=296) at /Users/Enda/code/mozilla/mozilla-central/memory/mozjemalloc/jemalloc.c:6926
#4  0x00007fff80142d1a in malloc_zone_calloc ()
#5  0x00007fff801467bb in calloc ()
#6  0x000000010b5267b5 in PR_Calloc (nelem=1, elsize=296) at /Users/Enda/code/mozilla/mozilla-central/nsprpub/pr/src/malloc/prmem.c:443
#7  0x000000010b54a69f in pt_AttachThread () at /Users/Enda/cod

Comment 15

[Eric Rescorla (:ekr)]

This is probably not your fault. I get shutdown crashes on mac regularly.

Comment 16

[Ethan Hugg [:ehugg]]

Comment on attachment 670425 [details] [diff] [review]
Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams


Did you upload the one that applies on top of M-C?  I get five .rej files from this on latest M-C.

Comment 17

[enda mannion (:emannion)]

(In reply to Eric Rescorla from comment #15)
> This is probably not your fault. I get shutdown crashes on mac regularly.

Yes, I built wothout my patch and get the same error.

Comment 18

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

I was building this patch on top of the hackhack patch and that would have causes a few conflicts.  This is definitly from mozilla-central branch.

Comment 19

[Ethan Hugg [:ehugg]]

Comment on attachment 670738 [details] [diff] [review]
Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

Review of attachment 670738 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.  I assume we will need a follow-on patch to check packets send/recv on these unittests along with a fake video generator.

Comment 20

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

When I merged on top of the m-c tip I yet again had a number of merge conflicts.

Comment 21

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

This time I ran hg qref

Comment 22

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 671115 [details] [diff] [review]
Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

Review of attachment 671115 [details] [diff] [review]:
-----------------------------------------------------------------

Needs some work, and some items here aren't nits.

Also, we're now using reviews in the 'normal' fashion. Code needs r+ before checkin.  Ethan, for media/webrtc/signaling, you can give r+ (unless for some reason you don't feel comfortable doing so).

::: media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp
@@ +106,5 @@
>          PeerConnectionImpl::kIdle == mSipccState) {
>        ChangeSipccState(PeerConnectionImpl::kStarted);
>      } else {
>        CSFLogError(logTag, "%s PeerConnection in wrong state", __FUNCTION__);
> +      //Cleanup();

Is there a reason this is being kept commented out?  Is it coming back, or does it have historical significance?

@@ +111,4 @@
>        MOZ_ASSERT(PR_FALSE);
>      }
>    } else {
> +    //Cleanup();

ditto

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ +82,5 @@
> +    return;
> +
> +  for (constraints_map::iterator it = mConstraints.begin();
> +          it != mConstraints.end(); ++it) {
> +    (*constraintarray)->constraints[i] = (cc_media_constraint_t*) malloc(sizeof(cc_media_constraint_t));

Either use cpr_malloc (infallible) or check the returns (repeat)

@@ +744,5 @@
>  
>  NS_IMETHODIMP
> +PeerConnectionImpl::CreateOffer(MediaConstraints& constraints) {
> +
> +  cc_media_constraints_t* cc_constraints = 0;

nullptr

@@ +745,5 @@
>  NS_IMETHODIMP
> +PeerConnectionImpl::CreateOffer(MediaConstraints& constraints) {
> +
> +  cc_media_constraints_t* cc_constraints = 0;
> +  constraints.buildArray(&cc_constraints);

At least in theory this can fail, though it doesn't have a good way to return it currently.  Give it a return value, and if it fails return an error.  Or make it infallible (cpr_malloc)

@@ +747,5 @@
> +
> +  cc_media_constraints_t* cc_constraints = 0;
> +  constraints.buildArray(&cc_constraints);
> +
> +  mCall->createOffer(cc_constraints);

Can this fail??

@@ +752,5 @@
> +  return NS_OK;
> +}
> +
> +NS_IMETHODIMP
> +PeerConnectionImpl::CreateAnswer(const char* constraints, const char* aOffer) {

Is this supposed to be unused?  Is this for the future?  If it is supposed to be here but is currently unused, use (void) constraints or some such to avoid compiler warnings.

@@ +759,5 @@
>  
>    CheckIceState();
>    mRole = kRoleAnswerer;  // TODO([e-mail]): Interrogate SIPCC here?
> +  MediaConstraints aconstraints;
> +  CreateAnswer(aconstraints, aOffer);

Can this fail??

@@ +766,5 @@
> +
> +NS_IMETHODIMP
> +PeerConnectionImpl::CreateAnswer(MediaConstraints& constraints, const char* offer) {
> +
> +  cc_media_constraints_t* cc_constraints = 0;

nullptr

@@ +767,5 @@
> +NS_IMETHODIMP
> +PeerConnectionImpl::CreateAnswer(MediaConstraints& constraints, const char* offer) {
> +
> +  cc_media_constraints_t* cc_constraints = 0;
> +  constraints.buildArray(&cc_constraints);

Repeat of previous comment

@@ +769,5 @@
> +
> +  cc_media_constraints_t* cc_constraints = 0;
> +  constraints.buildArray(&cc_constraints);
> +
> +  mCall->createAnswer(cc_constraints, offer);

Can this fail??

::: media/webrtc/signaling/src/sipcc/core/ccapp/cc_call_feature.c
@@ +120,5 @@
> +             * other usage of the hash table */
> +            session_id = abs(cpr_rand()) % 60000;
> +            sessionData = (session_data_t *)findhash(session_id);
> +            sessionData = cpr_malloc(sizeof(session_data_t));
> +            if (sessionData == NULL) {

cpr_malloc is infallible

::: media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c
@@ +2886,5 @@
>      }
>  
> +   if (msg->data.session.has_constraints) {
> +        sess_data_p = (session_data_t *)findhash(msg->data.session.sessionid);
> +        if (sess_data_p != NULL) {

if (sess_data_p) {  Unless that conflicts with file style.

@@ +2891,5 @@
> +            gsmsdp_process_cap_constraints(dcb, sess_data_p->cc_constraints);
> +
> +            if (0 > delhash(msg->data.session.sessionid)) {
> +                FSM_DEBUG_SM (DEB_F_PREFIX"failed to delete hash sessid=0x%08x\n",
> +                        DEB_F_PREFIX_ARGS(SIP_CC_PROV, __FUNCTION__), msg->data.session.sessionid);

fix indents, maybe split to 3 lines

@@ +2994,5 @@
> +            gsmsdp_process_cap_constraints(dcb, sess_data_p->cc_constraints);
> +
> +            if (0 > delhash(msg->data.session.sessionid)) {
> +                FSM_DEBUG_SM (DEB_F_PREFIX"failed to delete hash sessid=0x%08x\n",
> +                        DEB_F_PREFIX_ARGS(SIP_CC_PROV, __FUNCTION__), msg->data.session.sessionid);

indents again

@@ +3226,5 @@
>      if (JSEP_OFFER == action) {
>  
> +        cause = gsmsdp_process_offer_sdp(fcb, &msg_body, TRUE);
> +        if (cause != CC_CAUSE_OK) {
> +            ui_set_remote_description(evSetRemoteDescError, line, call_id, dcb->caller_id.call_instance_id, NULL, PC_SETREMOTEDESCERROR);

Wrap line please

::: media/webrtc/signaling/test/signaling_unittests.cpp
@@ +528,4 @@
>      // Create a media stream as if it came from GUM
>      nsRefPtr<nsDOMMediaStream> domMediaStream = new nsDOMMediaStream();
>  
> +    PRUint32 aHintContents = 0;

int32_t please

Comment 23

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

Incorporated all of rjesop's comments, excellent review thanks. I added a bug for the commented cleanup() code.  

https://bugzilla.mozilla.org/show_bug.cgi?id=801620

Comment 24

[Ethan Hugg [:ehugg]]

Comment on attachment 671435 [details] [diff] [review]
Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

Review of attachment 671435 [details] [diff] [review]:
-----------------------------------------------------------------

Please submit for review to Jesup. I'd like to get a r+ from him for this patch.

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ +73,5 @@
> +    return;
> +
> +  short i = 0;
> +  std::string tmpStr;
> +  *constraintarray = (cc_media_constraints_t*) malloc(sizeof(cc_media_constraints_t));

This should be cpr_malloc as well.  Then you can remove the check below.

@@ +77,5 @@
> +  *constraintarray = (cc_media_constraints_t*) malloc(sizeof(cc_media_constraints_t));
> +  if (!(*constraintarray))
> +    return;
> +
> +  (*constraintarray)->constraints = (cc_media_constraint_t**) malloc(mConstraints.size() * sizeof(cc_media_constraint_t));

Same with this one.

Comment 25

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

All comments now integrated.

Comment 26

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ba7d327ce67

Comment 27

[enda mannion (:emannion)]

Attachment: Revised: OfferWithoutAudio and OfferWithoutVideo unidirectional streams

Again re-merged to build on tip, no change required.

Comment 28

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/9ba7d327ce67

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+1]]

enda, I assume all the tests we can have, have been landed with the patch?

Comment 30

[enda mannion (:emannion)]

All tests I have written have landed with the patch, it is possble that I could write eve more tests to eek out other constraint permutations if required.