zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix race condition in cleanup of old frame records.

Browse Source 
VideoEncoderWrapper may be released and reused (Release() followed by
InitEncode()). This often happens back to back when encoders are
reconfigured. Because encoded frames are posted asynchronously to the
encoder queue, they may be processed after the encoder associated with
them has already been released.

In the existing code, if a frame for the new encoder had already been
received, the processing of the frame for the old encoder would clear
out the record for the new encoder's frame. This is now fixed by only
clearing out records that are older than the encoded frame being
processed.

A particularly bad symptom is when the new encoder is used for the same
stream as the old one (but was reconfigured for e.g. a change in
resolution). In that case, the new encoder's initial keyframe gets
dropped, and all subsequent difference frames are based off the last
sent frame from the old encoder. This all renders as garbage until a new
keyframe is sent.

Bug: webrtc:8849
Change-Id: I25094f12b38e03e158dc10ac66e92aa9ebaa5541
Reviewed-on: https://webrtc-review.googlesource.com/47549
Commit-Queue: Jonathan Yu <yujo@chromium.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21896}
This commit is contained in:
Jonathan Yu
2018-02-02 12:44:33 -08:00
committed by Commit Bot
parent eb0df088ca
commit 7092368982
1 changed files with 22 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
sdk/android/src/jni/videoencoderwrapper.cc
View File

@ -208,19 +208,29 @@ void VideoEncoderWrapper::OnEncodedFrame(JNIEnv* jni,
EncodedImageCallback* callback;
void operator()() const {
FrameExtraInfo frame_extra_info;
do {
if (frame_extra_infos->empty()) {
// Encoded frames are delivered in the order received, but some of them
// may be dropped, so remove records of frames older than the current one.
//
// NOTE: if the current frame is associated with Encoder A, in the time
// since this frame was received, Encoder A could have been Release()'ed,
// Encoder B InitEncode()'ed (due to reuse of Encoder A), and frames
// received by Encoder B. Thus there may be frame_extra_infos entries that
// don't belong to us, and we need to be careful not to remove them.
// Removing only those entries older than the current frame provides this
// guarantee.
while (!frame_extra_infos->empty() &&
frame_extra_infos->front().capture_time_ns < capture_time_ns) {
frame_extra_infos->pop_front();
}
if (frame_extra_infos->empty() ||
frame_extra_infos->front().capture_time_ns != capture_time_ns) {
RTC_LOG(LS_WARNING)
<< "Java encoder produced an unexpected frame with timestamp: "
<< capture_time_ns;
return;
}
frame_extra_info = frame_extra_infos->front();
FrameExtraInfo frame_extra_info = std::move(frame_extra_infos->front());
frame_extra_infos->pop_front();
// The encoder might drop frames so iterate through the queue until
// we find a matching timestamp.
} while (frame_extra_info.capture_time_ns != capture_time_ns);
RTPFragmentationHeader header =
video_encoder_wrapper->ParseFragmentationHeader(task_buffer);