diff mbox series

[FFmpeg-devel] libavformat/rtpdec_jpeg.c: quantization table headers not sent in every frame packet

Message ID BLAPR10MB51698536C8B34A162BDCFADDA3C49@BLAPR10MB5169.namprd10.prod.outlook.com
State New
Headers show
Series [FFmpeg-devel] libavformat/rtpdec_jpeg.c: quantization table headers not sent in every frame packet | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Hayden Myers Aug. 23, 2021, 11:23 p.m. UTC
MJPEG streams coming from Genetec VMS provide a quantization table
for each frame, but only in the first packet. Before the packet data is
copied to the frame buffer, a check is done to compare the fragment
offset against the frame - header.  The header is computed at
the beginning of each frame. The offset ends up with a value of -132
because the header size includes the quantization table data, but the
packet buffer doesn't.

Created a function to detect if a quantization header isn't present when
it should be, and use this to offset the extra bytes reported in the
jpeg header.

Signed-off-by: Hayden Myers <hmyers@skylinenet.net>

cleanup
---
 libavformat/rtpdec_jpeg.c | 54 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 4 deletions(-)

--
2.20.1
Hayden Myers
Principal Software Engineer
t: (410) 590-2027

Comments

Hayden Myers Sept. 10, 2021, 1:22 a.m. UTC | #1
Hayden Myers
Principal Software Engineer
t: (410) 590-2027
From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> on behalf of Hayden Myers <HMyers@skylinenet.net>
Sent: Monday, August 23, 2021 7:23 PM
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: [FFmpeg-devel] [PATCH] libavformat/rtpdec_jpeg.c: quantization table headers not sent in every frame packet 
 
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.


MJPEG streams coming from Genetec VMS provide a quantization table
for each frame, but only in the first packet. Before the packet data is
copied to the frame buffer, a check is done to compare the fragment
offset against the frame - header.  The header is computed at
the beginning of each frame. The offset ends up with a value of -132
because the header size includes the quantization table data, but the
packet buffer doesn't.

Created a function to detect if a quantization header isn't present when
it should be, and use this to offset the extra bytes reported in the
jpeg header.

Signed-off-by: Hayden Myers <hmyers@skylinenet.net>

cleanup
---
 libavformat/rtpdec_jpeg.c | 54 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/libavformat/rtpdec_jpeg.c b/libavformat/rtpdec_jpeg.c
index b32d074136..6e1c6d6b44 100644
--- a/libavformat/rtpdec_jpeg.c
+++ b/libavformat/rtpdec_jpeg.c
@@ -211,6 +211,52 @@ static void create_default_qtables(uint8_t *qtables, uint8_t q)
     }
 }

