Handle multiple PES packets in StillPicture()

Author: fwolter

pes.cpp

@@ -176,3 +176,28 @@ int cPes::GetPayloadSize()
 {
 	return m_size - (GetPayload() - m_data);
 }
+
+/**
+ * Get the total length of the PES packet
+ *
+ * Returns the complete size of the PES packet including both header and payload.
+ * The length is read from the PES packet header (bytes 4-5).
+ *
+ * For packets with a specified length field (common for audio):
+ *   - Returns the actual PES packet length from the header
+ *   - Calculated as 6 + length_field (per H.222.0 standard)
+ *   - The length_field specifies bytes after the 6-byte header prefix
+ *     (3 bytes start code + 1 byte stream ID + 2 bytes length field)
+ *
+ * For unbounded packets (length field = 0, common for video streams):
+ *   - Returns the input buffer size (m_size)
+ *
+ * @return Total size in bytes: actual packet length if specified, otherwise input buffer size
+ */
+int cPes::GetPacketLength()
+{
+	if (!PesHasLength(m_data))
+		return m_size; // Length field is 0, meaning unbounded/unspecified. Return raw data size.
+
+	return PesLength(m_data);
+}

pes.h

@@ -58,6 +58,7 @@ class cPes
 	int64_t GetPts();
 	const uint8_t *GetPayload();
 	int GetPayloadSize();
+	int GetPacketLength();
 
 protected:
 	void Init();

softhddevice.cpp

@@ -732,7 +732,7 @@ void cSoftHdDevice::OnEventReceived(const Event& event) {
 					SetState(TRICK_SPEED);
 				},
 				[this](const StillPictureEvent& s) {
-					m_pVideoStream->StillPicture(&s.pesPacket);
+					HandleStillPicture(s.data, s.size);
 				 },
 			}, event);
 			break;
@@ -753,7 +753,7 @@ void cSoftHdDevice::OnEventReceived(const Event& event) {
 					m_pRender->SetPlaybackPaused(false);
 				 },
 				[this](const StillPictureEvent& s) {
-					m_pVideoStream->StillPicture(&s.pesPacket);
+					HandleStillPicture(s.data, s.size);
 				 },
 			}, event);
 			break;
@@ -990,11 +990,33 @@ void cSoftHdDevice::StillPicture(const uchar *data, int size)
 		return;
 	}
 
