Closed Bug 903758 Opened 8 years ago Closed 8 years ago
Media Recording - setting the recorder state to in
Active on the stop method
The UA may call other function while recorder is stopping, that may cause unexpect problem. Also follow by spec suggestion.
Summary: Media Recording - setting the recorder state to invalid on the stop method → Media Recording - setting the recorder state to inActive on the stop method
Assignee: nobody → rlin
My bad, I think we may need to set to inactive first because other methods would check this state first and block the function call, for ex UA call the stop then call reqeustdata on other worker or something else.
Attachment #788513 - Flags: review?(roc)
Comment on attachment 788513 [details] [diff] [review] patch v1 Review of attachment 788513 [details] [diff] [review]: ----------------------------------------------------------------- Needs a test!
Attachment #788513 - Flags: review?(roc) → review+
Sure, right away~
test case for this one.
(In reply to Randy Lin [:rlin] from comment #4) > Created attachment 788523 [details] [diff] [review] > test case > > test case for this one. I've actually already got a patch coming in bug 903781 that will cover this workflow (I confirmed it triggers this bug). I forgot to put the patch on the bug until now though (sorry about that). If you've confirmed the patch here fixes the issue manually, can we land it and use the patch from bug 903781?
Comment on attachment 788523 [details] [diff] [review] test case Sure, the bug 903781 test case can cover this one. try server result https://tbpl.mozilla.org/?tree=Try&rev=61e014bb9904
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
The other try run on a separate bug shows that the mochitests do not trigger this bug with this patch, so marking as verified.
Status: RESOLVED → VERIFIED
Is this the correct fix? Setting the recording state to inactive at the call of Stop() immediately stops the while loop in ExtractEncodedData(), while the encoder might haven't finished encoding yet. Please see: http://dxr.mozilla.org/mozilla-central/source/content/media/MediaRecorder.cpp#l157 or maybe we should just do: } while (!mEncoder->IsShutdown()); Regardless of this fix, I think the approach brought out by Rob in bug 899953 comment 5 might be a better solution for the long term.
Sorry, should be bug 899935 comment 5.
MediaRecorder::ExtractEncodedData should not be reading mState off the main thread. That's the real problem here!
ya, we will remove mState check on encodedThread asap.
(In reply to Randy Lin [:rlin] from comment #14) > ya, we will remove mState check on encodedThread asap. Can we get a bug on file for this?
Yes, bug 899935 covers the problem here.
Component: Video/Audio → Video/Audio: Recording
You need to log in before you can comment on or make changes to this bug.