Change `AppendAndConsumeChunk`'s input type from `AudioChunk*` to `AudioChunk&&`
Categories
(Core :: Audio/Video: MediaStreamGraph, enhancement, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox91 | --- | fixed |
People
(Reporter: chunmin, Assigned: chunmin)
Details
Attachments
(1 file)
It's better to change the AppendAndConsumeChunk's input parameter type from AudioChunk* to AudioChunk&& since the AudioChunk is consumed in AppendAndConsumeChunk. That is, AudioChunk* AppendAndConsumeChunk(AudioChunk* aChunk) should be replaced by AudioChunk* AppendAndConsumeChunk(AudioChunk&& aChunk). As a result,
AudioChunk chunk = ...
audioSegment.AppendAndConsumeChunk(&chunk);
// chunk is consumed and it should not be used again
will become
AudioChunk chunk = ...
audioSegment.AppendAndConsumeChunk(std::move(chunk));
or
audioSegment.AppendAndConsumeChunk(CreateAudioChunk(...));
// CreateAudioChunk is a function returning AudioChunk
One benefit for doing so is to prevent the consumed AudioChunk from being used again after it's moved. Firefox has a clang-tidy check, bugprone-use-after-move , to catch this kind of error. We should utilize this clang-tidy check instead of catching this error by human eyes.
|
Assignee | |
Comment 1•3 years ago
|
||
It's better to change the input parameter's type of
AppendAndConsumeChunk from AudioChunk* to AudioChunk&& since the
AudioChunk will be consumed once it's fed to this function.
One benefit for doing so is to prevent the consumed AudioChunk from
being used again after it's moved/consumed. Gecko has a clang-tidy
check, bugprone-use-after-move [1], to avoid this kind of error. We
should utilize this check instead of catching used-after-move error by
human eyes.
Pushed by cchang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/da97faf315ee Change AppendAndConsumeChunk's input parameter type r=padenot
|
||
Comment 3•3 years ago
|
||
Backed out for causing Gtest failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/48d477ba4f9e332c47aaed2de90264d3c8173d41
Failure log: https://treeherder.mozilla.org/logviewer?job_id=343428959&repo=autoland&lineNumber=29965
|
|
Assignee | |
Comment 4•3 years ago
|
||
(In reply to Atila Butkovits from comment #3)
Backed out for causing Gtest failures.
The updated D117714 should be able to fix the issue
Pushed by cchang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e9b1ff295960 Change AppendAndConsumeChunk's input parameter type r=padenot
|
||
Comment 6•3 years ago
|
||
| bugherder | ||
Description
•