From 2eb32ee177c3470d2d1946d05106a9eb4662f09a Mon Sep 17 00:00:00 2001
From: Jacob Trimble <modmaker@google.com>
Date: Thu, 10 Dec 2020 15:03:41 -0800
Subject: [PATCH] Propagate Flush errors in MP2T parser.

Issue #832

Change-Id: I59f31ff491437b81ffc22ab5760ad0c059e9933e
---
 packager/media/formats/mp2t/es_parser.h          | 2 +-
 packager/media/formats/mp2t/es_parser_audio.cc   | 4 +++-
 packager/media/formats/mp2t/es_parser_audio.h    | 2 +-
 packager/media/formats/mp2t/es_parser_h26x.cc    | 5 +++--
 packager/media/formats/mp2t/es_parser_h26x.h     | 2 +-
 packager/media/formats/mp2t/mp2t_media_parser.cc | 9 +++++----
 packager/media/formats/mp2t/ts_section.h         | 2 +-
 packager/media/formats/mp2t/ts_section_pes.cc    | 6 +++---
 packager/media/formats/mp2t/ts_section_pes.h     | 2 +-
 packager/media/formats/mp2t/ts_section_psi.cc    | 3 ++-
 packager/media/formats/mp2t/ts_section_psi.h     | 2 +-
 11 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/packager/media/formats/mp2t/es_parser.h b/packager/media/formats/mp2t/es_parser.h
index 4d377a2118..c71709d527 100644
--- a/packager/media/formats/mp2t/es_parser.h
+++ b/packager/media/formats/mp2t/es_parser.h
@@ -35,7 +35,7 @@ class EsParser {
                      int64_t dts) = 0;
 
   // Flush any pending buffer.
-  virtual void Flush() = 0;
+  virtual bool Flush() = 0;
 
   // Reset the state of the ES parser.
   virtual void Reset() = 0;
diff --git a/packager/media/formats/mp2t/es_parser_audio.cc b/packager/media/formats/mp2t/es_parser_audio.cc
index ceb65f4bc9..f400ef7e26 100644
--- a/packager/media/formats/mp2t/es_parser_audio.cc
+++ b/packager/media/formats/mp2t/es_parser_audio.cc
@@ -182,7 +182,9 @@ bool EsParserAudio::Parse(const uint8_t* buf,
   return true;
 }
 
