From 40700d27f9a242c7854d5f30bedb4d0a4682d54b Mon Sep 17 00:00:00 2001
From: Zyta Szpak <zyta@google.com>
Date: Thu, 18 May 2023 15:14:03 +0000
Subject: [PATCH] Reland: "V4L2Decoder: Check if waiting for DRC needed""
This reverts commit c69b69e501cc928a7bcb1867f26ae9423744c709.
Reason for revert: this original commit revealed another bug
that got fixed in Chrome and verified by security tests. It
can now be safely relanded.
Bug: 280853786
Bug: 274864105
Test: android.media.cts.DecoderTest#testEOSBehaviorVP9
Change-Id: I6de423c5b8e6b9d11208d71d279b8ae31cf73f2d
(cherry picked from commit ea9b7f077241a75f461f73e292298a6797e72276)
---
common/H264NalParser.cpp | 10 +++++
common/HEVCNalParser.cpp | 10 +++++
.../v4l2_codec2/common/H264NalParser.h | 1 +
.../v4l2_codec2/common/HEVCNalParser.h | 2 +
common/include/v4l2_codec2/common/NalParser.h | 1 +
v4l2/V4L2Decoder.cpp | 44 +++++++++++++++++--
v4l2/include/v4l2_codec2/v4l2/V4L2Decoder.h | 3 ++
7 files changed, 68 insertions(+), 3 deletions(-)
diff --git a/common/H264NalParser.cpp b/common/H264NalParser.cpp
index 69a3f11..d2949c5 100644
--- a/common/H264NalParser.cpp
+++ b/common/H264NalParser.cpp
@@ -81,6 +81,16 @@ bool H264NalParser::locateSPS() {
return false;
}
+bool H264NalParser::locateIDR() {
+ while (locateNextNal()) {
+ if (length() == 0) continue;
+ if (type() != kIDRType) continue;
+ return true;
+ }
+
+ return false;
+}
+
uint8_t H264NalParser::type() const {
// First byte is forbidden_zero_bit (1) + nal_ref_idc (2) + nal_unit_type (5)
constexpr uint8_t kNALTypeMask = 0x1f;
diff --git a/common/HEVCNalParser.cpp b/common/HEVCNalParser.cpp
index 0499e70..89c67f7 100644
--- a/common/HEVCNalParser.cpp
+++ b/common/HEVCNalParser.cpp
@@ -202,6 +202,16 @@ bool HEVCNalParser::locateSPS() {
return false;
}
+bool HEVCNalParser::locateIDR() {
+ while (locateNextNal()) {
+ if (length() == 0) continue;
+ if (type() != kIDRType) continue;
+ return true;
+ }
+
+ return false;
+}
+
uint8_t HEVCNalParser::type() const {
// First bit is forbidden_zero_bit, next 6 are nal_unit_type
constexpr uint8_t kNALTypeMask = 0x7e;
diff --git a/common/include/v4l2_codec2/common/H264NalParser.h b/common/include/v4l2_codec2/common/H264NalParser.h
index 075411d..125efd8 100644
--- a/common/include/v4l2_codec2/common/H264NalParser.h
+++ b/common/include/v4l2_codec2/common/H264NalParser.h
@@ -26,6 +26,7 @@ public:
// Locate the sequence parameter set (SPS).
bool locateSPS() override;
+ bool locateIDR() override;
// Get the type of the current NAL unit.
uint8_t type() const;
diff --git a/common/include/v4l2_codec2/common/HEVCNalParser.h b/common/include/v4l2_codec2/common/HEVCNalParser.h
index 567115c..0f4574a 100644
--- a/common/include/v4l2_codec2/common/HEVCNalParser.h
+++ b/common/include/v4l2_codec2/common/HEVCNalParser.h
@@ -15,6 +15,7 @@ namespace android {
class HEVCNalParser : public NalParser {
public:
// Type of a SPS NAL unit.
+ static constexpr uint8_t kIDRType = 19; //IDR_W_RADL
static constexpr uint8_t kSPSType = 33;
HEVCNalParser(const uint8_t* data, size_t length);
@@ -22,6 +23,7 @@ public:
// Locate the sequence parameter set (SPS).
bool locateSPS() override;
+ bool locateIDR() override;
// Get the type of the current NAL unit.
uint8_t type() const override;
diff --git a/common/include/v4l2_codec2/common/NalParser.h b/common/include/v4l2_codec2/common/NalParser.h
index ad85a5f..50032d3 100644
--- a/common/include/v4l2_codec2/common/NalParser.h
+++ b/common/include/v4l2_codec2/common/NalParser.h
@@ -35,6 +35,7 @@ public:
// Locate the sequence parameter set (SPS).
virtual bool locateSPS() = 0;
+ virtual bool locateIDR() = 0;
// Gets current NAL data (start code is not included).
const uint8_t* data() const;
diff --git a/v4l2/V4L2Decoder.cpp b/v4l2/V4L2Decoder.cpp
index a852d93..e10a1ef 100644
--- a/v4l2/V4L2Decoder.cpp
+++ b/v4l2/V4L2Decoder.cpp
@@ -21,6 +21,8 @@
#include <v4l2_codec2/common/Common.h>
#include <v4l2_codec2/common/Fourcc.h>
+#include <v4l2_codec2/common/H264NalParser.h>
+#include <v4l2_codec2/common/HEVCNalParser.h>
#include <v4l2_codec2/plugin_store/DmabufHelpers.h>
namespace android {
@@ -29,6 +31,36 @@ namespace {
// Extra buffers for transmitting in the whole video pipeline.
constexpr size_t kNumExtraOutputBuffers = 4;
+bool waitForDRC(const C2ConstLinearBlock& input, std::optional<VideoCodec> codec) {
+ C2ReadView view = input.map().get();
+ const uint8_t* pos = view.data();
+ // frame type takes the (2) position in first byte of VP9 uncompressed header
+ const uint8_t kVP9FrameTypeMask = 0x4;
+ // frame type takes the (0) position in first byte of VP8 uncompressed header
+ const uint8_t kVP8FrameTypeMask = 0x1;
+
+ switch (*codec) {
+ case VideoCodec::H264: {
+ H264NalParser parser(view.data(), view.capacity());
+ return parser.locateIDR();
+ }
+ case VideoCodec::HEVC: {
+ HEVCNalParser parser(view.data(), view.capacity());
+ return parser.locateIDR();
+ }
+ // For VP8 and VP9 it is assumed that the input buffer contains a single
+ // frame that is not fragmented.
+ case VideoCodec::VP9:
+ // 0 - key frame; 1 - interframe
+ return ((pos[0] & kVP9FrameTypeMask) == 0);
+ case VideoCodec::VP8:
+ // 0 - key frame; 1 - interframe;
+ return ((pos[0] & kVP8FrameTypeMask) == 0);
+ }
+
+ return false;
+}
+
} // namespace
// static
@@ -91,6 +123,7 @@ bool V4L2Decoder::start(const VideoCodec& codec, const size_t inputBufferSize,
mGetPoolCb = std::move(getPoolCb);
mOutputCb = std::move(outputCb);
mErrorCb = std::move(errorCb);
+ mCodec = codec;
if (mState == State::Error) {
ALOGE("Ignore due to error state.");
@@ -326,6 +359,8 @@ void V4L2Decoder::decode(std::unique_ptr<ConstBitstreamBuffer> buffer, DecodeCB
setState(State::Decoding);
}
+ if (mInitialEosBuffer && !mPendingDRC) mPendingDRC = waitForDRC(buffer->dmabuf, mCodec);
+
mDecodeRequests.push(DecodeRequest(std::move(buffer), std::move(decodeCb)));
pumpDecodeRequest();
}
@@ -397,9 +432,12 @@ void V4L2Decoder::pumpDecodeRequest() {
// There is one more case that EOS frame cannot be dequeued because
// the first resolution change event wasn't dequeued before - output
// queues on the host are not streaming but ARCVM has no knowledge about
- // it. Check if first resolution change event was received and finish
- // drain now if it wasn't.
- if (mInitialEosBuffer) {
+ // it. Check if first resolution change event was received and if there
+ // was no previously sent non-empty frame (other than SPS/PPS/EOS) that
+ // may trigger config from host side.
+ // Drain can only be finished if we are sure there was no stream = no
+ // single frame in the stack.
+ if (mInitialEosBuffer && !mPendingDRC) {
ALOGV("Terminate drain, because there was no stream");
mTaskRunner->PostTask(FROM_HERE, ::base::BindOnce(std::move(request.decodeCb),
VideoDecoder::DecodeStatus::kOk));
diff --git a/v4l2/include/v4l2_codec2/v4l2/V4L2Decoder.h b/v4l2/include/v4l2_codec2/v4l2/V4L2Decoder.h
index fcfeb10..839574f 100644
--- a/v4l2/include/v4l2_codec2/v4l2/V4L2Decoder.h
+++ b/v4l2/include/v4l2_codec2/v4l2/V4L2Decoder.h
@@ -113,6 +113,9 @@ private:
std::queue<DecodeRequest> mDecodeRequests;
std::map<int32_t, DecodeCB> mPendingDecodeCbs;
+ // Marks that we need to wait for DRC before drain can complete.
+ bool mPendingDRC = false;
+ VideoCodec mCodec;
// Tracks the last DMA buffer ID which was used for a given V4L2 input
// buffer ID. Used to try to avoid re-importing buffers.
--
GitLab