Ticket #11328: keep-first-keyframe-v3.patch

mythtv/libs/libmythtv/recorders/dtvrecorder.cpp

@@ -87 +87 @@
 {
     SetPositionMapType(MARK_GOP_BYFRAME);
     _payload_buffer.reserve(TSPacket::kSize * (50 + 1));
+
     ResetForNewFile();
+
+    memset(_stream_id,  0, sizeof(_stream_id));
+    memset(_pid_status, 0, sizeof(_pid_status));
+    memset(_continuity_counter, 0xff, sizeof(_continuity_counter));
 }
 
 DTVRecorder::~DTVRecorder()
@@ -147 +152 @@
 void DTVRecorder::FinishRecording(void)
 {
     if (ringBuffer)
-    {
-        if (!_payload_buffer.empty())
-        {
-            ringBuffer->Write(&_payload_buffer[0], _payload_buffer.size());
-            _payload_buffer.clear();
-        }
         ringBuffer->WriterFlush();
-    }
 
     if (curRecording)
     {
@@ -171 +169 @@
     LOG(VB_RECORD, LOG_INFO, LOC + "ResetForNewFile(void)");
     QMutexLocker locker(&positionMapLock);
 
-    // _first_keyframe, _seen_psp and m_h264_parser should
+    // _seen_psp and m_h264_parser should
     // not be reset here. This will only be called just as
     // we're seeing the first packet of a new keyframe for
     // writing to the new file and anything that makes the
@@ -180 +178 @@
     // -- Daniel Kristjansson 2011-02-26
 
     _start_code                 = 0xffffffff;
-    //_first_keyframe
+    _first_keyframe             = -1;
     _has_written_other_keyframe = false;
     _last_keyframe_seen         = 0;
     _last_gop_seen              = 0;
@@ -191 +189 @@
     //_recording
     _error                      = QString();
 
-    memset(_stream_id,  0, sizeof(_stream_id));
-    memset(_pid_status, 0, sizeof(_pid_status));
-    memset(_continuity_counter, 0xff, sizeof(_continuity_counter));
-
     _progressive_sequence       = 0;
     _repeat_pict                = 0;
 
-    _pes_synced                 = false;
+    //_pes_synced
     //_seen_sps
     positionMap.clear();
     positionMapDelta.clear();
     durationMap.clear();
     durationMapDelta.clear();
-    _payload_buffer.clear();
 
     locker.unlock();
     ClearStatistics();
@@ -215 +208 @@
     RecorderBase::ClearStatistics();
 
     memset(_ts_count, 0, sizeof(_ts_count));
-    for (int i = 0; i < 256; i++)
+    for (int i = 0; i < 256; ++i)
         _ts_last[i] = -1LL;
-    for (int i = 0; i < 256; i++)
+    for (int i = 0; i < 256; ++i)
         _ts_first[i] = -1LL;
     //_ts_first_dt -- doesn't need to be cleared only used if _ts_first>=0
     _packet_count.fetchAndStoreRelaxed(0);
@@ -277 +270 @@
 void DTVRecorder::BufferedWrite(const TSPacket &tspacket)
 {
     // delay until first GOP to avoid decoder crash on res change
-    if (_wait_for_keyframe_option && _first_keyframe<0)
+    if (!_buffer_packets && _wait_for_keyframe_option && _first_keyframe < 0)
         return;
 
     if (curRecording && timeOfFirstDataIsSet.testAndSetRelaxed(0,1))
@@ -403 +396 @@
  */
 bool DTVRecorder::FindMPEG2Keyframes(const TSPacket* tspacket)
 {
-    bool haveBufferedData = !_payload_buffer.empty();
     if (!tspacket->HasPayload()) // no payload to scan
-        return !haveBufferedData;
+        return _first_keyframe >= 0;
 
     if (!ringBuffer)
-        return !haveBufferedData;
+        return _first_keyframe >= 0;
 
     // if packet contains start of PES packet, start
     // looking for first byte of MPEG start code (3 bytes 0 0 1)
@@ -540 +532 @@
 
     if (hasKeyFrame)
     {
+#if 0
+        LOG(VB_RECORD, LOG_ERR, LOC + QString
+            ("Keyframe @ %1 + %2 = %3")
+            .arg(ringBuffer->GetWritePosition())
+            .arg(_payload_buffer.size())
+            .arg(ringBuffer->GetWritePosition() + _payload_buffer.size()));
+#endif
         _last_keyframe_seen = _frames_seen_count;
-        HandleKeyframe(_frames_written_count, TSPacket::kSize);
+        HandleKeyframe(0);
     }
 
     if (hasFrame)
     {
+#if 0
+        LOG(VB_RECORD, LOG_ERR, LOC + QString
+            ("Frame @ %1 + %2 = %3")
+            .arg(ringBuffer->GetWritePosition())
+            .arg(_payload_buffer.size())
+            .arg(ringBuffer->GetWritePosition() + _payload_buffer.size()));
+#endif
+        _buffer_packets = false;  // We know know if it is a keyframe, or not
         _frames_seen_count++;
-        if (!_wait_for_keyframe_option || _first_keyframe>=0)
+        if (!_wait_for_keyframe_option || _first_keyframe >= 0)
             UpdateFramesWritten();
+        else
+        {
+            /* Found a frame that is not a keyframe, and we want to
+             * start on a keyframe */
+            _payload_buffer.clear();
+        }
     }
 
     if ((aspectRatio > 0) && (aspectRatio != m_videoAspect))
@@ -572 +585 @@
         FrameRateChange(frameRate, _frames_written_count);
     }
 
-    return hasKeyFrame || (_payload_buffer.size() >= (188*50));
+    return _first_keyframe >= 0;
 }
 
 void DTVRecorder::HandleTimestamps(int stream_id, int64_t pts, int64_t dts)
@@ -680 +693 @@
         if (1 == (_frames_seen_count & 0x7))
         {
             _last_keyframe_seen = _frames_seen_count;
-            HandleKeyframe(_frames_written_count);
+            HandleKeyframe(_payload_buffer.size());
             hasKeyFrame = true;
         }
 
@@ -708 +721 @@
     UpdateFramesWritten();
     _last_keyframe_seen = _frames_seen_count;
 
-    HandleKeyframe(_frames_written_count);
+    HandleKeyframe(_payload_buffer.size());
 
     _has_written_other_keyframe = true;
 
@@ -719 +732 @@
  *  \brief This save the current frame to the position maps
  *         and handles ringbuffer switching.
  */
-void DTVRecorder::HandleKeyframe(uint64_t frameNum, int64_t extra)
+void DTVRecorder::HandleKeyframe(int64_t extra)
 {
     if (!ringBuffer)
         return;
 
-#if 0
-    unsigned long long frameNum = _frames_written_count;
-#endif
+    // Perform ringbuffer switch if needed.
+    CheckForRingBufferSwitch();
 
+    uint64_t frameNum = _frames_written_count;
     _first_keyframe = (_first_keyframe < 0) ? frameNum : _first_keyframe;
 
     // Add key frame to position map
     positionMapLock.lock();
     if (!positionMap.contains(frameNum))
     {
-        long long startpos = ringBuffer->GetWritePosition();
-        // FIXME: handle keyframes with start code spanning over two ts packets
-        startpos += _payload_buffer.size() - extra;
+        int64_t startpos = ringBuffer->GetWritePosition() + extra;
 
         // Don't put negative offsets into the database, they get munged into
         // MAX_INT64 - offset, which is an exceedingly large number, and
@@ -750 +761 @@
         }
     }
     positionMapLock.unlock();
-
-    // Perform ringbuffer switch if needed.
-    CheckForRingBufferSwitch();
 }
 
 /** \fn DTVRecorder::FindH264Keyframes(const TSPacket*)
@@ -762 +770 @@
  */
 bool DTVRecorder::FindH264Keyframes(const TSPacket *tspacket)
 {
+    if (!tspacket->HasPayload()) // no payload to scan
+        return _first_keyframe >= 0;
+
     if (!ringBuffer)
     {
         LOG(VB_GENERAL, LOG_ERR, LOC + "FindH264Keyframes: No ringbuffer");
-        return false;
+        return _first_keyframe >= 0;
     }
 
-    bool haveBufferedData = !_payload_buffer.empty();
-    if (!tspacket->HasPayload()) // no payload to scan
-        return !haveBufferedData;
-
     const bool payloadStart = tspacket->PayloadStart();
     if (payloadStart)
     {
@@ -790 +797 @@
 
     // scan for PES packets and H.264 NAL units
     uint i = tspacket->AFCOffset();
-    for (; i < TSPacket::kSize; i++)
+    for (; i < TSPacket::kSize; ++i)
     {
         // special handling required when a new PES packet begins
         if (payloadStart && !_pes_synced)
@@ -858 +865 @@
 
         // scan for a NAL unit start code
 
-        uint32_t bytes_used = m_h264_parser.addBytes(
-            tspacket->data() + i, TSPacket::kSize - i,
-            ringBuffer->GetWritePosition() + _payload_buffer.size()
-            );
+        uint32_t bytes_used = m_h264_parser.addBytes
+                              (tspacket->data() + i, TSPacket::kSize - i,
+                               ringBuffer->GetWritePosition());
         i += (bytes_used - 1);
 
         if (m_h264_parser.stateChanged())
@@ -879 +885 @@
                 frameRate = m_h264_parser.frameRate();
             }
         }
-    } // for (; i < TSPacket::kSize; i++)
+    } // for (; i < TSPacket::kSize; ++i)
 
     if (hasKeyFrame)
     {
+#if 0
+        LOG(VB_RECORD, LOG_ERR, LOC + QString
+            ("Keyframe @ %1 + %2 = %3 AU %4")
+            .arg(ringBuffer->GetWritePosition())
+            .arg(_payload_buffer.size())
+            .arg(ringBuffer->GetWritePosition() + _payload_buffer.size())
+            .arg(m_h264_parser.keyframeAUstreamOffset()));
+#endif
         _last_keyframe_seen = _frames_seen_count;
         HandleH264Keyframe();
     }
 
     if (hasFrame)
     {
+#if 0
+        LOG(VB_RECORD, LOG_ERR, LOC + QString
+            ("Frame @ %1 + %2 = %3 AU %4")
+            .arg(ringBuffer->GetWritePosition())
+            .arg(_payload_buffer.size())
+            .arg(ringBuffer->GetWritePosition() + _payload_buffer.size())
+            .arg(m_h264_parser.keyframeAUstreamOffset()));
+#endif
+        _buffer_packets = false;  // We now know if this is a keyframe
         _frames_seen_count++;
         if (!_wait_for_keyframe_option || _first_keyframe >= 0)
             UpdateFramesWritten();
+        else
+        {
+            /* Found a frame that is not a keyframe, and we want to
+             * start on a keyframe */
+            _payload_buffer.clear();
+        }
     }
 
     if ((aspectRatio > 0) && (aspectRatio != m_videoAspect))
