fix: PR feedback

include/packager/packager.h

@@ -75,6 +75,9 @@ struct PackagingParams {
 
   /// Only use to package init segment separately.
   bool init_segment_only = false;
+
+  /// Specify weather or not to enable null packet stuffing for TS segments.
+  bool enable_null_ts_packet_stuffing = false;
 };
 
 /// Defines a single input/output stream.

packager/app/muxer_factory.cc

@@ -24,7 +24,9 @@ MuxerFactory::MuxerFactory(const PackagingParams& packaging_params)
       temp_dir_(packaging_params.temp_dir),
       transport_stream_timestamp_offset_ms_(
           packaging_params.transport_stream_timestamp_offset_ms),
-      init_segment_only_(packaging_params.init_segment_only) {}
+      init_segment_only_(packaging_params.init_segment_only),
+      enable_null_ts_packet_stuffing_(
+          packaging_params.enable_null_ts_packet_stuffing) {}
 
 std::shared_ptr<Muxer> MuxerFactory::CreateMuxer(
     MediaContainerName output_format,
@@ -37,6 +39,7 @@ std::shared_ptr<Muxer> MuxerFactory::CreateMuxer(
   options.output_file_name = stream.output;
   options.segment_template = stream.segment_template;
   options.bandwidth = stream.bandwidth;
+  options.enable_null_ts_packet_stuffing = enable_null_ts_packet_stuffing_;
 
   std::shared_ptr<Muxer> muxer;
 

packager/app/muxer_factory.h

@@ -54,6 +54,9 @@ class MuxerFactory {
 
   // enable init segment packaging separately
   bool init_segment_only_;
+
+  // enable null TS packet stuffing
+  bool enable_null_ts_packet_stuffing_;
 };
 
 }  // namespace media

packager/live_packager.cc

@@ -330,6 +330,7 @@ Status LivePackager::Package(const Segment& init_segment,
   packaging_params.mp4_output_params.include_pssh_in_stream = false;
   packaging_params.transport_stream_timestamp_offset_ms =
       config_.m2ts_offset_ms;
+  packaging_params.enable_null_ts_packet_stuffing = true;
 
   EncryptionParams& encryption_params = packaging_params.encryption_params;
   // As a side effect of InitializeEncryption, encryption_params will be

packager/media/base/muxer_options.h

@@ -44,6 +44,9 @@ struct MuxerOptions {
   /// User-specified bit rate for the media stream. If zero, the muxer will
   /// attempt to estimate.
   uint32_t bandwidth = 0;
+
+  /// Specify weather or not to enable null packet stuffing for TS segments.
+  bool enable_null_ts_packet_stuffing = false;
 };
 
 }  // namespace media

packager/media/formats/mp2t/continuity_counter.cc

@@ -10,20 +10,19 @@ namespace shaka {
 namespace media {
 namespace mp2t {
 
-ContinuityCounter::ContinuityCounter() : ContinuityCounter(0) {}
-ContinuityCounter::ContinuityCounter(unsigned int segment_number)
+ContinuityCounter::ContinuityCounter(int segment_number)
     : counter_(segment_number & 0xF) {}
 
 ContinuityCounter::~ContinuityCounter() = default;
 
-unsigned int ContinuityCounter::GetNext() {
+int ContinuityCounter::GetNext() {
   unsigned int ret = counter_;
   ++counter_;
   counter_ %= 16;
   return ret;
 }
 
-unsigned int ContinuityCounter::GetCurrent() const {
+int ContinuityCounter::GetCurrent() const {
   return counter_;
 }
 

packager/media/formats/mp2t/continuity_counter.h

@@ -15,20 +15,19 @@ namespace mp2t {
 
 class ContinuityCounter {
  public:
-  ContinuityCounter(unsigned int segment_number);
-  ContinuityCounter();
+  ContinuityCounter(int segment_number = 0);
   ~ContinuityCounter();
 
   /// As specified by the spec, this starts from 0 and is incremented by 1 until
   /// it wraps back to 0 when it reaches 16.
   /// @return counter value.
-  unsigned int GetNext();
+  int GetNext();
 
   /// @return the current value of the continuity counter.
-  [[nodiscard]] unsigned int GetCurrent() const;
+  [[nodiscard]] int GetCurrent() const;
 
  private:
-  unsigned int counter_;
+  int counter_;
   DISALLOW_COPY_AND_ASSIGN(ContinuityCounter);
 };
 

packager/media/formats/mp2t/program_map_table_writer.cc

@@ -193,15 +193,12 @@ void WritePmtWithParameters(uint8_t stream_type,
 
 }  // namespace
 
-ProgramMapTableWriter::ProgramMapTableWriter(Codec codec)
-    : ProgramMapTableWriter(codec, 0) {}
-
 // use segment number as continuity counter as PMT types have single packets,
 // therefore, using the segment number as the CC will be continuous across
 // segments
 ProgramMapTableWriter::ProgramMapTableWriter(Codec codec,
                                              unsigned int segment_number)
-    : codec_(codec), continuity_counter_(segment_number) {}
+    : codec_(codec), continuity_counter_(static_cast<int>(segment_number)) {}
 
 bool ProgramMapTableWriter::EncryptedSegmentPmt(BufferWriter* writer) {
   if (encrypted_pmt_.Size() == 0) {
@@ -273,9 +270,6 @@ bool ProgramMapTableWriter::ClearSegmentPmt(BufferWriter* writer) {
   return true;
 }
 
-VideoProgramMapTableWriter::VideoProgramMapTableWriter(Codec codec)
-    : VideoProgramMapTableWriter(codec, 0) {}
-
 VideoProgramMapTableWriter::VideoProgramMapTableWriter(
     Codec codec,
     unsigned int segment_number)
@@ -296,11 +290,6 @@ bool VideoProgramMapTableWriter::WriteDescriptors(
   return true;
 }
 
-AudioProgramMapTableWriter::AudioProgramMapTableWriter(
-    Codec codec,
-    const std::vector<uint8_t>& audio_specific_config)
-    : AudioProgramMapTableWriter(codec, audio_specific_config, 0) {}
-
 AudioProgramMapTableWriter::AudioProgramMapTableWriter(
     Codec codec,
     const std::vector<uint8_t>& audio_specific_config,

packager/media/formats/mp2t/program_map_table_writer.h

@@ -25,8 +25,7 @@ namespace mp2t {
 /// Puts PMT into TS packets and writes them to buffer.
 class ProgramMapTableWriter {
  public:
-  ProgramMapTableWriter(Codec codec);
-  ProgramMapTableWriter(Codec codec, unsigned int segment_number);
+  explicit ProgramMapTableWriter(Codec codec, unsigned int segment_number = 0);
   virtual ~ProgramMapTableWriter() = default;
 
   /// Writes TS packets with PMT for encrypted segments.
@@ -64,8 +63,8 @@ class ProgramMapTableWriter {
 /// ProgramMapTableWriter for video codecs.
 class VideoProgramMapTableWriter : public ProgramMapTableWriter {
  public:
-  explicit VideoProgramMapTableWriter(Codec codec);
-  VideoProgramMapTableWriter(Codec codec, unsigned int segment_number);
+  explicit VideoProgramMapTableWriter(Codec codec,
+                                      unsigned int segment_number = 0);
   ~VideoProgramMapTableWriter() override = default;
 
  private:
@@ -79,11 +78,9 @@ class VideoProgramMapTableWriter : public ProgramMapTableWriter {
 /// ProgramMapTableWriter for video codecs.
 class AudioProgramMapTableWriter : public ProgramMapTableWriter {
  public:
-  AudioProgramMapTableWriter(Codec codec,
-                             const std::vector<uint8_t>& audio_specific_config);
   AudioProgramMapTableWriter(Codec codec,
                              const std::vector<uint8_t>& audio_specific_config,
-                             unsigned int segment_number);
+                             unsigned int segment_number = 0);
   ~AudioProgramMapTableWriter() override = default;
 
  private:

packager/media/formats/mp2t/ts_segmenter.cc

@@ -17,6 +17,7 @@
 #include <packager/media/event/muxer_listener.h>
 #include <packager/media/formats/mp2t/pes_packet.h>
 #include <packager/media/formats/mp2t/program_map_table_writer.h>
+#include <packager/media/formats/mp2t/ts_packet_writer_util.h>
 #include <packager/status.h>
 
 namespace shaka {
@@ -101,8 +102,7 @@ Status TsSegmenter::AddSample(const MediaSample& sample) {
       DCHECK(IsVideoCodec(codec_));
       pmt_writer.reset(new VideoProgramMapTableWriter(codec_, segment_number));
     }
-    ts_writer_.reset(
-        new TsStuffingWriter(std::move(pmt_writer), segment_number));
+    ts_writer_.reset(new TsWriter(std::move(pmt_writer), segment_number));
   }
 
   if (sample.is_encrypted())
@@ -175,6 +175,23 @@ Status TsSegmenter::FinalizeSegment(int64_t start_timestamp, int64_t duration) {
   if (!status.ok())
     return status;
 
+  if (muxer_options_.enable_null_ts_packet_stuffing) {
+    ContinuityCounter* es_continuity_counter =
+        ts_writer_->es_continuity_counter();
+    if (es_continuity_counter) {
+      do {
+        const int pid = ProgramMapTableWriter::kElementaryPid;
+        BufferWriter null_ts_packet_buffer;
+        // TODO(Fordyce): do the stuffing packets need a payload?
+        // null_ts_packet_buffer.AppendInt(static_cast<uint8_t>(TsSection::kPidNullPacket));
+        WritePayloadToBufferWriter(
+            null_ts_packet_buffer.Buffer(), null_ts_packet_buffer.Size(), false,
+            pid, false, 0, es_continuity_counter, &segment_buffer_);
+
+      } while ((es_continuity_counter->GetCurrent() & 0x0F) != 0);
+    }
+  }
+
   // This method may be called from Finalize() so segment_started_ could
   // be false.
   if (!segment_started_)

packager/media/formats/mp2t/ts_writer.cc

@@ -162,18 +162,12 @@ bool WritePesToBuffer(const PesPacket& pes,
 
 }  // namespace
 
-TsWriter::TsWriter(std::unique_ptr<ProgramMapTableWriter> pmt_writer)
-    : TsWriter(std::move(pmt_writer), 0) {}
-
 TsWriter::TsWriter(std::unique_ptr<ProgramMapTableWriter> pmt_writer,
                    unsigned int segment_number)
-    : pat_continuity_counter_(segment_number),
+    : pat_continuity_counter_(static_cast<int>(segment_number)),
       pmt_writer_(std::move(pmt_writer)) {}
 
-TsStuffingWriter::TsStuffingWriter(
-    std::unique_ptr<ProgramMapTableWriter> pmt_writer,
-    unsigned int segment_number)
-    : TsWriter(std::move(pmt_writer), segment_number) {}
+TsWriter::~TsWriter() = default;
 
 bool TsWriter::NewSegment(BufferWriter* buffer) {
   BufferWriter psi;
@@ -208,33 +202,6 @@ bool TsWriter::AddPesPacket(std::unique_ptr<PesPacket> pes_packet,
   return true;
 }
 
-bool TsStuffingWriter::AddPesPacket(std::unique_ptr<PesPacket> pes_packet,
-                                    BufferWriter* buffer) {
-  if (!WritePesToBuffer(*pes_packet, &elementary_stream_continuity_counter_,
-                        buffer)) {
-    LOG(ERROR) << "Failed to write pes to buffer.";
-    return false;
-  }
-
-  // We must end all ES packets at 0xf so that the next segment can start at
-  // 0x0. This can be done by stuffing null packets at the end of the segment
-  // for each elementary stream
-  do {
-    const int pid = ProgramMapTableWriter::kElementaryPid;
-    BufferWriter null_ts_packet_buffer;
-    // TODO(Fordyce): do the stuffing packets need a payload?
-    // null_ts_packet_buffer.AppendInt(static_cast<uint8_t>(TsSection::kPidNullPacket));
-    WritePayloadToBufferWriter(null_ts_packet_buffer.Buffer(),
-                               null_ts_packet_buffer.Size(),
-                               !kPayloadUnitStartIndicator, pid, !kHasPcr, 0,
-                               &elementary_stream_continuity_counter_, buffer);
-
-  } while ((elementary_stream_continuity_counter_.GetCurrent() & 0x0F) != 0);
-
-  // No need to keep pes_packet around so not passing it anywhere.
-  return true;
-}
-
 }  // namespace mp2t
 }  // namespace media
 }  // namespace shaka

packager/media/formats/mp2t/ts_writer.h

@@ -30,15 +30,13 @@ class ProgramMapTableWriter;
 /// the data to file. This also creates PSI from StreamInfo.
 class TsWriter {
  public:
-  TsWriter(std::unique_ptr<ProgramMapTableWriter> pmt_writer);
-
   /// Create TsWriter with segment_number
   /// @param pmt_writer the writes PMT into ts packets
   //// @param segment_number is used to set the continuity counter for PAT
   /// packets.
-  TsWriter(std::unique_ptr<ProgramMapTableWriter> pmt_writer,
-           unsigned int segment_number);
-  virtual ~TsWriter() = default;
+  explicit TsWriter(std::unique_ptr<ProgramMapTableWriter> pmt_writer,
+                    unsigned int segment_number = 0);
+  virtual ~TsWriter();
 
   /// This will fail if the current segment is not finalized.
   /// @param buffer to write segment data.
@@ -56,12 +54,9 @@ class TsWriter {
   virtual bool AddPesPacket(std::unique_ptr<PesPacket> pes_packet,
                             BufferWriter* buffer);
 
-  //  [[nodiscard]] ContinuityCounter es_continuity_counter() const {
-  //    return elementary_stream_continuity_counter_;
-  //  }
-
- protected:
-  ContinuityCounter elementary_stream_continuity_counter_;
+  ContinuityCounter* es_continuity_counter() {
+    return &elementary_stream_continuity_counter_;
+  }
 
  private:
   TsWriter(const TsWriter&) = delete;
@@ -71,20 +66,11 @@ class TsWriter {
   bool encrypted_ = false;
 
   ContinuityCounter pat_continuity_counter_;
+  ContinuityCounter elementary_stream_continuity_counter_;
 
   std::unique_ptr<ProgramMapTableWriter> pmt_writer_;
 };
 
-/// TsWriter to handle stuffing null TS Packets
-class TsStuffingWriter : public TsWriter {
- public:
-  TsStuffingWriter(std::unique_ptr<ProgramMapTableWriter> pmt_writer,
-                   unsigned int segment_number);
-
-  bool AddPesPacket(std::unique_ptr<PesPacket> pes_packet,
-                    BufferWriter* buffer) override;
-};
-
 }  // namespace mp2t
 }  // namespace media
 }  // namespace shaka