PR #25394 opened by LostPatrol URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25394 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25394.patch
`ff_rtp_send_jpeg()` drops the DRI restart interval but retains restart markers, causing decoding errors after RTP transport. For scans with restart markers, use RTP/JPEG type 64 or 65 and include a Restart Marker header in every packet, accounting for its four bytes during fragmentation. Set F=L=1 and Restart Count to `0x3fff` per [RFC 2435 ยง3.1.7](https://www.rfc-editor.org/rfc/rfc2435#section-3.1.7), since packet boundaries are not guaranteed to align with restart intervals. Respect JPEG segment boundaries and marker fill bytes to avoid interpreting APP data as DRI. Retain types 0 and 1 for scans without restart markers. For single-scan frames with three interleaved components and dimensions known from SOF, skip the entropy search when the interval is at least the derived MCU count. Check shorter nonzero intervals with a bounded search using `memchr`. Four FATE regression tests pass; RTP decoding of 4:2:0 and 4:2:2 samples matches direct-decoding MD5s. Related work: [rayjay's earlier DRI proposal](https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/321697.html). From 15a31dafbad62d90d3f190088f5351071338126d Mon Sep 17 00:00:00 2001 From: LostPatrol <[email protected]> Date: Fri, 23 Oct 2026 16:57:36 +0000 Subject: [PATCH] avformat/rtpenc_jpeg: forward JPEG restart intervals ff_rtp_send_jpeg() strips the JPEG DRI marker while sending type 0 or 1. For scans containing restart markers, receivers then lack the interval needed to decode the frame. A captured 1920x1080 4:2:2 UVC frame with DRI=120 produces error dc and a different frame checksum over RTP. Send type 64 or 65 with the RFC 2435 restart header in each packet when the scan uses restart markers. Account for the header when fragmenting, and use F=L=1 with restart count 0x3fff for unaligned fragments. Keep the ordinary packet format when the declared interval is unused. Use the MCU count from SOF to avoid searching a whole scan when the interval covers the entire frame; shorter intervals use a bounded memchr search for actual RST. Follow JPEG segment boundaries and handle marker fill bytes so metadata cannot be interpreted as DRI. Add FATE checks for restart headers and fragment offsets, DRI-like APP data, unused intervals, marker fill and partial edge MCUs. Map temporary file arguments to the target path so the tests also work with remote executors. The captured frame now decodes through RTP to the same checksum as direct JPEG decoding. Plain and unused-interval frames retain their RTP payload. Related: https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/321697.html Signed-off-by: LostPatrol <[email protected]> --- libavformat/rtpenc_jpeg.c | 64 ++++++++++++++++++- tests/fate/ffmpeg.mak | 48 ++++++++++++++ tests/ref/fate/ffmpeg-rtp-jpeg-app-marker | 1 + tests/ref/fate/ffmpeg-rtp-jpeg-marker-fill | 1 + tests/ref/fate/ffmpeg-rtp-jpeg-restart | 2 + tests/ref/fate/ffmpeg-rtp-jpeg-unused-restart | 1 + 6 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 tests/ref/fate/ffmpeg-rtp-jpeg-app-marker create mode 100644 tests/ref/fate/ffmpeg-rtp-jpeg-marker-fill create mode 100644 tests/ref/fate/ffmpeg-rtp-jpeg-restart create mode 100644 tests/ref/fate/ffmpeg-rtp-jpeg-unused-restart diff --git a/libavformat/rtpenc_jpeg.c b/libavformat/rtpenc_jpeg.c index ffffb7cd154..33abfa3a54f 100644 --- a/libavformat/rtpenc_jpeg.c +++ b/libavformat/rtpenc_jpeg.c @@ -25,6 +25,7 @@ #include "libavutil/intreadwrite.h" #include "rtpenc.h" +/** Packetize a JPEG scan, forwarding the tables and restart interval it uses. */ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size) { RTPMuxContext *s = s1->priv_data; @@ -36,7 +37,10 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size) int off = 0; /* fragment offset of the current JPEG frame */ int len; int i; + int segment_end = 0; + int nb_mcus = 0; int default_huffman_tables = 0; + uint16_t restart_interval = 0; s->buf_ptr = s->buf; s->timestamp = s->cur_timestamp; @@ -47,7 +51,7 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size) /* preparse the header for getting some info */ for (i = 0; i < size; i++) { - if (buf[i] != 0xff) + if (i < segment_end || buf[i] != 0xff) continue; if (size - i < 5 || (buf[i + 1] == SOF0 && size - i < 18)) { @@ -55,6 +59,18 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size) return; } + /* Fill bytes and stuffed zeros do not start marker segments. */ + if (buf[i + 1] == 0xff || buf[i + 1] == 0x00) + continue; + + /* Marker-like bytes within a segment are not JPEG markers. */ + if (buf[i + 1] != SOI && buf[i + 1] != EOI && buf[i + 1] != TEM && + (buf[i + 1] < RST0 || buf[i + 1] > RST7)) { + int marker_length = AV_RB16(buf + i + 2); + if (marker_length >= 2 && marker_length <= size - i - 2) + segment_end = i + 2 + marker_length; + } + if (buf[i + 1] == DQT) { int tables, j; if (buf[i + 4] & 0xF0) @@ -98,6 +114,12 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size) av_log(s1, AV_LOG_ERROR, "Unsupported pixel format\n"); return; } + /* Count MCUs using this frame's SOF, not stream dimensions. */ + if (AV_RB16(buf + i + 2) == 17 && buf[i + 9] == 3) + nb_mcus = AV_CEIL_RSHIFT(AV_RB16(buf + i + 7), 4) * + AV_CEIL_RSHIFT(AV_RB16(buf + i + 5), 3 + type); + else + nb_mcus = 0; } else if (buf[i + 1] == DHT) { int dht_size = AV_RB16(&buf[i + 2]); default_huffman_tables |= 1 << 4; @@ -160,7 +182,15 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size) dht_size = 0; continue; } + } else if (buf[i + 1] == DRI) { + if (size - i < 6 || AV_RB16(buf + i + 2) != 4) { + av_log(s1, AV_LOG_ERROR, "Invalid JPEG DRI marker. Aborted!\n"); + return; + } + restart_interval = AV_RB16(buf + i + 4); } else if (buf[i + 1] == SOS) { + if (AV_RB16(buf + i + 2) != 12 || buf[i + 4] != 3) + nb_mcus = 0; /* SOS is last marker in the header */ i += AV_RB16(&buf[i + 2]) + 2; if (i > size) { @@ -201,10 +231,36 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size) } } + /* A whole-frame interval has no RST in a single interleaved scan. */ + if (nb_mcus && restart_interval >= nb_mcus) + restart_interval = 0; + + if (restart_interval) { + /* Preserve the ordinary type when a shorter interval is unused too. */ + const uint8_t *scan = buf; + const uint8_t *end = buf + size; + + while (end - scan > 1) { + /* Leave one byte for the marker code after 0xff. */ + scan = memchr(scan, 0xff, end - scan - 1); + if (!scan) + break; + if (scan[1] >= RST0 && scan[1] <= RST7) { + type |= 0x40; + break; + } + scan++; + } + if (!(type & 0x40)) + restart_interval = 0; + } + p = s->buf_ptr; while (size > 0) { int hdr_size = 8; + if (restart_interval) + hdr_size += 4; if (off == 0 && nb_qtables) hdr_size += 4 + 64 * nb_qtables; @@ -225,6 +281,12 @@ void ff_rtp_send_jpeg(AVFormatContext *s1, const uint8_t *buf, int size) bytestream_put_byte(&p, w); bytestream_put_byte(&p, h); + if (restart_interval) { + bytestream_put_be16(&p, restart_interval); + /* Fragments are not aligned to restart intervals. */ + bytestream_put_be16(&p, 0xffff); + } + if (off == 0 && nb_qtables) { /* set quantization tables header */ bytestream_put_byte(&p, 0); diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak index aa682ba202f..704a8f88812 100644 --- a/tests/fate/ffmpeg.mak +++ b/tests/fate/ffmpeg.mak @@ -409,6 +409,54 @@ fate-ffmpeg-no-overwrite: CMP = null fate-ffmpeg-no-overwrite: CMD = out=tests/data/fate/no-overwrite.wav; touch $$out; ffmpeg -f lavfi -i anullsrc -t 0.01 -c:a pcm_u8 -f wav -n $$out; ret=$$?; rm -f $$out; test $$ret -ne 0 FATE_FFMPEG-$(call ALLYES, LAVFI_INDEV ANULLSRC_FILTER PCM_U8_DECODER PCM_U8_ENCODER WAV_MUXER FILE_PROTOCOL) += fate-ffmpeg-no-overwrite +# Check restart headers and fragment offsets with partial edge MCUs. +fate-ffmpeg-rtp-jpeg-restart: CMD = \ + out="$${outfile}.rtp"; cleanfiles="$${cleanfiles} $$out"; \ + run $(FFMPEG) -nostdin -hide_banner -loglevel error -f lavfi -i color=s=32x32:r=1 \ + -vf scale=17:17 -frames:v 1 -c:v mjpeg -pix_fmt yuvj420p -huffman default -slices 2 \ + -rtpflags skip_rtcp -pkt_size 100 -f rtp -y "$$(target_path "$$out")" >/dev/null && \ + od -An -tx1 -j 16 -N 8 "$$out" && \ + od -An -tx1 -j 112 -N 12 "$$out" +FATE_FFMPEG-$(call ALLYES, LAVFI_INDEV COLOR_FILTER WRAPPED_AVFRAME_DECODER SCALE_FILTER MJPEG_ENCODER RTP_MUXER FILE_PROTOCOL) += fate-ffmpeg-rtp-jpeg-restart + +# An invalid DRI-looking sequence in APP metadata must not be parsed as DRI. +fate-ffmpeg-rtp-jpeg-app-marker: CMD = \ + plain="$${outfile}.jpg"; marked="$${outfile}.app.jpg"; out="$${outfile}.rtp"; \ + cleanfiles="$${cleanfiles} $$plain $$marked $$out"; \ + run $(FFMPEG) -nostdin -hide_banner -loglevel error -f lavfi -i color=s=320x240:r=1 \ + -frames:v 1 -c:v mjpeg -pix_fmt yuvj420p -huffman default -f image2 -y "$$(target_path "$$plain")" >/dev/null && \ + printf "\377\330\377\341\000\010\377\335\000\003\000\170" > "$$marked" && \ + dd if="$$plain" bs=1 skip=2 2>/dev/null >> "$$marked" && \ + run $(FFMPEG) -nostdin -hide_banner -loglevel error -f image2 -i "$$(target_path "$$marked")" \ + -frames:v 1 -c:v copy -rtpflags skip_rtcp -pkt_size 4096 -f rtp -y "$$(target_path "$$out")" >/dev/null && \ + od -An -tx1 -j 16 -N 1 "$$out" +FATE_FFMPEG-$(call ALLYES, LAVFI_INDEV COLOR_FILTER WRAPPED_AVFRAME_DECODER SCALE_FILTER MJPEG_ENCODER MJPEG_DECODER MJPEG_PARSER IMAGE2_MUXER IMAGE2_DEMUXER RTP_MUXER FILE_PROTOCOL) += fate-ffmpeg-rtp-jpeg-app-marker + +# A nonzero interval without restart markers retains the ordinary RTP type. +fate-ffmpeg-rtp-jpeg-unused-restart: CMD = \ + plain="$${outfile}.jpg"; marked="$${outfile}.dri.jpg"; out="$${outfile}.rtp"; \ + cleanfiles="$${cleanfiles} $$plain $$marked $$out"; \ + run $(FFMPEG) -nostdin -hide_banner -loglevel error -f lavfi -i color=s=320x240:r=1 \ + -frames:v 1 -c:v mjpeg -pix_fmt yuvj420p -huffman default -slices 1 -f image2 -y "$$(target_path "$$plain")" >/dev/null && \ + printf "\377\330\377\335\000\004\377\377" > "$$marked" && \ + dd if="$$plain" bs=1 skip=2 2>/dev/null >> "$$marked" && \ + run $(FFMPEG) -nostdin -hide_banner -loglevel error -f image2 -i "$$(target_path "$$marked")" \ + -frames:v 1 -c:v copy -rtpflags skip_rtcp -pkt_size 4096 -f rtp -y "$$(target_path "$$out")" >/dev/null && \ + od -An -tx1 -j 16 -N 1 "$$out" +FATE_FFMPEG-$(call ALLYES, LAVFI_INDEV COLOR_FILTER WRAPPED_AVFRAME_DECODER SCALE_FILTER MJPEG_ENCODER MJPEG_DECODER MJPEG_PARSER IMAGE2_MUXER IMAGE2_DEMUXER RTP_MUXER FILE_PROTOCOL) += fate-ffmpeg-rtp-jpeg-unused-restart + +# A fill byte before SOI must not be interpreted as a segment length. +fate-ffmpeg-rtp-jpeg-marker-fill: CMD = \ + plain="$${outfile}.jpg"; filled="$${outfile}.fill.jpg"; out="$${outfile}.rtp"; \ + cleanfiles="$${cleanfiles} $$plain $$filled $$out"; \ + run $(FFMPEG) -nostdin -hide_banner -loglevel error -f lavfi -i testsrc2=s=1920x1080:r=1 \ + -frames:v 1 -c:v mjpeg -pix_fmt yuvj420p -huffman default -slices 1 -q:v 2 -f image2 -y "$$(target_path "$$plain")" >/dev/null && \ + printf "\377" > "$$filled" && cat "$$plain" >> "$$filled" && \ + run $(FFMPEG) -nostdin -hide_banner -loglevel error -f image2 -i "$$(target_path "$$filled")" \ + -frames:v 1 -c:v copy -rtpflags skip_rtcp -pkt_size 4096 -f rtp -y "$$(target_path "$$out")" >/dev/null && \ + od -An -tx1 -j 16 -N 1 "$$out" +FATE_FFMPEG-$(call ALLYES, LAVFI_INDEV TESTSRC2_FILTER WRAPPED_AVFRAME_DECODER SCALE_FILTER MJPEG_ENCODER MJPEG_DECODER MJPEG_PARSER IMAGE2_MUXER IMAGE2_DEMUXER RTP_MUXER FILE_PROTOCOL) += fate-ffmpeg-rtp-jpeg-marker-fill + # test input -bsf # use -stream_loop, because it tests flushing bsfs fate-ffmpeg-bsf-input: CMD = framecrc -stream_loop 2 -bsf setts=PTS*2 -i $(TARGET_SAMPLES)/hevc/extradata-reload-multi-stsd.mov -c copy diff --git a/tests/ref/fate/ffmpeg-rtp-jpeg-app-marker b/tests/ref/fate/ffmpeg-rtp-jpeg-app-marker new file mode 100644 index 00000000000..8e83db0865d --- /dev/null +++ b/tests/ref/fate/ffmpeg-rtp-jpeg-app-marker @@ -0,0 +1 @@ + 01 diff --git a/tests/ref/fate/ffmpeg-rtp-jpeg-marker-fill b/tests/ref/fate/ffmpeg-rtp-jpeg-marker-fill new file mode 100644 index 00000000000..8e83db0865d --- /dev/null +++ b/tests/ref/fate/ffmpeg-rtp-jpeg-marker-fill @@ -0,0 +1 @@ + 01 diff --git a/tests/ref/fate/ffmpeg-rtp-jpeg-restart b/tests/ref/fate/ffmpeg-rtp-jpeg-restart new file mode 100644 index 00000000000..abb57b03154 --- /dev/null +++ b/tests/ref/fate/ffmpeg-rtp-jpeg-restart @@ -0,0 +1,2 @@ + 41 ff 03 03 00 02 ff ff + 00 00 00 08 41 ff 03 03 00 02 ff ff diff --git a/tests/ref/fate/ffmpeg-rtp-jpeg-unused-restart b/tests/ref/fate/ffmpeg-rtp-jpeg-unused-restart new file mode 100644 index 00000000000..8e83db0865d --- /dev/null +++ b/tests/ref/fate/ffmpeg-rtp-jpeg-unused-restart @@ -0,0 +1 @@ + 01 -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