@@ -909 +938 @@
 
     if (frameRate != 0 && frameRate != m_frameRate)
     {
-
         LOG(VB_RECORD, LOG_INFO, LOC +
             QString("FindH264Keyframes: timescale: %1, tick: %2, framerate: %3")
                       .arg( m_h264_parser.GetTimeScale() )
@@ -919 +947 @@
         FrameRateChange(frameRate, _frames_written_count);
     }
 
-    return hasKeyFrame || (_payload_buffer.size() >= (188*50));
+    return _seen_sps;
 }
 
 /** \fn DTVRecorder::HandleH264Keyframe(void)
@@ -928 +956 @@
  */
 void DTVRecorder::HandleH264Keyframe(void)
 {
-    unsigned long long frameNum = _frames_written_count;
+    // Perform ringbuffer switch if needed.
+    CheckForRingBufferSwitch();
 
-    _first_keyframe = (_first_keyframe < 0) ? frameNum : _first_keyframe;
+    uint64_t startpos;
+    uint64_t frameNum = _frames_written_count;
+
+    if (_first_keyframe < 0)
+    {
+        _first_keyframe = frameNum;
+        startpos = 0;
+    }
+    else
+        startpos = m_h264_parser.keyframeAUstreamOffset();
 
     // Add key frame to position map
     positionMapLock.lock();
     if (!positionMap.contains(frameNum))
     {
-        positionMapDelta[frameNum] = m_h264_parser.keyframeAUstreamOffset();
-        positionMap[frameNum]      = m_h264_parser.keyframeAUstreamOffset();
+        positionMapDelta[frameNum] = startpos;
+        positionMap[frameNum]      = startpos;
         durationMap[frameNum]      = _total_duration / 1000;
         durationMapDelta[frameNum] = _total_duration / 1000;
     }
     positionMapLock.unlock();
-
-    // Perform ringbuffer switch if needed.
-    CheckForRingBufferSwitch();
 }
 
 void DTVRecorder::FindPSKeyFrames(const uint8_t *buffer, uint len)
