Closed Bug 1429390 Opened 3 years ago Closed 3 years ago
.264 Encode dispatch asynchronous
When encoding  H.264 video we make synchronous thread dispatches. The historical reasons for doing so no longer seem to apply, as the frame buffer is now kept in a refptr. I've caught a deadlock on shutdown which seems to be caused by this, where the main thread is waiting on the EncoderQueue thread which is waiting on a sync dispatch to the GMPThread which is waiting on a sync dispatch to the main thread. This seems to happen when we are asked to encode a frame during shutdown. There is an existing moderate frequency intermittent (Bug 1375540) that has similar symptoms, but there is not enough information in the logs to say for sure whether it is the same problem. Decoding H.264 also makes a synchronous dispatch  but the frame buffer there is kept as a raw pointer, so we would still be stuck copying data if we were to make it asynchronous.  https://searchfox.org/mozilla-central/rev/cf149b7b63ff97023e28723167725e38cf5df757/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp#319  https://searchfox.org/mozilla-central/rev/cf149b7b63ff97023e28723167725e38cf5df757/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp#809
Try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1be8c70c96d9fc42ae14cb47df6975ba7dd2ccac
Status: NEW → ASSIGNED
Comment on attachment 8942319 [details] [diff] [review] Call encode asynchronously Review of attachment 8942319 [details] [diff] [review]: ----------------------------------------------------------------- Provisional r+ if copying aInputImage is safe; please verify that and document ::: media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp @@ +322,5 @@ > + > + mGMPThread->Dispatch(WrapRunnableNM(&WebrtcGmpVideoEncoder::Encode_g, > + RefPtr<WebrtcGmpVideoEncoder>(this), > + aInputImage, > + *aFrameTypes), See comment below; need to note that both aInputImage and aFrameTypes are being copied here (and copying the vector will allocate) @@ +359,5 @@ > } > > +void > +WebrtcGmpVideoEncoder::Encode_g(RefPtr<WebrtcGmpVideoEncoder>& aEncoder, > + webrtc::VideoFrame aInputImage, You're copying aInputImage; I presume that has a well-defined safe lifetime for this async encode for the Image and everything in it? Please document this in the comments (probably at the Dispatch, and a reference here back to that).
Attachment #8942319 - Flags: review?(rjesup) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4b3b45da65d7 Make H.264 Encode dispatch asynchronous; r=jesup
You need to log in before you can comment on or make changes to this bug.