PR #25156 opened by James Almer (jamrial) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25156 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25156.patch
Opening a file for this is hacky and the tests added for it are broken on Windows. >From e4d02c2d72598bc9c49dbaee7a1660111acda280 Mon Sep 17 00:00:00 2001 From: James Almer <[email protected]> Date: Sat, 17 Oct 2026 15:25:26 -0300 Subject: [PATCH] avcodec/bsf/jpegapp: use an AVOption of binary type for the sidecar Opening a file for this is hacky and the tests added for it are broken on Windows. Signed-off-by: James Almer <[email protected]> --- doc/bitstream_filters.texi | 7 +--- libavcodec/bsf/jpegapp.c | 78 +++++++++----------------------------- tests/fate/jpegapp.mak | 16 ++------ 3 files changed, 23 insertions(+), 78 deletions(-) diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi index 47bb75892b..cc1cd453a7 100644 --- a/doc/bitstream_filters.texi +++ b/doc/bitstream_filters.texi @@ -635,7 +635,7 @@ the two bytes immediately following SOI, that APP0 segment is dropped. APP0 segments preceded by fill bytes or another segment are retained. An APP1-only payload keeps the encoder APP0: SOI, APP1, APP0, DQT, ... -The filter requires @option{filename}. Extra unused sidecar bytes are +The filter requires @option{sidecar}. Extra unused sidecar bytes are ignored; a packet without a matching record is an error. Records follow packets in arrival order. Flushing the filter does not rewind the sidecar, so a reused context needs a sidecar positioned for @@ -645,12 +645,9 @@ by that filter context. To resume, create a new context with a sidecar matching the remaining packets. Flushing does not clear this error. @example -ffmpeg -i in.mp4 -c:v mjpeg -bsf:v jpegapp=filename=app.bin frame_%d.jpg +ffmpeg -i in.mp4 -c:v mjpeg -bsf:v jpegapp=sidecar=00000004DEADBEEF frame_%d.jpg @end example -On Windows, escape the drive colon in the BSF option: -@code{filename='E\:/path/app.bin'}. - @section mjpeg2jpeg Convert MJPEG/AVI1 packets to full JPEG/JFIF packets. diff --git a/libavcodec/bsf/jpegapp.c b/libavcodec/bsf/jpegapp.c index c75822dfb6..8024b77d46 100644 --- a/libavcodec/bsf/jpegapp.c +++ b/libavcodec/bsf/jpegapp.c @@ -56,16 +56,16 @@ #include "libavcodec/bsf.h" #include "libavcodec/bsf_internal.h" +#include "libavcodec/bytestream.h" #include "libavcodec/mjpeg.h" #define JPEGAPP_MAX_RECORD (16 * 1024 * 1024) typedef struct JPEGAppContext { const AVClass *class; - char *filename; - FILE *file; - uint8_t *buf; - int buf_size; + GetByteContext gb; + uint8_t *sidecar; + int sidecar_len; int read_error; int64_t packet_index; } JPEGAppContext; @@ -99,84 +99,43 @@ static int jpegapp_init(AVBSFContext *ctx) { JPEGAppContext *s = ctx->priv_data; - if (!s->filename || !s->filename[0]) { - av_log(ctx, AV_LOG_ERROR, "Option filename is required\n"); + if (s->sidecar_len <= 0) { + av_log(ctx, AV_LOG_ERROR, "Option sidecar is required\n"); return AVERROR(EINVAL); } - s->file = avpriv_fopen_utf8(s->filename, "rb"); - if (!s->file) { - int err = errno ? AVERROR(errno) : AVERROR(EIO); - av_log(ctx, AV_LOG_ERROR, "Could not open '%s': %s\n", - s->filename, av_err2str(err)); - return err; - } + bytestream2_init(&s->gb, s->sidecar, s->sidecar_len); + return 0; } -static void jpegapp_close(AVBSFContext *ctx) -{ - JPEGAppContext *s = ctx->priv_data; - - if (s->file) { - fclose(s->file); - s->file = NULL; - } - av_freep(&s->buf); - s->buf_size = 0; -} - static int jpegapp_read_record(AVBSFContext *ctx, int *out_size) { JPEGAppContext *s = ctx->priv_data; - uint8_t hdr[4]; uint32_t n; - size_t got; int ret; - errno = 0; - got = fread(hdr, 1, 4, s->file); - ret = errno; - if (got == 0 && feof(s->file) && !ferror(s->file)) { + if (!bytestream2_get_bytes_left(&s->gb)) { av_log(ctx, AV_LOG_ERROR, "jpegapp: no sidecar record for packet %"PRId64"\n", s->packet_index); return AVERROR_INVALIDDATA; } - if (got != 4) { - if (ferror(s->file)) { - av_log(ctx, AV_LOG_ERROR, "jpegapp: sidecar read failed\n"); - return ret ? AVERROR(ret) : AVERROR(EIO); - } + if (bytestream2_get_bytes_left(&s->gb) < 4) { av_log(ctx, AV_LOG_ERROR, "jpegapp: truncated length prefix\n"); return AVERROR_INVALIDDATA; } - n = AV_RB32(hdr); + n = bytestream2_get_be32(&s->gb); if (n > JPEGAPP_MAX_RECORD) { av_log(ctx, AV_LOG_ERROR, "jpegapp: record length %u too large\n", n); return AVERROR_INVALIDDATA; } - if (n > (uint32_t)s->buf_size) { - ret = av_reallocp(&s->buf, n); - if (ret < 0) { - s->buf_size = 0; - return ret; - } - s->buf_size = n; - } if (n) { - errno = 0; - got = fread(s->buf, 1, n, s->file); - ret = errno; - if (got != n) { - if (ferror(s->file)) { - av_log(ctx, AV_LOG_ERROR, "jpegapp: sidecar read failed\n"); - return ret ? AVERROR(ret) : AVERROR(EIO); - } + if (n > bytestream2_get_bytes_left(&s->gb)) { av_log(ctx, AV_LOG_ERROR, "jpegapp: truncated record payload\n"); return AVERROR_INVALIDDATA; } - ret = jpegapp_validate_payload(s->buf, n); + ret = jpegapp_validate_payload(s->gb.buffer, n); if (ret < 0) { av_log(ctx, AV_LOG_ERROR, "jpegapp: payload is not a sequence of APP0..APP15 segments\n"); @@ -213,7 +172,7 @@ static int jpegapp_filter(AVBSFContext *ctx, AVPacket *out) } src_off = 2; - if (rec_size >= 2 && s->buf[0] == 0xff && s->buf[1] == APP0 && + if (rec_size >= 2 && s->gb.buffer[0] == 0xff && s->gb.buffer[1] == APP0 && in->size >= 4 && in->data[2] == 0xff && in->data[3] == APP0) { int skip = jpeg_app_segment_len(in->data + 2, in->size - 2); if (skip < 0) { @@ -243,7 +202,7 @@ static int jpegapp_filter(AVBSFContext *ctx, AVPacket *out) goto fail; memcpy(out->data, in->data, 2); - memcpy(out->data + 2, s->buf, rec_size); + bytestream2_get_buffer(&s->gb, out->data + 2, rec_size); memcpy(out->data + 2 + rec_size, in->data + src_off, in->size - src_off); av_packet_free(&in); return 0; @@ -258,10 +217,8 @@ fail: #define OFFSET(x) offsetof(JPEGAppContext, x) #define FLAGS (AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_BSF_PARAM) static const AVOption jpegapp_options[] = { - { "filename", "sidecar of u32be-length + APP-segment records", OFFSET(filename), - AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, FLAGS }, - { "file", "sidecar of u32be-length + APP-segment records", OFFSET(filename), - AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, FLAGS }, + { "sidecar", "sidecar of u32be-length + APP-segment records", OFFSET(sidecar), + AV_OPT_TYPE_BINARY, .flags = FLAGS }, { NULL }, }; @@ -282,6 +239,5 @@ const FFBitStreamFilter ff_jpegapp_bsf = { .p.priv_class = &jpegapp_class, .priv_data_size = sizeof(JPEGAppContext), .init = jpegapp_init, - .close = jpegapp_close, .filter = jpegapp_filter, }; diff --git a/tests/fate/jpegapp.mak b/tests/fate/jpegapp.mak index 995189af11..36463b1930 100644 --- a/tests/fate/jpegapp.mak +++ b/tests/fate/jpegapp.mak @@ -5,10 +5,8 @@ FATE_JPEGAPP-$(call ALLYES, SMJPEG_DEMUXER JPEGAPP_BSF MJPEG_MUXER FILE_PROTOCOL) += fate-bsf-jpegapp fate-bsf-jpegapp: CMD = \ - cleanfiles="$${cleanfiles} $${outfile}.bin"; \ - printf "\\000\\000\\000\\010\\377\\341\\000\\006FATE\\000\\000\\000\\000\\000\\000\\000\\010\\377\\340\\000\\006FATE" > "$${outfile}.bin"; \ md5 -i $(TARGET_SAMPLES)/smjpeg/scenwin.mjpg -map 0:v:0 -frames:v 3 -c:v copy \ - -bsf:v "jpegapp=filename=$$(target_path "$${outfile}.bin" | sed "s/:/\\\\\\\\:/g")" -fflags +bitexact -f mjpeg + -bsf:v "jpegapp=sidecar=00000008FFE10006464154450000000000000008FFE0000646415445" -fflags +bitexact -f mjpeg # One record for three packets: the second packet must fail instead of # silently passing through without its metadata. @@ -16,10 +14,8 @@ FATE_JPEGAPP-$(call ALLYES, SMJPEG_DEMUXER JPEGAPP_BSF MJPEG_MUXER FILE_PROTOCOL fate-bsf-jpegapp-missing: CMP = grep fate-bsf-jpegapp-missing: REF = jpegapp: no sidecar record for packet 1 fate-bsf-jpegapp-missing: CMD = \ - cleanfiles="$${cleanfiles} $${outfile}.bin $${outfile}.mjpeg"; \ - printf "\\000\\000\\000\\000" > "$${outfile}.bin"; \ ffmpeg -xerror -i $(TARGET_SAMPLES)/smjpeg/scenwin.mjpg -map 0:v:0 -frames:v 3 -c:v copy \ - -bsf:v "jpegapp=filename=$$(target_path "$${outfile}.bin" | sed "s/:/\\\\\\\\:/g")" \ + -bsf:v "jpegapp=sidecar=00000000" \ -fflags +bitexact -f mjpeg -y "$$(target_path "$${outfile}.mjpeg")" ; \ test $$? -ne 0 @@ -28,10 +24,8 @@ FATE_JPEGAPP-$(call ALLYES, SMJPEG_DEMUXER JPEGAPP_BSF MJPEG_MUXER FILE_PROTOCOL fate-bsf-jpegapp-oversize: CMP = grep fate-bsf-jpegapp-oversize: REF = jpegapp: record length 16777217 too large fate-bsf-jpegapp-oversize: CMD = \ - cleanfiles="$${cleanfiles} $${outfile}.bin $${outfile}.mjpeg"; \ - printf "\\001\\000\\000\\001\\377\\341\\000\\006FATE" > "$${outfile}.bin"; \ ffmpeg -xerror -i $(TARGET_SAMPLES)/smjpeg/scenwin.mjpg -map 0:v:0 -frames:v 3 -c:v copy \ - -bsf:v "jpegapp=filename=$$(target_path "$${outfile}.bin" | sed "s/:/\\\\\\\\:/g")" \ + -bsf:v "jpegapp=sidecar=01000001FFE1000646415445" \ -fflags +bitexact -f mjpeg -y "$$(target_path "$${outfile}.mjpeg")" ; \ test $$? -ne 0 @@ -40,9 +34,7 @@ fate-bsf-jpegapp-oversize: CMD = \ # after the oversized prefix would form valid records if reading resumed. FATE_JPEGAPP-$(call ALLYES, SMJPEG_DEMUXER JPEGAPP_BSF MJPEG_MUXER FILE_PROTOCOL) += fate-bsf-jpegapp-oversize-continuation fate-bsf-jpegapp-oversize-continuation: CMD = \ - cleanfiles="$${cleanfiles} $${outfile}.bin"; \ - printf "\\000\\000\\000\\000\\001\\000\\000\\001\\000\\000\\000\\010\\377\\341\\000\\006FATE\\000\\000\\000\\000" > "$${outfile}.bin"; \ md5 -i $(TARGET_SAMPLES)/smjpeg/scenwin.mjpg -map 0:v:0 -frames:v 4 -c:v copy \ - -bsf:v "jpegapp=filename=$$(target_path "$${outfile}.bin" | sed "s/:/\\\\\\\\:/g")" -fflags +bitexact -f mjpeg + -bsf:v "jpegapp=sidecar=000000000100000100000008ffe100064641544500000000" -fflags +bitexact -f mjpeg FATE_SAMPLES_FFMPEG += $(FATE_JPEGAPP-yes) -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