@@ -1065 +1100 @@
         if (hasKeyFrame)
         {
             _last_keyframe_seen = _frames_seen_count;
-            HandleKeyframe(_frames_written_count, bufptr - bufstart);
+            HandleKeyframe(_payload_buffer.size() - (bufptr - bufstart));
         }
 
         if ((aspectRatio > 0) && (aspectRatio != m_videoAspect))
@@ -1149 +1184 @@
 
     if (!pmtpid)
     {
-        LOG(VB_RECORD, LOG_ERR, LOC + "SetPAT(): "
-            "Ignoring PAT not containing our desired program...");
+        LOG(VB_RECORD, LOG_ERR, LOC +
+            QString("SetPAT(): Ignoring PAT not containing our desired "
+                    "program (%1)...").arg(progNum));
         return;
     }
 
@@ -1162 +1198 @@
     delete oldpat;
 
     // Listen for the other PMTs for faster channel switching
-    for (uint i = 0; _input_pat && (i < _input_pat->ProgramCount()); i++)
+    for (uint i = 0; _input_pat && (i < _input_pat->ProgramCount()); ++i)
     {
         uint pmt_pid = _input_pat->ProgramPID(i);
         if (!_stream_data->IsListeningPID(pmt_pid))
@@ -1183 +1219 @@
         QString sistandard = GetSIStandard();
 
         bool has_no_av = true;
-        for (uint i = 0; i < _input_pmt->StreamCount() && has_no_av; i++)
+        for (uint i = 0; i < _input_pmt->StreamCount() && has_no_av; ++i)
         {
             has_no_av &= !_input_pmt->IsVideo(i, sistandard);
             has_no_av &= !_input_pmt->IsAudio(i, sistandard);
@@ -1210 +1246 @@
     pat->tsheader()->SetContinuityCounter(next_cc);
     pat->GetAsTSPackets(_scratch, next_cc);
 
-    for (uint i = 0; i < _scratch.size(); i++)
+    for (uint i = 0; i < _scratch.size(); ++i)
         DTVRecorder::BufferedWrite(_scratch[i]);
 }
 
@@ -1223 +1259 @@
     }
 
     // collect stream types for H.264 (MPEG-4 AVC) keyframe detection
