| 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 |
| 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 |
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 --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); }
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