Recording a stream while adding/removing a track is not well defined

[youennf]

According https://w3c.github.io/mediacapture-record/#event-summary, when adding/removing a track to a stream being recorded, recorder should dispatch a MediaRecorderErrorEvent event.
It is unclear whether this is part of https://w3c.github.io/mediacapture-record/#mediarecorder-methods start step 5.3, in which case it should be UnknownError or something else.

[yellowdoge]

I'm not sure what the issue is: do you mean that if, upon start(), the Track set has changed (from those presented to the constructor), MediaRecorder should fire a MediaRecorderErrorEvent event? Or that said step 5.3 should also mention the error event itself?

[youennf]

step 5.3 does not seem to state that, once started, on track change, we should throw an error event and with which exception. It would be nice to clarify this.

[yellowdoge]

That's reasonable. Would you mind putting up a PR for it :-) ?

[alvestrand]

Note: events are dispatched, not thrown.... in this case, they would be dispatched to the MediaRecorder object, I assume.
Agree that we need the code to be clear.

[alvestrand]

I remember having the discussion about what to do when the set of tracks changes over the recorder's lifetime. The conclusion (I think) was that so few formats support doing this that we shouldn't bother, and just return an error, but the discussion's result could have been a little more obvious than just an "e.g." in the event methods summary.

[youennf]

PTAL at the PR.
I went with InvalidModificationError as error.
I'll write a corresponding WPT test once we agreed on this or another error.

[youennf]

Doing a quick check in Chrome and Firefox, this error handling does not seem implemented.

[Pehrsons]

I think the best way to resolve this is by continuing the discussion in the original issue, #4.

[youennf]

Let's close this issue then.
I'll upload a WPT test to align with the spec.
We can always revisit spec and tests based on #4

[Pehrsons]

@youennf Did you get around to that WPT test? I can at least not find one containing InvalidModificationError in mediacapture-record.

Also this change went from "MUST stop gathering data" to "MUST immediately stop gathering data, discard any data that it has gathered". For SecurityError it makes sense to discard the data (though does it make sense to still fire "dataavailable"?) but for modifying the track set I'm not so sure. Was this intended?

[youennf]

Thanks for pinging me, I forgot about this one.
Test uploaded and up for review.

I see your point about not discarding the data, I agree we should do this.
I am not sure to see any tangible difference between 'immediately stop gathering' and 'stop gathering'.
Should we just use 'stop gathering' everywhere?

[Pehrsons]

SGTM

[youennf]

Filed #162 for not discarding the data.
PR should be WPT testable.

PR also removes 'immediately'.
Maybe the spec could state clearly that the blob MUST be empty in such a case.
Or, the blob content could be cleared in the steps executed as part of the DOM manipulation task source.
Or, if backward compatible, do not fire the event as you point out.