-    for (uint i = 0; i < pmt->StreamCount(); i++)
+    for (uint i = 0; i < pmt->StreamCount(); ++i)
         _stream_id[pmt->StreamPID(i)] = pmt->StreamType(i);
 
     if (!ringBuffer)
@@ -1233 +1269 @@
     pmt->tsheader()->SetContinuityCounter(next_cc);
     pmt->GetAsTSPackets(_scratch, next_cc);
 
-    for (uint i = 0; i < _scratch.size(); i++)
+    for (uint i = 0; i < _scratch.size(); ++i)
         DTVRecorder::BufferedWrite(_scratch[i]);
 }
 
@@ -1260 +1296 @@
     // Only create fake keyframe[s] if there are no audio/video streams
     if (_input_pmt && _has_no_av)
     {
-        _buffer_packets = !FindOtherKeyframes(&tspacket);
+        FindOtherKeyframes(&tspacket);
+        _buffer_packets = false;
     }
     else
     {
@@ -1284 +1321 @@
 
     uint streamType = _stream_id[tspacket.PID()];
 
-    // Check for keyframes and count frames
-    if (streamType == StreamID::H264Video)
+    if (tspacket.HasPayload() && tspacket.PayloadStart())
     {
-        _buffer_packets = !FindH264Keyframes(&tspacket);
-        if (_wait_for_keyframe_option && !_seen_sps)
-            return true;
+        // buffer packets until we know if this is a keyframe
+        _buffer_packets = true;
+#if 0
+        LOG(VB_RECORD, LOG_ERR, LOC + QString
+            ("ProcessVideoTSPacket PayloadStart @ %1 + %2 = %3")
+            .arg(ringBuffer->GetWritePosition())
+            .arg(_payload_buffer.size())
+            .arg(ringBuffer->GetWritePosition() + _payload_buffer.size()));
+#endif
     }
+
+    // Check for keyframes and count frames
+    if (streamType == StreamID::H264Video)
+        FindH264Keyframes(&tspacket);
+    else if (streamType != 0)
+        FindMPEG2Keyframes(&tspacket);
     else
-    {
-        _buffer_packets = !FindMPEG2Keyframes(&tspacket);
-    }
+        LOG(VB_RECORD, LOG_ERR, LOC +
+            "ProcessVideoTSPacket: unknown stream type!");
 
     return ProcessAVTSPacket(tspacket);
 }