-	cPesVideo pesPacket((const uint8_t*)data, size);
-	if (pesPacket.IsValid()) {
-		OnEventReceived(StillPictureEvent{pesPacket});
-	} else
-		m_pVideoStream->ResetFragmentationBuffer();
+	OnEventReceived(StillPictureEvent{data, size});
+}
+
+/**
+ * The still picture data received from VDR can contain multiple PES packets.
+ * This sends each PES packet's raw data separately to PlayVideo(), and does a flush to display the frame immediately.
+ *
+ * @param data       pes data of one or more frames
+ * @param size       length of data area
+ */
+void cSoftHdDevice::HandleStillPicture(const uchar *data, int size)
+{
+	const uchar *currentPacketStart = data;
+	while (currentPacketStart < data + size) {
+		cPesVideo pesPacket((const uint8_t*)currentPacketStart, size - (currentPacketStart - data));
+
+		if (pesPacket.IsValid())
+			PlayVideo(currentPacketStart, pesPacket.GetPacketLength());
+		else {
+			LOGWARNING("device: %s: invalid PES packet", __FUNCTION__);
+			break;
+		}
+
+		currentPacketStart += pesPacket.GetPacketLength();
+	}
+
+	m_pVideoStream->Flush();
 }
 
 /**

softhddevice.h

@@ -53,7 +53,8 @@ struct TrickSpeedEvent {
 	bool forward;
 };
 struct StillPictureEvent {
-	cPesVideo& pesPacket;
+	const uchar *data;
+	int size;
 };
 
 using Event = std::variant<PlayEvent, PauseEvent, StopEvent, TrickSpeedEvent, StillPictureEvent>;
@@ -219,6 +220,7 @@ class cSoftHdDevice:public cDevice
 	int PesHeadLength(const uint8_t *);
 	void OnEventReceived(const Event&);
 	void HandlePause(void);
+	void HandleStillPicture(const uchar *data, int size);
 };
 
 #endif

tests/test_pes.cpp

@@ -28,7 +28,7 @@ extern "C" {
 // - Stream ID: 1 byte
 // - PES packet length: 2 bytes
 // - Optional PES header fields
-std::vector<uint8_t> createBasicPesVideoHeader(uint8_t streamId, bool withPts = false) {
+std::vector<uint8_t> createBasicPesVideoHeader(uint8_t streamId, bool withPts = false, uint16_t pesLength = 0) {
     std::vector<uint8_t> data;
 
     // Start code prefix
@@ -40,8 +40,8 @@ std::vector<uint8_t> createBasicPesVideoHeader(uint8_t streamId, bool withPts =
     data.push_back(streamId);
 
     // PES packet length (0 = unspecified)
-    data.push_back(0x00);
-    data.push_back(0x00);
+    data.push_back((pesLength >> 8) & 0xFF);
+    data.push_back(pesLength & 0xFF);
 
     // PES extension
     data.push_back(0x80); // '10'xxxxxx (no PES scrambling control, PES priority, data alignment indicator, copyright, original or copy)
@@ -334,6 +334,116 @@ TEST_CASE("cPesVideo - Payload extraction", "[pes]") {
     }
 }
 
+TEST_CASE("cPesVideo - Packet length", "[pes]") {
+    SECTION("Get packet length for unbounded MPEG2 (length field = 0)") {
+        auto data = createMpeg2PesPacket();
+        cPesVideo pes(data.data(), data.size());
+
+        REQUIRE(pes.GetPacketLength() == static_cast<int>(data.size()));
+    }
+
+    SECTION("Get packet length for unbounded H.264 (length field = 0)") {
+        auto data = createH264PesPacket(false);
+        cPesVideo pes(data.data(), data.size());
+
+        REQUIRE(pes.GetPacketLength() == static_cast<int>(data.size()));
+    }
+
+    SECTION("Get packet length with specified length field") {
+        // Create a PES packet with a specific length
+        // PES length field specifies bytes after the length field itself
+        uint16_t pesPayloadLength = 20; // Header data (3 bytes) + actual payload
+        auto data = createBasicPesVideoHeader(0xE0, false, pesPayloadLength);
+
+        // Add some payload to match the specified length
+        for (int i = data.size() - 6; i < pesPayloadLength; i++) {
+            data.push_back(0x00);
+        }
+
+        cPesVideo pes(data.data(), data.size());
+
+        REQUIRE(pes.GetPacketLength() == 6 + pesPayloadLength);
+    }
+
+    SECTION("Get packet length for packet with PTS and specified length") {
+        // PTS takes 5 bytes, so header data length = 5
+        // Total PES header = 9 (fixed header) + 5 (PTS) = 14 bytes
+        // If we want total packet of 50 bytes, length field = 50 - 6 = 44
+        uint16_t pesPayloadLength = 44;
+        auto data = createBasicPesVideoHeader(0xE0, true, pesPayloadLength);
+
+        // Add payload to make total packet 50 bytes
+        int currentSize = data.size();
+        int targetTotalSize = 6 + pesPayloadLength;
+        for (int i = currentSize; i < targetTotalSize; i++) {
+            data.push_back(0x00);
+        }
+
+        cPesVideo pes(data.data(), data.size());
+
+        REQUIRE(pes.GetPacketLength() == 50);
+    }
+
+    SECTION("Get packet length when input buffer is larger than PES packet") {
+        // Create a PES packet with specified length
+        uint16_t pesPayloadLength = 20;
+        auto data = createBasicPesVideoHeader(0xE0, false, pesPayloadLength);
+
+        // Add payload matching the PES length
+        for (int i = data.size() - 6; i < pesPayloadLength; i++) {
+            data.push_back(0xAA);
+        }
+
+        // Add extra data beyond the PES packet (simulating buffer with multiple packets)
+        for (int i = 0; i < 50; i++) {
+            data.push_back(0xFF);
+        }
+
+        cPesVideo pes(data.data(), data.size());
+
+        REQUIRE(pes.GetPacketLength() == 6 + pesPayloadLength);
+        REQUIRE(pes.GetPacketLength() < static_cast<int>(data.size()));
+    }
+
+    SECTION("Unbounded packet with buffer larger than actual data") {
+        // Create an unbounded packet (length field = 0)
+        auto data = createBasicPesVideoHeader(0xE0, false, 0);
+
+        // Add some actual payload
+        for (int i = 0; i < 30; i++) {
+            data.push_back(0xAA);
+        }
+
+        // Store the actual data size
+        int actualSize = data.size();
+
+        // Add extra buffer space (simulating oversized buffer)
+        for (int i = 0; i < 50; i++) {
+            data.push_back(0xFF);
+        }
+
+        cPesVideo pes(data.data(), data.size());
+
+        REQUIRE(pes.GetPacketLength() == static_cast<int>(data.size()));
+        REQUIRE(pes.GetPacketLength() > actualSize);
+    }
+
+    SECTION("Get packet length for audio packet with specified length") {
+        // Audio packets typically have bounded length
+        uint16_t pesPayloadLength = 30;
+        auto data = createBasicPesVideoHeader(0xC0, false, pesPayloadLength);
+
+        // Add audio payload
+        for (int i = data.size() - 6; i < pesPayloadLength; i++) {
+            data.push_back(0xFF);
+        }
+
+        cPesAudio pes(data.data(), data.size());
+
+        REQUIRE(pes.GetPacketLength() == 6 + pesPayloadLength);
+    }
+}
+
 TEST_CASE("cPesVideo - Audio stream handling", "[pes]") {
     SECTION("Audio stream without codec parsing") {
         auto data = createAudioPesPacket();

videorender.cpp

@@ -577,6 +577,7 @@ bool cVideoRender::IsKeyFrame(AVFrame *frame)
 void cVideoRender::MarkAsTrickspeedFrame(AVFrame *frame)
 {
 	SetFrameFlags(frame, FRAME_FLAG_TRICKSPEED);
+	MarkAsProgressiveFrame(frame);
 }
 
 /**
@@ -588,24 +589,21 @@ void cVideoRender::MarkAsStillpictureFrame(AVFrame *frame)
 {
 	SetFrameFlags(frame, FRAME_FLAG_STILLPICTURE);
 	frame->pts = AV_NOPTS_VALUE;
+	MarkAsProgressiveFrame(frame);
 }
 
 /**
  * Force this frame to be a progressive frame
  *
  * @param frame     AVFrame
- *
- * @returns         true, if the frame was an interlaced frame before
  */
