From bd27bd0eacbc514ecacb221f44b1cc371d24ee1f Mon Sep 17 00:00:00 2001 From: Chih-Yu Huang Date: Thu, 29 Oct 2020 14:16:15 +0900 Subject: [PATCH] virtio: video: Only clear output resource after ProvidePictureBuffers() At CL:2494322, we moved the clearing output resources logic from handle_provide_picture_buffers() to clear_queue(). That CL only considered the resolution change case, but didn't handle the flush case correctly. When the userspace flush the decoding, it streamoff the output queue. But VDA doesn't request new set of buffers, ProvidePictureBuffers() won't be called. In this case, crosvm shouldn't clear the output resources. Crosvm only needs to clear the set of queued resource ids. Also, when VDA sends ProvidePictureBuffers() the first time, V4L2 output queue at the userspace is not streaming. So the userspace won't streamoff the output queue. In this case, we also don't need to clear the output resources. BUG=b:171860073 TEST=pass android.media.cts.MediaCodecTest#testDecodeAfterFlush TEST=pass android.media.cts.AdaptivePlaybackTest Change-Id: I2a425c1fd9e61322d92dc54930dd88242c964de3 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2505284 Commit-Queue: Chih-Yu Huang Tested-by: Chih-Yu Huang Tested-by: kokoro Reviewed-by: Alexandre Courbot Reviewed-by: Alex Lau --- devices/src/virtio/video/decoder/mod.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/devices/src/virtio/video/decoder/mod.rs b/devices/src/virtio/video/decoder/mod.rs index 52c85946ab..e2075f1fd2 100644 --- a/devices/src/virtio/video/decoder/mod.rs +++ b/devices/src/virtio/video/decoder/mod.rs @@ -165,6 +165,9 @@ struct Context { in_res: InputResources, out_res: OutputResources, + + // Set the flag if we need to clear output resource when the output queue is cleared next time. + is_clear_out_res_needed: bool, } impl Context { @@ -261,6 +264,12 @@ impl Context { // No need to set `frame_rate`, as it's only for the encoder. ..Default::default() }; + + // That eos_resource_id has value means there are previous output resources. + // Clear the output resources when the output queue is cleared next time. + if self.out_res.eos_resource_id.is_some() { + self.is_clear_out_res_needed = true; + } } fn handle_picture_ready( @@ -742,7 +751,11 @@ impl<'a> Decoder<'a> { session.reset().map_err(VideoError::VdaError)?; } QueueType::Output => { - ctx.out_res = Default::default(); + if std::mem::replace(&mut ctx.is_clear_out_res_needed, false) { + ctx.out_res = Default::default(); + } else { + ctx.out_res.queued_res_ids.clear(); + } } } Ok(())