+/*
+ * If the q header isn't present in th packet, subtract from the
+ * jpeg header. The first packet in the frame could contain the q header.
+ *
+ * Some implementation do not include the quanization header with each packet.
+ * I'm specifically calling out Genetec VMS, but it could be rooted in specific
+ * cameras.
+ *
+ * @param ctx - The context, just used for logging.
+ * @param buf - current packet buffer
+ * @param q - quantizer value
+ *
+ * @return the number of bytes to remove if q header isn't present in the packet
+ * ,or 0 if the header is detected, or q <=127.
+ *
+ */
+static int get_q_hdr_bytes_to_remove(AVFormatContext *ctx, const uint8_t *buf,
+                                     uint8_t q)
+{
+    int ret=0, mbz=0, precision=0;
+
+    /* Use the first byte to detect if the quantization table header is
+     * present.  If mbz isn't zero, and the  precision byte isn't <= 1 quant
+     * table isn't present, and we're into jpeg image payload data already.
+     */
+    mbz = AV_RB8(buf); /* reserved byte should always be 0 */
+    precision  = AV_RB8(buf + 1);    /* size of coefficients */
+    /* best attempt to determine if the q header is present. Dont remove
+     * anything if a qtable is detected or q < 128.
+     */
+    if ((mbz == 0 && precision <= 1) || q < 128) {
+        ret = 0;
+    // no header detected in pkt, must remove q hdr bytes
+    } else {
+        //1 byte - reserved MBZ
+        //1 byte - precision
+        //2 bytes - length
+        ret += 4;
+        //128 bytes - 2 * 64 byte tables (8bit precision)
+        ret += 128;
+        av_log(ctx,AV_LOG_WARNING,
+             "Q header missing, reducing jpeg->hdr_size\n");
+    }
+    return ret;
+}
+
 static int jpeg_parse_packet(AVFormatContext *ctx, PayloadContext *jpeg,
                              AVStream *st, AVPacket *pkt, uint32_t *timestamp,
                              const uint8_t *buf, int len, uint16_t seq,
@@ -220,7 +266,7 @@ static int jpeg_parse_packet(AVFormatContext *ctx, PayloadContext *jpeg,
     const uint8_t *qtables = NULL;
     uint16_t qtable_len;
     uint32_t off;
-    int ret, dri = 0;
+    int ret, dri = 0, q_hdr_bytes_to_remove = 0;

     if (len < 8) {
         av_log(ctx, AV_LOG_ERROR, "Too short RTP/JPEG packet.\n");
@@ -235,6 +281,7 @@ static int jpeg_parse_packet(AVFormatContext *ctx, PayloadContext *jpeg,
     height = AV_RB8(buf + 7);   /* frame height in 8 pixel blocks */
     buf += 8;
     len -= 8;
+    q_hdr_bytes_to_remove = get_q_hdr_bytes_to_remove(ctx, buf, q);

     if (type & 0x40) {
         if (len < 4) {
@@ -350,9 +397,8 @@ static int jpeg_parse_packet(AVFormatContext *ctx, PayloadContext *jpeg,
         return AVERROR_INVALIDDATA;
     }

-    if (off != avio_tell(jpeg->frame) - jpeg->hdr_size) {
-        av_log(ctx, AV_LOG_ERROR,
-               "Missing packets; dropping frame.\n");
+    if (off != (avio_tell(jpeg->frame) - (jpeg->hdr_size - q_hdr_bytes_to_remove))) {
+        av_log(ctx, AV_LOG_ERROR, "Missing packets; dropping frame.\n");
         return AVERROR(EAGAIN);
     }

I haven't seen any activity for this patch, and I've found problems in my code.  The patch is flawed, so please disregard.  The root of the problem lies in the fragmentation offset reported in the packet's jpeg header.  The rtp jpeg decoder expects the offset to NOT include the quantization header and data bytes.  I've found a popular stream provider which includes the quantization header and data table in the value of the fragmentation offset.  This results in the decoder dropping all packets and ffmpeg unable to decode the stream.  I'll submit another patch soon which doesn't drop the frame if the offset is off by q header + q table len.  Please provide feedback on https://datatracker.ietf.org/doc/html/rfc2435#section-3.1.2.  It's not clear if it's invalid or up in the air for the fragmentation offset to include the q header and data table bytes in the length. Thanks in advance.
diff mbox series

Patch

diff --git a/libavformat/rtpdec_jpeg.c b/libavformat/rtpdec_jpeg.c
index b32d074136..6e1c6d6b44 100644
--- a/libavformat/rtpdec_jpeg.c
+++ b/libavformat/rtpdec_jpeg.c
@@ -211,6 +211,52 @@  static void create_default_qtables(uint8_t *qtables, uint8_t q)
     }
 }

+/*
+ * If the q header isn't present in th packet, subtract from the
+ * jpeg header. The first packet in the frame could contain the q header.
+ *
+ * Some implementation do not include the quanization header with each packet.
+ * I'm specifically calling out Genetec VMS, but it could be rooted in specific
+ * cameras.
+ *
+ * @param ctx - The context, just used for logging.
+ * @param buf - current packet buffer
+ * @param q - quantizer value
+ *
+ * @return the number of bytes to remove if q header isn't present in the packet
+ * ,or 0 if the header is detected, or q <=127.
+ *
+ */
+static int get_q_hdr_bytes_to_remove(AVFormatContext *ctx, const uint8_t *buf,
+                                     uint8_t q)
+{
+    int ret=0, mbz=0, precision=0;
+
+    /* Use the first byte to detect if the quantization table header is
+     * present.  If mbz isn't zero, and the  precision byte isn't <= 1 quant
+     * table isn't present, and we're into jpeg image payload data already.
+     */
+    mbz = AV_RB8(buf); /* reserved byte should always be 0 */
+    precision  = AV_RB8(buf + 1);    /* size of coefficients */
+    /* best attempt to determine if the q header is present. Dont remove
+     * anything if a qtable is detected or q < 128.
+     */
+    if ((mbz == 0 && precision <= 1) || q < 128) {
+        ret = 0;
+    // no header detected in pkt, must remove q hdr bytes
+    } else {
+        //1 byte - reserved MBZ
+        //1 byte - precision
+        //2 bytes - length
+        ret += 4;
+        //128 bytes - 2 * 64 byte tables (8bit precision)
+        ret += 128;
+        av_log(ctx,AV_LOG_WARNING,
+             "Q header missing, reducing jpeg->hdr_size\n");
+    }
+    return ret;
+}
+
 static int jpeg_parse_packet(AVFormatContext *ctx, PayloadContext *jpeg,
                              AVStream *st, AVPacket *pkt, uint32_t *timestamp,
                              const uint8_t *buf, int len, uint16_t seq,
@@ -220,7 +266,7 @@  static int jpeg_parse_packet(AVFormatContext *ctx, PayloadContext *jpeg,
     const uint8_t *qtables = NULL;
     uint16_t qtable_len;
     uint32_t off;
-    int ret, dri = 0;
+    int ret, dri = 0, q_hdr_bytes_to_remove = 0;

     if (len < 8) {
         av_log(ctx, AV_LOG_ERROR, "Too short RTP/JPEG packet.\n");
@@ -235,6 +281,7 @@  static int jpeg_parse_packet(AVFormatContext *ctx, PayloadContext *jpeg,
     height = AV_RB8(buf + 7);   /* frame height in 8 pixel blocks */
     buf += 8;
     len -= 8;
+    q_hdr_bytes_to_remove = get_q_hdr_bytes_to_remove(ctx, buf, q);

     if (type & 0x40) {
         if (len < 4) {
@@ -350,9 +397,8 @@  static int jpeg_parse_packet(AVFormatContext *ctx, PayloadContext *jpeg,
         return AVERROR_INVALIDDATA;
     }

-    if (off != avio_tell(jpeg->frame) - jpeg->hdr_size) {
-        av_log(ctx, AV_LOG_ERROR,
-               "Missing packets; dropping frame.\n");
+    if (off != (avio_tell(jpeg->frame) - (jpeg->hdr_size - q_hdr_bytes_to_remove))) {
+        av_log(ctx, AV_LOG_ERROR, "Missing packets; dropping frame.\n");
         return AVERROR(EAGAIN);
     }