-bool cVideoRender::MarkAsProgressiveFrame(AVFrame *frame)
+void cVideoRender::MarkAsProgressiveFrame(AVFrame *frame)
 {
-	bool wasInterlaced = IsInterlacedFrame(frame);
 #if LIBAVUTIL_VERSION_INT < AV_VERSION_INT(58,7,100)
 	frame->interlaced_frame = 0;
 #else
 	frame->flags &= ~AV_FRAME_FLAG_INTERLACED;
 #endif
-	return wasInterlaced;
 }
 
 /**
@@ -1121,11 +1119,8 @@ void cVideoRender::EnqueueFB(AVFrame *inframe)
  *
  * @param videoCtx   ffmpeg video codec context
  * @param frame       frame to render
- *
- * @retval 0          success or error, return (frame is either freed or moved to the render ringbuffer)
- * @retval -1         ringbuffer full, try again
  */
-int cVideoRender::RenderFrame(AVCodecContext * videoCtx, AVFrame * frame)
+void cVideoRender::RenderFrame(AVCodecContext * videoCtx, AVFrame * frame)
 {
 	if (!m_startCounter) {
 		m_timebaseMutex.Lock();
@@ -1180,7 +1175,7 @@ int cVideoRender::RenderFrame(AVCodecContext * videoCtx, AVFrame * frame)
 			LOGDEBUG("videorender: %s: wakeup filter thread", __FUNCTION__);
 			if (m_pFilterThread->Init(videoCtx, frame, m_deintDisabled)) {
 				av_frame_free(&frame);
-				return 0;
+				return;
 			} else {
 				m_pFilterThread->Start();
 			}
@@ -1200,7 +1195,7 @@ int cVideoRender::RenderFrame(AVCodecContext * videoCtx, AVFrame * frame)
 				FramesRbUnlock();
 			} else {
 				FramesRbUnlock();
-				return -1;
+				return;
 			}
 		} else {
 			// AV_PIX_FMT_DRM_NV12 ?
@@ -1211,12 +1206,10 @@ int cVideoRender::RenderFrame(AVCodecContext * videoCtx, AVFrame * frame)
 				EnqueueFB(frame);
 			} else {
 				FramesRbUnlock();
-				return -1;
+				return;
 			}
 		}
 	}
-
-	return 0;
 }
 
 

videorender.h

@@ -136,7 +136,7 @@ class cVideoRender
 	int DrmHandleEvent(void);
 
 	// Frame and buffer
-	int RenderFrame(AVCodecContext *, AVFrame *);
+	void RenderFrame(AVCodecContext *, AVFrame *);
 	void DisplayFrame(AVFrame *);
 	void EnqueueFB(AVFrame *);
 	int GetFramesFilled(void) { return atomic_read(&m_framesFilled); };
@@ -150,7 +150,6 @@ class cVideoRender
 	bool IsKeyFrame(AVFrame *);
 	void MarkAsTrickspeedFrame(AVFrame *);
 	void MarkAsStillpictureFrame(AVFrame *);
-	bool MarkAsProgressiveFrame(AVFrame *);
 	void ScheduleDisplayBlackFrame(void) { m_displayBlackFrame = true; };
 	void DestroyFrameBuffers(void);
 	void ClearDecoderToDisplayQueue(void);
@@ -236,6 +235,7 @@ class cVideoRender
 #endif
 	int GetFrameFlags(AVFrame *);
 	void SetFrameFlags(AVFrame *, int);
+	void MarkAsProgressiveFrame(AVFrame *);
 	void SetVideoClock(int64_t);
 	bool ShouldWaitForAudio(void);
 	void WaitForAudioReady(int64_t, int64_t);