@@ -1304 +1351 @@
     if (!ringBuffer)
         return true;
 
-    _buffer_packets = !FindAudioKeyframes(&tspacket);
+    if (tspacket.HasPayload() && tspacket.PayloadStart())
+    {
+        // buffer packets until we know if this is a keyframe
+        _buffer_packets = true;
+#if 0
+        LOG(VB_RECORD, LOG_ERR, LOC + QString
+            ("ProcessAudioTSPacket PayloadStart @ %1 + %2 = %3")
+            .arg(ringBuffer->GetWritePosition())
+            .arg(_payload_buffer.size())
+            .arg(ringBuffer->GetWritePosition() + _payload_buffer.size()));
+#endif
+    }
+
+    FindAudioKeyframes(&tspacket);
     return ProcessAVTSPacket(tspacket);
 }
 
 /// Common code for processing either audio or video packets
 bool DTVRecorder::ProcessAVTSPacket(const TSPacket &tspacket)
 {
+    // Sync recording start to first keyframe
+    if (!_buffer_packets && _wait_for_keyframe_option && _first_keyframe < 0)
+        return true;
+
     const uint pid = tspacket.PID();
 
     if (pid != 0x1fff)
@@ -1328 +1392 @@
                 .arg(erate,5,'f',2));
     }
 
-    // Sync recording start to first keyframe
-    if (_wait_for_keyframe_option && _first_keyframe < 0)
-        return true;
-
     // Sync streams to the first Payload Unit Start Indicator
     // _after_ first keyframe iff _wait_for_keyframe_option is true
     if (!(_pid_status[pid] & kPayloadStartSeen) && tspacket.HasPayload())

mythtv/libs/libmythtv/recorders/dtvrecorder.h

@@ -93 +93 @@
     void FinishRecording(void);
     void ResetForNewFile(void);
 
-    void HandleKeyframe(uint64_t frameNum, int64_t extra = 0);
+    void HandleKeyframe(int64_t extra);
     void HandleTimestamps(int stream_id, int64_t pts, int64_t dts);
     void UpdateFramesWritten(void);