Closed Bug 1424191 Opened 2 years ago Closed 2 years ago

MediaRecorder(videoStream) unexpectedly stops itself

Categories

(Core :: Audio/Video: Recording, defect, P1)

59 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- unaffected
firefox59 + fixed

People

(Reporter: tristan.fraipont, Assigned: mchiang)

References

Details

(Keywords: regression)

Attachments

(2 files)

Attached file MediaRecorderBug.html
When using the MediaRecorder with an LocalMediaStream fed by a VideoTrack, the MediaRecorder stops itself unexpectedly.

The dataavailable event fires, but with an empty Blob as .data .

The LocalMediaStream still reports has active.

Apparently caused by bug#1388219


mozregression results: 

Last good revision: 574f4f58fe09dd590ea892406e237318c31705b4 (2017-12-03)
First bad revision: 18a9cb5cb32d0e8031d0a80901b199d5e9827d83 (2017-12-05)

Last good revision: bdb599143009e2facf0306169f729e679729d493
First bad revision: 4a003542df78bc7f08b37c4759e3a64c71e74552

Last good revision: 1dd65ba14e0f0f67f6660273949c3a81a4cd1fa8
INFO: First bad revision: 741eed8ed628c81dec7e8d2b038f641591c71f9b
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=1dd65ba14e0f0f67f6660273949c3a81a4cd1fa8&tochange=741eed8ed628c81dec7e8d2b038f641591c71f9b
So after I read what incriminated bug#1388219 did, I found that setting a delay after the stream has started makes the MediaRecorder happy. 

So I guess the problem is that the stream is available before the constraints get found, and causes the stream to somehow switch between constraints, while being recorded?

Though I'm not sure about this, since even when setting the constraints to exact values the issue is still there.
Blocks: 1388219
Assignee: nobody → mchiang
This is the same as we were discussing on IRC last week, right?
If so, this is due to a video frame reaching the recorder with either a real frame of resolution 0x0 or metadata for "intrinsic size" of 0x0. It seems to happen on the first real frame that comes from our video capture backend.

[Tracking Requested - why for this release]:
Regression that makes a very common case of recording a video track throw an error.
Status: UNCONFIRMED → NEW
Rank: 8
Ever confirmed: true
Keywords: regression
Priority: -- → P1
Yes. I think this is what florian encountered.
Tracking for 59, since it sounds like this is a recent regression that may affect many users.
Comment on attachment 8936454 [details]
Bug 1424191 - set mImages[i] to nullptr in MediaEngineRemoteVideoSource::Start().

https://reviewboard.mozilla.org/r/207166/#review213422

::: dom/media/webrtc/MediaEngineCameraVideoSource.cpp:33
(Diff revision 2)
> +  // If we sent an Image object with 0 size, it will cause encoder init check failure.
> +  if (size.width == 0 || size.height == 0) {
> +    image = nullptr;
> +  }

The issue here is that we pass along a real image with resolution (0,0), isn't it?

When we append a non-null image it should have non-zero width and height. Making sure they're non-zero would be the proper fix for this.

(I don't understand how image->GetSize() returns (0,0) - how is that happening? Scaling goes wrong?)
Attachment #8936454 - Flags: review?(apehrson) → review-
Here is the behavior before any of multi-e10s patches landed:
Before any MediaEngineRemoteVideoSource::DeliverFrame() is called by Camera IPC thread, mImage is null [1]
We keep calling AppendToTrack with null mImage [2]
Since image is null, we set the size to 0 [3]
[1] https://hg.mozilla.org/mozilla-central/file/91fc3a79606b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp#l436
[2] https://hg.mozilla.org/mozilla-central/file/91fc3a79606b/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp#l357
[3] https://hg.mozilla.org/mozilla-central/file/91fc3a79606b/dom/media/webrtc/MediaEngineCameraVideoSource.cpp#l31

I am not expert of MSG, but according to my observation, in this period (before the first DeliverFrame being called),
If we call AppendToTrack with 0 size and non-null image, there will be encoder init error.
If we return directly when the size is 0, it will cause audio latency.
The only way which works is to keep feeding 0 size and null image to MSG.

Back to the latest behavior of my WIP.
Now we will set an empty image to mImages[i] in MediaEngineRemoteVideoSource::Start() [4]
So before any DeliverFrame() is called, mImages[i] is not null already. In default, GetSize() will return 0x0.
So in this period, we will keep feeding non-null image with 0 size, which causes the encoder init failure.

[4] https://searchfox.org/mozilla-central/rev/b0098afaeaad24a6752aa418ca3e86eade5fab17/dom/media/webrtc/MediaEngineRemoteVideoSource.cpp#193
s/Back to the latest behavior of my WIP/Back to the behavior of latest nightly/
Comment on attachment 8936454 [details]
Bug 1424191 - set mImages[i] to nullptr in MediaEngineRemoteVideoSource::Start().

https://reviewboard.mozilla.org/r/207166/#review213640

Looks good! Thanks.

::: dom/media/webrtc/MediaEngineCameraVideoSource.cpp:31
(Diff revision 3)
> +  // Before the first DeliverFrame() arrives, we need to keep sending
> +  // null image with size 0x0 to avoid blocking audio/video track.

nit: I think this is quite self-explained.
Attachment #8936454 - Flags: review?(apehrson) → review+
Pushed by mchiang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bb8aa824b37b
set mImages[i] to nullptr in MediaEngineRemoteVideoSource::Start(). r=pehrsons
https://hg.mozilla.org/mozilla-central/rev/bb8aa824b37b
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.