-void EsParserAudio::Flush() {}
+bool EsParserAudio::Flush() {
+  return true;
+}
 
 void EsParserAudio::Reset() {
   es_byte_queue_.Reset();
diff --git a/packager/media/formats/mp2t/es_parser_audio.h b/packager/media/formats/mp2t/es_parser_audio.h
index e92a9fefb0..97c8f39474 100644
--- a/packager/media/formats/mp2t/es_parser_audio.h
+++ b/packager/media/formats/mp2t/es_parser_audio.h
@@ -36,7 +36,7 @@ class EsParserAudio : public EsParser {
 
   // EsParser implementation.
   bool Parse(const uint8_t* buf, int size, int64_t pts, int64_t dts) override;
-  void Flush() override;
+  bool Flush() override;
   void Reset() override;
 
  private:
diff --git a/packager/media/formats/mp2t/es_parser_h26x.cc b/packager/media/formats/mp2t/es_parser_h26x.cc
index bf3838a264..92a8d35942 100644
--- a/packager/media/formats/mp2t/es_parser_h26x.cc
+++ b/packager/media/formats/mp2t/es_parser_h26x.cc
@@ -77,7 +77,7 @@ bool EsParserH26x::Parse(const uint8_t* buf,
   return ParseInternal();
 }
 
-void EsParserH26x::Flush() {
+bool EsParserH26x::Flush() {
   DVLOG(1) << "EsParserH26x::Flush";
 
   // Simulate two additional AUDs to force emitting the last access unit
@@ -95,7 +95,7 @@ void EsParserH26x::Flush() {
     es_queue_->Push(aud, sizeof(aud));
   }
 
-  CHECK(ParseInternal());
+  RCHECK(ParseInternal());
 
   if (pending_sample_) {
     // Flush pending sample.
@@ -103,6 +103,7 @@ void EsParserH26x::Flush() {
     pending_sample_->set_duration(pending_sample_duration_);
     emit_sample_cb_.Run(std::move(pending_sample_));
   }
+  return true;
 }
 
 void EsParserH26x::Reset() {
diff --git a/packager/media/formats/mp2t/es_parser_h26x.h b/packager/media/formats/mp2t/es_parser_h26x.h
index 124e2cdb43..26c85248a7 100644
--- a/packager/media/formats/mp2t/es_parser_h26x.h
+++ b/packager/media/formats/mp2t/es_parser_h26x.h
@@ -35,7 +35,7 @@ class EsParserH26x : public EsParser {
 
   // EsParser implementation overrides.
   bool Parse(const uint8_t* buf, int size, int64_t pts, int64_t dts) override;
-  void Flush() override;
+  bool Flush() override;
   void Reset() override;
 
  protected:
diff --git a/packager/media/formats/mp2t/mp2t_media_parser.cc b/packager/media/formats/mp2t/mp2t_media_parser.cc
index 665f25a517..894ecb31c9 100644
--- a/packager/media/formats/mp2t/mp2t_media_parser.cc
+++ b/packager/media/formats/mp2t/mp2t_media_parser.cc
@@ -45,7 +45,7 @@ class PidState {
 
   // Flush the PID state (possibly emitting some pending frames)
   // and reset its state.
-  void Flush();
+  bool Flush();
 
   // Enable/disable the PID.
   // Disabling a PID will reset its state and ignore any further incoming TS
@@ -119,9 +119,10 @@ bool PidState::PushTsPacket(const TsPacket& ts_packet) {
   return status;
 }
 
-void PidState::Flush() {
-  section_parser_->Flush();
+bool PidState::Flush() {
+  RCHECK(section_parser_->Flush());
   ResetState();
+  return true;
 }
 
 void PidState::Enable() {
@@ -174,7 +175,7 @@ bool Mp2tMediaParser::Flush() {
   for (const auto& pair : pids_) {
     DVLOG(1) << "Flushing PID: " << pair.first;
     PidState* pid_state = pair.second.get();
-    pid_state->Flush();
+    RCHECK(pid_state->Flush());
   }
   bool result = EmitRemainingSamples();
   pids_.clear();
diff --git a/packager/media/formats/mp2t/ts_section.h b/packager/media/formats/mp2t/ts_section.h
index 88bf084218..433e2792b4 100644
--- a/packager/media/formats/mp2t/ts_section.h
+++ b/packager/media/formats/mp2t/ts_section.h
@@ -30,7 +30,7 @@ class TsSection {
 
   // Process bytes that have not been processed yet (pending buffers in the
   // pipe). Flush might thus results in frame emission, as an example.
-  virtual void Flush() = 0;
+  virtual bool Flush() = 0;
 
   // Reset the state of the parser to its initial state.
   virtual void Reset() = 0;
diff --git a/packager/media/formats/mp2t/ts_section_pes.cc b/packager/media/formats/mp2t/ts_section_pes.cc
index ac93dcf6a2..31f799cb11 100644
--- a/packager/media/formats/mp2t/ts_section_pes.cc
+++ b/packager/media/formats/mp2t/ts_section_pes.cc
@@ -128,13 +128,13 @@ bool TsSectionPes::Parse(bool payload_unit_start_indicator,
   return (parse_result && Emit(false));
 }
 
-void TsSectionPes::Flush() {
+bool TsSectionPes::Flush() {
   // Try emitting a packet since we might have a pending PES packet
   // with an undefined size.
-  Emit(true);
+  RCHECK(Emit(true));
 
   // Flush the underlying ES parser.
-  es_parser_->Flush();
+  return es_parser_->Flush();
 }
 
 void TsSectionPes::Reset() {
diff --git a/packager/media/formats/mp2t/ts_section_pes.h b/packager/media/formats/mp2t/ts_section_pes.h
index 7ece99d58f..110d0180ab 100644
--- a/packager/media/formats/mp2t/ts_section_pes.h
+++ b/packager/media/formats/mp2t/ts_section_pes.h
@@ -26,7 +26,7 @@ class TsSectionPes : public TsSection {
   bool Parse(bool payload_unit_start_indicator,
              const uint8_t* buf,
              int size) override;
-  void Flush() override;
+  bool Flush() override;
   void Reset() override;
 
  private:
diff --git a/packager/media/formats/mp2t/ts_section_psi.cc b/packager/media/formats/mp2t/ts_section_psi.cc
index d5a293cf71..2057f3a308 100644
--- a/packager/media/formats/mp2t/ts_section_psi.cc
+++ b/packager/media/formats/mp2t/ts_section_psi.cc
@@ -118,7 +118,8 @@ bool TsSectionPsi::Parse(bool payload_unit_start_indicator,
   return status;
 }
 
-void TsSectionPsi::Flush() {
+bool TsSectionPsi::Flush() {
+  return true;
 }
 
 void TsSectionPsi::Reset() {
diff --git a/packager/media/formats/mp2t/ts_section_psi.h b/packager/media/formats/mp2t/ts_section_psi.h
index 83280d8512..b4cac26e4e 100644
--- a/packager/media/formats/mp2t/ts_section_psi.h
+++ b/packager/media/formats/mp2t/ts_section_psi.h
@@ -25,7 +25,7 @@ class TsSectionPsi : public TsSection {
   bool Parse(bool payload_unit_start_indicator,
              const uint8_t* buf,
              int size) override;
-  void Flush() override;
+  bool Flush() override;
   void Reset() override;
 
   // Parse the content of the PSI section.