* [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer @ 2023-09-05 21:37 Paul B Mahol 2023-09-06 9:27 ` Andreas Rheinhardt 2023-09-10 12:02 ` Paul B Mahol 0 siblings, 2 replies; 17+ messages in thread From: Paul B Mahol @ 2023-09-05 21:37 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1: Type: text/plain, Size: 10 bytes --] Attached. [-- Attachment #2: 0001-avformat-add-CRI-USM-demuxer.patch --] [-- Type: text/x-patch, Size: 11757 bytes --] From fc0f592a04ffa99b94b1402905d0bcfb2b15270c Mon Sep 17 00:00:00 2001 From: Paul B Mahol <onemda@gmail.com> Date: Tue, 5 Sep 2023 16:53:32 +0200 Subject: [PATCH] avformat: add CRI USM demuxer Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavformat/Makefile | 1 + libavformat/allformats.c | 1 + libavformat/usmdec.c | 327 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 329 insertions(+) create mode 100644 libavformat/usmdec.c diff --git a/libavformat/Makefile b/libavformat/Makefile index cc1b12360a..329055ccfd 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -588,6 +588,7 @@ OBJS-$(CONFIG_TTY_DEMUXER) += tty.o sauce.o OBJS-$(CONFIG_TY_DEMUXER) += ty.o OBJS-$(CONFIG_TXD_DEMUXER) += txd.o OBJS-$(CONFIG_UNCODEDFRAMECRC_MUXER) += uncodedframecrcenc.o framehash.o +OBJS-$(CONFIG_USM_DEMUXER) += usmdec.o OBJS-$(CONFIG_V210_DEMUXER) += rawvideodec.o OBJS-$(CONFIG_V210X_DEMUXER) += rawvideodec.o OBJS-$(CONFIG_VAG_DEMUXER) += vag.o diff --git a/libavformat/allformats.c b/libavformat/allformats.c index f4210e4932..d4b505a5a3 100644 --- a/libavformat/allformats.c +++ b/libavformat/allformats.c @@ -471,6 +471,7 @@ extern const AVInputFormat ff_txd_demuxer; extern const AVInputFormat ff_tty_demuxer; extern const AVInputFormat ff_ty_demuxer; extern const FFOutputFormat ff_uncodedframecrc_muxer; +extern const AVInputFormat ff_usm_demuxer; extern const AVInputFormat ff_v210_demuxer; extern const AVInputFormat ff_v210x_demuxer; extern const AVInputFormat ff_vag_demuxer; diff --git a/libavformat/usmdec.c b/libavformat/usmdec.c new file mode 100644 index 0000000000..63b580b9ea --- /dev/null +++ b/libavformat/usmdec.c @@ -0,0 +1,327 @@ +/* + * USM demuxer + * Copyright (c) 2023 Paul B Mahol + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "libavutil/intreadwrite.h" +#include "libavcodec/bytestream.h" + +#include "avformat.h" +#include "internal.h" + +#define VIDEOI 0 +#define AUDIOI 1 + +typedef struct USMChannel { + int index; + int used; +} USMChannel; + +typedef struct USMDemuxContext { + USMChannel ch[2][256]; + int nb_channels[2]; + uint8_t *header; + unsigned header_size; +} USMDemuxContext; + +static int usm_probe(const AVProbeData *p) +{ + if (AV_RL32(p->buf) != MKTAG('C','R','I','D')) + return 0; + + if (AV_RL32(p->buf + 4) == 0) + return 0; + + return AVPROBE_SCORE_MAX / 3; +} + +static int usm_read_header(AVFormatContext *s) +{ + s->ctx_flags |= AVFMTCTX_NOHEADER; + return 0; +} + +static int parse_utf(AVFormatContext *s, AVIOContext *pb, + AVStream *st, AVCodecParameters *par, int is_audio) +{ + USMDemuxContext *usm = s->priv_data; + GetByteContext gb, ugb, sgb; + uint32_t chunk_type, chunk_size, offset; + uint32_t unique_offset, string_offset; + uint32_t byte_offset, payload_name_offset; + int nb_items, unique_size, nb_dictionaries; + AVRational fps = { 0 }; + int type; + + chunk_type = avio_rb32(pb); + chunk_size = avio_rb32(pb); + + if (chunk_type != MKBETAG('@','U','T','F')) + return AVERROR_INVALIDDATA; + + av_fast_malloc(&usm->header, &usm->header_size, + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); + if (!usm->header) + return AVERROR(ENOMEM); + + if (avio_read(pb, usm->header, chunk_size) != chunk_size) + return AVERROR_INVALIDDATA; + + bytestream2_init(&gb, usm->header, chunk_size); + ugb = gb; + sgb = gb; + unique_offset = bytestream2_get_be32(&gb); + string_offset = bytestream2_get_be32(&gb); + byte_offset = bytestream2_get_be32(&gb); + payload_name_offset = bytestream2_get_be32(&gb); + nb_items = bytestream2_get_be16(&gb); + unique_size = bytestream2_get_be16(&gb); + nb_dictionaries = bytestream2_get_be32(&gb); + if (nb_dictionaries == 0) + return AVERROR_INVALIDDATA; + + bytestream2_skip(&ugb, unique_offset); + bytestream2_skip(&sgb, string_offset); + + for (int i = 0; i < nb_items; i++) { + GetByteContext *xgb; + uint8_t key[256]; + int64_t value; + int n = 0; + + type = bytestream2_get_byte(&gb); + offset = bytestream2_get_be32(&gb); + + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET); + while (bytestream2_get_bytes_left(&sgb) > 0) { + key[n] = bytestream2_get_byte(&sgb); + if (!key[n]) + break; + if (n >= sizeof(key) - 1) + break; + n++; + } + key[n] = '\0'; + + if ((type >> 5) == 1) + xgb = &gb; + else + xgb = &ugb; + + switch (type & 0x1F) { + case 0x10: + case 0x11: + value = bytestream2_get_byte(xgb); + break; + case 0x12: + case 0x13: + value = bytestream2_get_be16(xgb); + break; + case 0x14: + case 0x15: + value = bytestream2_get_be32(xgb); + break; + case 0x16: + case 0x17: + value = bytestream2_get_be64(xgb); + break; + case 0x18: + value = av_int2float(bytestream2_get_be32(xgb)); + break; + case 0x19: + value = av_int2double(bytestream2_get_be64(xgb)); + break; + case 0x1A: + break; + } + + if (is_audio) { + if (!strcmp(key, "sampling_rate")) { + par->sample_rate = value; + avpriv_set_pts_info(st, 64, 1, value); + } else if (!strcmp(key, "num_channels")) { + par->ch_layout.nb_channels = value; + } else if (!strcmp(key, "total_samples")) { + st->duration = value; + } else if (!strcmp(key, "audio_codec")) { + switch (value) { + case 2: + par->codec_id = AV_CODEC_ID_ADPCM_ADX; + break; + case 4: + par->codec_id = AV_CODEC_ID_HCA; + break; + default: + av_log(s, AV_LOG_ERROR, "unsupported audio: %d\n", (int)value); + break; + } + } + } else { + if (!strcmp(key, "width")) { + par->width = value; + } else if (!strcmp(key, "height")) { + par->height = value; + } else if (!strcmp(key, "total_frames")) { + st->nb_frames = value; + } else if (!strcmp(key, "framerate_n")) { + fps.num = value; + } else if (!strcmp(key, "framerate_d")) { + fps.den = value; + } else if (!strcmp(key, "mpeg_codec")) { + switch (value) { + case 1: + par->codec_id = AV_CODEC_ID_MPEG1VIDEO; + break; + case 5: + par->codec_id = AV_CODEC_ID_H264; + break; + case 9: + par->codec_id = AV_CODEC_ID_VP9; + break; + default: + av_log(s, AV_LOG_ERROR, "unsupported video: %d\n", (int)value); + break; + } + } + } + } + + if (!is_audio && fps.num && fps.den) + avpriv_set_pts_info(st, 64, fps.den, fps.num); + + return 0; +} + +static int parse_chunk(AVFormatContext *s, AVIOContext *pb, + uint32_t chunk_type, uint32_t chunk_size, + AVPacket *pkt) +{ + USMDemuxContext *usm = s->priv_data; + int64_t chunk_start; + int stream_index, payload_type; + int payload_offset, frame_time; + int frame_rate, padding_size, ret; + int is_audio = chunk_type == MKBETAG('@','S','F','A'); + + chunk_start = avio_tell(pb); + avio_skip(pb, 1); + payload_offset = avio_r8(pb); + padding_size = avio_rb16(pb); + stream_index = avio_r8(pb); + avio_skip(pb, 2); + payload_type = avio_r8(pb); + frame_time = avio_rb32(pb); + frame_rate = avio_rb32(pb); + avio_skip(pb, 8); + + if (payload_type == 1) { + if (usm->ch[is_audio][stream_index].used == 0) { + AVStream *st = avformat_new_stream(s, NULL); + AVCodecParameters *par; + if (!st) + return AVERROR(ENOMEM); + + par = st->codecpar; + par->codec_type = is_audio ? AVMEDIA_TYPE_AUDIO : AVMEDIA_TYPE_VIDEO; + + usm->ch[is_audio][stream_index].index = st->index; + usm->ch[is_audio][stream_index].used = 1; + usm->nb_channels[is_audio]++; + + ret = parse_utf(s, pb, st, par, is_audio); + if (ret < 0) + return ret; + + if (!st->time_base.num || !st->time_base.den) + avpriv_set_pts_info(st, 64, 100, frame_rate); + ffstream(st)->need_parsing = AVSTREAM_PARSE_FULL_RAW; + } + } else if (payload_type == 0) { + if (usm->ch[is_audio][stream_index].used == 1) { + uint32_t pkt_size = chunk_size - (avio_tell(pb) - chunk_start); + + ret = av_get_packet(pb, pkt, pkt_size); + if (ret < 0) + return ret; + + pkt->stream_index = usm->ch[is_audio][stream_index].index; + + return 1; + } + } + + avio_skip(pb, padding_size); + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); + + return 0; +} + +static int usm_read_packet(AVFormatContext *s, AVPacket *pkt) +{ + AVIOContext *pb = s->pb; + int64_t ret = AVERROR_EOF; + + while (!avio_feof(pb)) { + uint32_t chunk_type, chunk_size; + int got_packet = 0; + + chunk_type = avio_rb32(pb); + chunk_size = avio_rb32(pb); + + switch (chunk_type) { + case MKBETAG('C','R','I','D'): + default: + ret = avio_skip(pb, chunk_size); + break; + case MKBETAG('@','S','F','A'): + case MKBETAG('@','S','F','V'): + ret = parse_chunk(s, pb, chunk_type, chunk_size, pkt); + got_packet = ret == 1; + break; + } + + if (got_packet) + ret = 0; + + if (got_packet || ret < 0) + break; + } + + return ret; +} + +static int usm_read_close(AVFormatContext *s) +{ + USMDemuxContext *usm = s->priv_data; + av_freep(&usm->header); + usm->header_size = 0; + return 0; +} + +const AVInputFormat ff_usm_demuxer = { + .name = "usm", + .long_name = NULL_IF_CONFIG_SMALL("CRI USM"), + .priv_data_size = sizeof(USMDemuxContext), + .read_probe = usm_probe, + .read_header = usm_read_header, + .read_packet = usm_read_packet, + .read_close = usm_read_close, + .extensions = "usm", + .flags = AVFMT_GENERIC_INDEX, +}; -- 2.39.1 [-- Attachment #3: Type: text/plain, Size: 251 bytes --] _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-05 21:37 [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer Paul B Mahol @ 2023-09-06 9:27 ` Andreas Rheinhardt 2023-09-06 9:57 ` Paul B Mahol 2023-09-10 12:02 ` Paul B Mahol 1 sibling, 1 reply; 17+ messages in thread From: Andreas Rheinhardt @ 2023-09-06 9:27 UTC (permalink / raw) To: ffmpeg-devel Paul B Mahol: > > + chunk_type = avio_rb32(pb); > + chunk_size = avio_rb32(pb); You are not checking whether the chunk here exceeds its containing chunk. > > + av_fast_malloc(&usm->header, &usm->header_size, > + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); > + if (!usm->header) > + return AVERROR(ENOMEM); The bytestream2 API does not rely on the buffer being padded at all. > > + bytestream2_skip(&sgb, string_offset); This is unnecessary, because you seek with an absolute offset lateron anyway before using sgb. > > + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET); > + while (bytestream2_get_bytes_left(&sgb) > 0) { > + key[n] = bytestream2_get_byte(&sgb); > + if (!key[n]) > + break; > + if (n >= sizeof(key) - 1) > + break; > + n++; > + } > + key[n] = '\0'; IMO this would be easier with strnlen(), avoiding sgb altogether. You would of course need to explicitly check that you are not overreading, but that is good practice anyway. > > + chunk_start = avio_tell(pb); > + avio_skip(pb, 1); > + payload_offset = avio_r8(pb); > + padding_size = avio_rb16(pb); > + stream_index = avio_r8(pb); > + avio_skip(pb, 2); > + payload_type = avio_r8(pb); > + frame_time = avio_rb32(pb); > + frame_rate = avio_rb32(pb); > + avio_skip(pb, 8); payload_offset and frame_time are set-but-unused; this might lead to compiler warnings. > + if (usm->ch[is_audio][stream_index].used == 1) { > + uint32_t pkt_size = chunk_size - (avio_tell(pb) - chunk_start); > + This is unnecessary: Unless we already had a read error, pkt_size is chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). (Notice that in case padding_size is > 0, it will be part of the packet with the current code; not sure if that is an issue.) > > + > + avio_skip(pb, padding_size); > + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); > + Simpler to just use avio_seek(pb, chunk_start + chunk_size, SEEK_SET); - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-06 9:27 ` Andreas Rheinhardt @ 2023-09-06 9:57 ` Paul B Mahol 2023-09-06 11:31 ` Andreas Rheinhardt 0 siblings, 1 reply; 17+ messages in thread From: Paul B Mahol @ 2023-09-06 9:57 UTC (permalink / raw) To: FFmpeg development discussions and patches On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > Paul B Mahol: > > > > + chunk_type = avio_rb32(pb); > > + chunk_size = avio_rb32(pb); > > You are not checking whether the chunk here exceeds its containing chunk. > > > > > + av_fast_malloc(&usm->header, &usm->header_size, > > + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); > > + if (!usm->header) > > + return AVERROR(ENOMEM); > > The bytestream2 API does not rely on the buffer being padded at all. > > > > > + bytestream2_skip(&sgb, string_offset); > > This is unnecessary, because you seek with an absolute offset lateron > anyway before using sgb. > > > > > + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET); > > + while (bytestream2_get_bytes_left(&sgb) > 0) { > > + key[n] = bytestream2_get_byte(&sgb); > > + if (!key[n]) > > + break; > > + if (n >= sizeof(key) - 1) > > + break; > > + n++; > > + } > > + key[n] = '\0'; > > IMO this would be easier with strnlen(), avoiding sgb altogether. > You would of course need to explicitly check that you are not > overreading, but that is good practice anyway. > > > > > + chunk_start = avio_tell(pb); > > + avio_skip(pb, 1); > > + payload_offset = avio_r8(pb); > > + padding_size = avio_rb16(pb); > > + stream_index = avio_r8(pb); > > + avio_skip(pb, 2); > > + payload_type = avio_r8(pb); > > + frame_time = avio_rb32(pb); > > + frame_rate = avio_rb32(pb); > > + avio_skip(pb, 8); > > payload_offset and frame_time are set-but-unused; this might lead to > compiler warnings. > > > + if (usm->ch[is_audio][stream_index].used == 1) { > > + uint32_t pkt_size = chunk_size - (avio_tell(pb) - > chunk_start); > > + > > This is unnecessary: Unless we already had a read error, pkt_size is > chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). > > (Notice that in case padding_size is > 0, it will be part of the packet > with the current code; not sure if that is an issue.) > > > > > + > > + avio_skip(pb, padding_size); > > + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); > > + > > Simpler to just use avio_seek(pb, chunk_start + chunk_size, SEEK_SET); > But input might not be seekable. > > - Andreas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-06 9:57 ` Paul B Mahol @ 2023-09-06 11:31 ` Andreas Rheinhardt 2023-09-06 12:01 ` Paul B Mahol 0 siblings, 1 reply; 17+ messages in thread From: Andreas Rheinhardt @ 2023-09-06 11:31 UTC (permalink / raw) To: ffmpeg-devel Paul B Mahol: > On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < > andreas.rheinhardt@outlook.com> wrote: > >> Paul B Mahol: >>> >>> + chunk_type = avio_rb32(pb); >>> + chunk_size = avio_rb32(pb); >> >> You are not checking whether the chunk here exceeds its containing chunk. >> >>> >>> + av_fast_malloc(&usm->header, &usm->header_size, >>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); >>> + if (!usm->header) >>> + return AVERROR(ENOMEM); >> >> The bytestream2 API does not rely on the buffer being padded at all. >> >>> >>> + bytestream2_skip(&sgb, string_offset); >> >> This is unnecessary, because you seek with an absolute offset lateron >> anyway before using sgb. >> >>> >>> + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET); >>> + while (bytestream2_get_bytes_left(&sgb) > 0) { >>> + key[n] = bytestream2_get_byte(&sgb); >>> + if (!key[n]) >>> + break; >>> + if (n >= sizeof(key) - 1) >>> + break; >>> + n++; >>> + } >>> + key[n] = '\0'; >> >> IMO this would be easier with strnlen(), avoiding sgb altogether. >> You would of course need to explicitly check that you are not >> overreading, but that is good practice anyway. >> >>> >>> + chunk_start = avio_tell(pb); >>> + avio_skip(pb, 1); >>> + payload_offset = avio_r8(pb); >>> + padding_size = avio_rb16(pb); >>> + stream_index = avio_r8(pb); >>> + avio_skip(pb, 2); >>> + payload_type = avio_r8(pb); >>> + frame_time = avio_rb32(pb); >>> + frame_rate = avio_rb32(pb); >>> + avio_skip(pb, 8); >> >> payload_offset and frame_time are set-but-unused; this might lead to >> compiler warnings. >> >>> + if (usm->ch[is_audio][stream_index].used == 1) { >>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) - >> chunk_start); >>> + >> >> This is unnecessary: Unless we already had a read error, pkt_size is >> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). >> >> (Notice that in case padding_size is > 0, it will be part of the packet >> with the current code; not sure if that is an issue.) >> >>> >>> + >>> + avio_skip(pb, padding_size); >>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); >>> + >> >> Simpler to just use avio_seek(pb, chunk_start + chunk_size, SEEK_SET); >> > > But input might not be seekable. > And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset, SEEK_CUR)? - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-06 11:31 ` Andreas Rheinhardt @ 2023-09-06 12:01 ` Paul B Mahol 2023-09-10 12:00 ` Andreas Rheinhardt 0 siblings, 1 reply; 17+ messages in thread From: Paul B Mahol @ 2023-09-06 12:01 UTC (permalink / raw) To: FFmpeg development discussions and patches On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > Paul B Mahol: > > On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < > > andreas.rheinhardt@outlook.com> wrote: > > > >> Paul B Mahol: > >>> > >>> + chunk_type = avio_rb32(pb); > >>> + chunk_size = avio_rb32(pb); > >> > >> You are not checking whether the chunk here exceeds its containing > chunk. > >> > >>> > >>> + av_fast_malloc(&usm->header, &usm->header_size, > >>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); > >>> + if (!usm->header) > >>> + return AVERROR(ENOMEM); > >> > >> The bytestream2 API does not rely on the buffer being padded at all. > >> > >>> > >>> + bytestream2_skip(&sgb, string_offset); > >> > >> This is unnecessary, because you seek with an absolute offset lateron > >> anyway before using sgb. > >> > >>> > >>> + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET); > >>> + while (bytestream2_get_bytes_left(&sgb) > 0) { > >>> + key[n] = bytestream2_get_byte(&sgb); > >>> + if (!key[n]) > >>> + break; > >>> + if (n >= sizeof(key) - 1) > >>> + break; > >>> + n++; > >>> + } > >>> + key[n] = '\0'; > >> > >> IMO this would be easier with strnlen(), avoiding sgb altogether. > >> You would of course need to explicitly check that you are not > >> overreading, but that is good practice anyway. > >> > >>> > >>> + chunk_start = avio_tell(pb); > >>> + avio_skip(pb, 1); > >>> + payload_offset = avio_r8(pb); > >>> + padding_size = avio_rb16(pb); > >>> + stream_index = avio_r8(pb); > >>> + avio_skip(pb, 2); > >>> + payload_type = avio_r8(pb); > >>> + frame_time = avio_rb32(pb); > >>> + frame_rate = avio_rb32(pb); > >>> + avio_skip(pb, 8); > >> > >> payload_offset and frame_time are set-but-unused; this might lead to > >> compiler warnings. > >> > >>> + if (usm->ch[is_audio][stream_index].used == 1) { > >>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) - > >> chunk_start); > >>> + > >> > >> This is unnecessary: Unless we already had a read error, pkt_size is > >> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). > >> > >> (Notice that in case padding_size is > 0, it will be part of the packet > >> with the current code; not sure if that is an issue.) > >> > >>> > >>> + > >>> + avio_skip(pb, padding_size); > >>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); > >>> + > >> > >> Simpler to just use avio_seek(pb, chunk_start + chunk_size, SEEK_SET); > >> > > > > But input might not be seekable. > > > > And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset, > SEEK_CUR)? > And? Do you know that SEEK_SET is different from SEEK_CUR with positive argument. > > - Andreas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-06 12:01 ` Paul B Mahol @ 2023-09-10 12:00 ` Andreas Rheinhardt 2023-09-10 12:10 ` Paul B Mahol 0 siblings, 1 reply; 17+ messages in thread From: Andreas Rheinhardt @ 2023-09-10 12:00 UTC (permalink / raw) To: ffmpeg-devel Paul B Mahol: > On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt < > andreas.rheinhardt@outlook.com> wrote: > >> Paul B Mahol: >>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < >>> andreas.rheinhardt@outlook.com> wrote: >>> >>>> Paul B Mahol: >>>>> >>>>> + chunk_type = avio_rb32(pb); >>>>> + chunk_size = avio_rb32(pb); >>>> >>>> You are not checking whether the chunk here exceeds its containing >> chunk. >>>> >>>>> >>>>> + av_fast_malloc(&usm->header, &usm->header_size, >>>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); >>>>> + if (!usm->header) >>>>> + return AVERROR(ENOMEM); >>>> >>>> The bytestream2 API does not rely on the buffer being padded at all. >>>> >>>>> >>>>> + bytestream2_skip(&sgb, string_offset); >>>> >>>> This is unnecessary, because you seek with an absolute offset lateron >>>> anyway before using sgb. >>>> >>>>> >>>>> + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET); >>>>> + while (bytestream2_get_bytes_left(&sgb) > 0) { >>>>> + key[n] = bytestream2_get_byte(&sgb); >>>>> + if (!key[n]) >>>>> + break; >>>>> + if (n >= sizeof(key) - 1) >>>>> + break; >>>>> + n++; >>>>> + } >>>>> + key[n] = '\0'; >>>> >>>> IMO this would be easier with strnlen(), avoiding sgb altogether. >>>> You would of course need to explicitly check that you are not >>>> overreading, but that is good practice anyway. >>>> >>>>> >>>>> + chunk_start = avio_tell(pb); >>>>> + avio_skip(pb, 1); >>>>> + payload_offset = avio_r8(pb); >>>>> + padding_size = avio_rb16(pb); >>>>> + stream_index = avio_r8(pb); >>>>> + avio_skip(pb, 2); >>>>> + payload_type = avio_r8(pb); >>>>> + frame_time = avio_rb32(pb); >>>>> + frame_rate = avio_rb32(pb); >>>>> + avio_skip(pb, 8); >>>> >>>> payload_offset and frame_time are set-but-unused; this might lead to >>>> compiler warnings. >>>> >>>>> + if (usm->ch[is_audio][stream_index].used == 1) { >>>>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) - >>>> chunk_start); >>>>> + >>>> >>>> This is unnecessary: Unless we already had a read error, pkt_size is >>>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). >>>> >>>> (Notice that in case padding_size is > 0, it will be part of the packet >>>> with the current code; not sure if that is an issue.) >>>> >>>>> >>>>> + >>>>> + avio_skip(pb, padding_size); >>>>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); >>>>> + >>>> >>>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, SEEK_SET); >>>> >>> >>> But input might not be seekable. >>> >> >> And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset, >> SEEK_CUR)? >> > > And? Do you know that SEEK_SET is different from SEEK_CUR with positive > argument. > You are using SEEK_CUR with -avio_tell(pb), which effectively makes it an absolute seek. - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-10 12:00 ` Andreas Rheinhardt @ 2023-09-10 12:10 ` Paul B Mahol 2023-09-10 12:07 ` Andreas Rheinhardt 0 siblings, 1 reply; 17+ messages in thread From: Paul B Mahol @ 2023-09-10 12:10 UTC (permalink / raw) To: FFmpeg development discussions and patches On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > Paul B Mahol: > > On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt < > > andreas.rheinhardt@outlook.com> wrote: > > > >> Paul B Mahol: > >>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < > >>> andreas.rheinhardt@outlook.com> wrote: > >>> > >>>> Paul B Mahol: > >>>>> > >>>>> + chunk_type = avio_rb32(pb); > >>>>> + chunk_size = avio_rb32(pb); > >>>> > >>>> You are not checking whether the chunk here exceeds its containing > >> chunk. > >>>> > >>>>> > >>>>> + av_fast_malloc(&usm->header, &usm->header_size, > >>>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); > >>>>> + if (!usm->header) > >>>>> + return AVERROR(ENOMEM); > >>>> > >>>> The bytestream2 API does not rely on the buffer being padded at all. > >>>> > >>>>> > >>>>> + bytestream2_skip(&sgb, string_offset); > >>>> > >>>> This is unnecessary, because you seek with an absolute offset lateron > >>>> anyway before using sgb. > >>>> > >>>>> > >>>>> + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET); > >>>>> + while (bytestream2_get_bytes_left(&sgb) > 0) { > >>>>> + key[n] = bytestream2_get_byte(&sgb); > >>>>> + if (!key[n]) > >>>>> + break; > >>>>> + if (n >= sizeof(key) - 1) > >>>>> + break; > >>>>> + n++; > >>>>> + } > >>>>> + key[n] = '\0'; > >>>> > >>>> IMO this would be easier with strnlen(), avoiding sgb altogether. > >>>> You would of course need to explicitly check that you are not > >>>> overreading, but that is good practice anyway. > >>>> > >>>>> > >>>>> + chunk_start = avio_tell(pb); > >>>>> + avio_skip(pb, 1); > >>>>> + payload_offset = avio_r8(pb); > >>>>> + padding_size = avio_rb16(pb); > >>>>> + stream_index = avio_r8(pb); > >>>>> + avio_skip(pb, 2); > >>>>> + payload_type = avio_r8(pb); > >>>>> + frame_time = avio_rb32(pb); > >>>>> + frame_rate = avio_rb32(pb); > >>>>> + avio_skip(pb, 8); > >>>> > >>>> payload_offset and frame_time are set-but-unused; this might lead to > >>>> compiler warnings. > >>>> > >>>>> + if (usm->ch[is_audio][stream_index].used == 1) { > >>>>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) - > >>>> chunk_start); > >>>>> + > >>>> > >>>> This is unnecessary: Unless we already had a read error, pkt_size is > >>>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). > >>>> > >>>> (Notice that in case padding_size is > 0, it will be part of the > packet > >>>> with the current code; not sure if that is an issue.) > >>>> > >>>>> > >>>>> + > >>>>> + avio_skip(pb, padding_size); > >>>>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); > >>>>> + > >>>> > >>>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, SEEK_SET); > >>>> > >>> > >>> But input might not be seekable. > >>> > >> > >> And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset, > >> SEEK_CUR)? > >> > > > > And? Do you know that SEEK_SET is different from SEEK_CUR with positive > > argument. > > > > You are using SEEK_CUR with -avio_tell(pb), which effectively makes it > an absolute seek. > Nope, I skip data only, not seeking backward ever. > > - Andreas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-10 12:10 ` Paul B Mahol @ 2023-09-10 12:07 ` Andreas Rheinhardt 2023-09-10 12:15 ` Paul B Mahol 0 siblings, 1 reply; 17+ messages in thread From: Andreas Rheinhardt @ 2023-09-10 12:07 UTC (permalink / raw) To: ffmpeg-devel Paul B Mahol: > On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt < > andreas.rheinhardt@outlook.com> wrote: > >> Paul B Mahol: >>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt < >>> andreas.rheinhardt@outlook.com> wrote: >>> >>>> Paul B Mahol: >>>>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < >>>>> andreas.rheinhardt@outlook.com> wrote: >>>>> >>>>>> Paul B Mahol: >>>>>>> >>>>>>> + chunk_type = avio_rb32(pb); >>>>>>> + chunk_size = avio_rb32(pb); >>>>>> >>>>>> You are not checking whether the chunk here exceeds its containing >>>> chunk. >>>>>> >>>>>>> >>>>>>> + av_fast_malloc(&usm->header, &usm->header_size, >>>>>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); >>>>>>> + if (!usm->header) >>>>>>> + return AVERROR(ENOMEM); >>>>>> >>>>>> The bytestream2 API does not rely on the buffer being padded at all. >>>>>> >>>>>>> >>>>>>> + bytestream2_skip(&sgb, string_offset); >>>>>> >>>>>> This is unnecessary, because you seek with an absolute offset lateron >>>>>> anyway before using sgb. >>>>>> >>>>>>> >>>>>>> + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET); >>>>>>> + while (bytestream2_get_bytes_left(&sgb) > 0) { >>>>>>> + key[n] = bytestream2_get_byte(&sgb); >>>>>>> + if (!key[n]) >>>>>>> + break; >>>>>>> + if (n >= sizeof(key) - 1) >>>>>>> + break; >>>>>>> + n++; >>>>>>> + } >>>>>>> + key[n] = '\0'; >>>>>> >>>>>> IMO this would be easier with strnlen(), avoiding sgb altogether. >>>>>> You would of course need to explicitly check that you are not >>>>>> overreading, but that is good practice anyway. >>>>>> >>>>>>> >>>>>>> + chunk_start = avio_tell(pb); >>>>>>> + avio_skip(pb, 1); >>>>>>> + payload_offset = avio_r8(pb); >>>>>>> + padding_size = avio_rb16(pb); >>>>>>> + stream_index = avio_r8(pb); >>>>>>> + avio_skip(pb, 2); >>>>>>> + payload_type = avio_r8(pb); >>>>>>> + frame_time = avio_rb32(pb); >>>>>>> + frame_rate = avio_rb32(pb); >>>>>>> + avio_skip(pb, 8); >>>>>> >>>>>> payload_offset and frame_time are set-but-unused; this might lead to >>>>>> compiler warnings. >>>>>> >>>>>>> + if (usm->ch[is_audio][stream_index].used == 1) { >>>>>>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) - >>>>>> chunk_start); >>>>>>> + >>>>>> >>>>>> This is unnecessary: Unless we already had a read error, pkt_size is >>>>>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). >>>>>> >>>>>> (Notice that in case padding_size is > 0, it will be part of the >> packet >>>>>> with the current code; not sure if that is an issue.) >>>>>> >>>>>>> >>>>>>> + >>>>>>> + avio_skip(pb, padding_size); >>>>>>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); >>>>>>> + >>>>>> >>>>>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, SEEK_SET); >>>>>> >>>>> >>>>> But input might not be seekable. >>>>> >>>> >>>> And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset, >>>> SEEK_CUR)? >>>> >>> >>> And? Do you know that SEEK_SET is different from SEEK_CUR with positive >>> argument. >>> >> >> You are using SEEK_CUR with -avio_tell(pb), which effectively makes it >> an absolute seek. >> > > Nope, I skip data only, not seeking backward ever. > avio_seek() internally translates any avio_seek() with SEEK_CUR to an absolute seek (as required by the seek callbacks) and does not care about whether it is seeking forward or backward. - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-10 12:07 ` Andreas Rheinhardt @ 2023-09-10 12:15 ` Paul B Mahol 2023-09-10 12:19 ` Andreas Rheinhardt 0 siblings, 1 reply; 17+ messages in thread From: Paul B Mahol @ 2023-09-10 12:15 UTC (permalink / raw) To: FFmpeg development discussions and patches On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > Paul B Mahol: > > On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt < > > andreas.rheinhardt@outlook.com> wrote: > > > >> Paul B Mahol: > >>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt < > >>> andreas.rheinhardt@outlook.com> wrote: > >>> > >>>> Paul B Mahol: > >>>>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < > >>>>> andreas.rheinhardt@outlook.com> wrote: > >>>>> > >>>>>> Paul B Mahol: > >>>>>>> > >>>>>>> + chunk_type = avio_rb32(pb); > >>>>>>> + chunk_size = avio_rb32(pb); > >>>>>> > >>>>>> You are not checking whether the chunk here exceeds its containing > >>>> chunk. > >>>>>> > >>>>>>> > >>>>>>> + av_fast_malloc(&usm->header, &usm->header_size, > >>>>>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); > >>>>>>> + if (!usm->header) > >>>>>>> + return AVERROR(ENOMEM); > >>>>>> > >>>>>> The bytestream2 API does not rely on the buffer being padded at all. > >>>>>> > >>>>>>> > >>>>>>> + bytestream2_skip(&sgb, string_offset); > >>>>>> > >>>>>> This is unnecessary, because you seek with an absolute offset > lateron > >>>>>> anyway before using sgb. > >>>>>> > >>>>>>> > >>>>>>> + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET); > >>>>>>> + while (bytestream2_get_bytes_left(&sgb) > 0) { > >>>>>>> + key[n] = bytestream2_get_byte(&sgb); > >>>>>>> + if (!key[n]) > >>>>>>> + break; > >>>>>>> + if (n >= sizeof(key) - 1) > >>>>>>> + break; > >>>>>>> + n++; > >>>>>>> + } > >>>>>>> + key[n] = '\0'; > >>>>>> > >>>>>> IMO this would be easier with strnlen(), avoiding sgb altogether. > >>>>>> You would of course need to explicitly check that you are not > >>>>>> overreading, but that is good practice anyway. > >>>>>> > >>>>>>> > >>>>>>> + chunk_start = avio_tell(pb); > >>>>>>> + avio_skip(pb, 1); > >>>>>>> + payload_offset = avio_r8(pb); > >>>>>>> + padding_size = avio_rb16(pb); > >>>>>>> + stream_index = avio_r8(pb); > >>>>>>> + avio_skip(pb, 2); > >>>>>>> + payload_type = avio_r8(pb); > >>>>>>> + frame_time = avio_rb32(pb); > >>>>>>> + frame_rate = avio_rb32(pb); > >>>>>>> + avio_skip(pb, 8); > >>>>>> > >>>>>> payload_offset and frame_time are set-but-unused; this might lead to > >>>>>> compiler warnings. > >>>>>> > >>>>>>> + if (usm->ch[is_audio][stream_index].used == 1) { > >>>>>>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) - > >>>>>> chunk_start); > >>>>>>> + > >>>>>> > >>>>>> This is unnecessary: Unless we already had a read error, pkt_size is > >>>>>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). > >>>>>> > >>>>>> (Notice that in case padding_size is > 0, it will be part of the > >> packet > >>>>>> with the current code; not sure if that is an issue.) > >>>>>> > >>>>>>> > >>>>>>> + > >>>>>>> + avio_skip(pb, padding_size); > >>>>>>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); > >>>>>>> + > >>>>>> > >>>>>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, > SEEK_SET); > >>>>>> > >>>>> > >>>>> But input might not be seekable. > >>>>> > >>>> > >>>> And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset, > >>>> SEEK_CUR)? > >>>> > >>> > >>> And? Do you know that SEEK_SET is different from SEEK_CUR with positive > >>> argument. > >>> > >> > >> You are using SEEK_CUR with -avio_tell(pb), which effectively makes it > >> an absolute seek. > >> > > > > Nope, I skip data only, not seeking backward ever. > > > > avio_seek() internally translates any avio_seek() with SEEK_CUR to an > absolute seek (as required by the seek callbacks) and does not care > about whether it is seeking forward or backward. > Irrelevant. Seeking needs enough cache to work on non-seekable input. > > - Andreas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-10 12:15 ` Paul B Mahol @ 2023-09-10 12:19 ` Andreas Rheinhardt 2023-09-10 12:29 ` Paul B Mahol 0 siblings, 1 reply; 17+ messages in thread From: Andreas Rheinhardt @ 2023-09-10 12:19 UTC (permalink / raw) To: ffmpeg-devel Paul B Mahol: > On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt < > andreas.rheinhardt@outlook.com> wrote: > >> Paul B Mahol: >>> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt < >>> andreas.rheinhardt@outlook.com> wrote: >>> >>>> Paul B Mahol: >>>>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt < >>>>> andreas.rheinhardt@outlook.com> wrote: >>>>> >>>>>> Paul B Mahol: >>>>>>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < >>>>>>> andreas.rheinhardt@outlook.com> wrote: >>>>>>> >>>>>>>> Paul B Mahol: >>>>>>>>> >>>>>>>>> + chunk_type = avio_rb32(pb); >>>>>>>>> + chunk_size = avio_rb32(pb); >>>>>>>> >>>>>>>> You are not checking whether the chunk here exceeds its containing >>>>>> chunk. >>>>>>>> >>>>>>>>> >>>>>>>>> + av_fast_malloc(&usm->header, &usm->header_size, >>>>>>>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); >>>>>>>>> + if (!usm->header) >>>>>>>>> + return AVERROR(ENOMEM); >>>>>>>> >>>>>>>> The bytestream2 API does not rely on the buffer being padded at all. >>>>>>>> >>>>>>>>> >>>>>>>>> + bytestream2_skip(&sgb, string_offset); >>>>>>>> >>>>>>>> This is unnecessary, because you seek with an absolute offset >> lateron >>>>>>>> anyway before using sgb. >>>>>>>> >>>>>>>>> >>>>>>>>> + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET); >>>>>>>>> + while (bytestream2_get_bytes_left(&sgb) > 0) { >>>>>>>>> + key[n] = bytestream2_get_byte(&sgb); >>>>>>>>> + if (!key[n]) >>>>>>>>> + break; >>>>>>>>> + if (n >= sizeof(key) - 1) >>>>>>>>> + break; >>>>>>>>> + n++; >>>>>>>>> + } >>>>>>>>> + key[n] = '\0'; >>>>>>>> >>>>>>>> IMO this would be easier with strnlen(), avoiding sgb altogether. >>>>>>>> You would of course need to explicitly check that you are not >>>>>>>> overreading, but that is good practice anyway. >>>>>>>> >>>>>>>>> >>>>>>>>> + chunk_start = avio_tell(pb); >>>>>>>>> + avio_skip(pb, 1); >>>>>>>>> + payload_offset = avio_r8(pb); >>>>>>>>> + padding_size = avio_rb16(pb); >>>>>>>>> + stream_index = avio_r8(pb); >>>>>>>>> + avio_skip(pb, 2); >>>>>>>>> + payload_type = avio_r8(pb); >>>>>>>>> + frame_time = avio_rb32(pb); >>>>>>>>> + frame_rate = avio_rb32(pb); >>>>>>>>> + avio_skip(pb, 8); >>>>>>>> >>>>>>>> payload_offset and frame_time are set-but-unused; this might lead to >>>>>>>> compiler warnings. >>>>>>>> >>>>>>>>> + if (usm->ch[is_audio][stream_index].used == 1) { >>>>>>>>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) - >>>>>>>> chunk_start); >>>>>>>>> + >>>>>>>> >>>>>>>> This is unnecessary: Unless we already had a read error, pkt_size is >>>>>>>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). >>>>>>>> >>>>>>>> (Notice that in case padding_size is > 0, it will be part of the >>>> packet >>>>>>>> with the current code; not sure if that is an issue.) >>>>>>>> >>>>>>>>> >>>>>>>>> + >>>>>>>>> + avio_skip(pb, padding_size); >>>>>>>>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); >>>>>>>>> + >>>>>>>> >>>>>>>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, >> SEEK_SET); >>>>>>>> >>>>>>> >>>>>>> But input might not be seekable. >>>>>>> >>>>>> >>>>>> And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset, >>>>>> SEEK_CUR)? >>>>>> >>>>> >>>>> And? Do you know that SEEK_SET is different from SEEK_CUR with positive >>>>> argument. >>>>> >>>> >>>> You are using SEEK_CUR with -avio_tell(pb), which effectively makes it >>>> an absolute seek. >>>> >>> >>> Nope, I skip data only, not seeking backward ever. >>> >> >> avio_seek() internally translates any avio_seek() with SEEK_CUR to an >> absolute seek (as required by the seek callbacks) and does not care >> about whether it is seeking forward or backward. >> > > Irrelevant. Seeking needs enough cache to work on non-seekable input. > Given that avio_skip() is just a wrapper around avio_seek(), the behaviour will not change even when the input is non-seekable. - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-10 12:19 ` Andreas Rheinhardt @ 2023-09-10 12:29 ` Paul B Mahol 2023-09-10 12:55 ` Andreas Rheinhardt 0 siblings, 1 reply; 17+ messages in thread From: Paul B Mahol @ 2023-09-10 12:29 UTC (permalink / raw) To: FFmpeg development discussions and patches On Sun, Sep 10, 2023 at 2:18 PM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > Paul B Mahol: > > On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt < > > andreas.rheinhardt@outlook.com> wrote: > > > >> Paul B Mahol: > >>> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt < > >>> andreas.rheinhardt@outlook.com> wrote: > >>> > >>>> Paul B Mahol: > >>>>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt < > >>>>> andreas.rheinhardt@outlook.com> wrote: > >>>>> > >>>>>> Paul B Mahol: > >>>>>>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < > >>>>>>> andreas.rheinhardt@outlook.com> wrote: > >>>>>>> > >>>>>>>> Paul B Mahol: > >>>>>>>>> > >>>>>>>>> + chunk_type = avio_rb32(pb); > >>>>>>>>> + chunk_size = avio_rb32(pb); > >>>>>>>> > >>>>>>>> You are not checking whether the chunk here exceeds its containing > >>>>>> chunk. > >>>>>>>> > >>>>>>>>> > >>>>>>>>> + av_fast_malloc(&usm->header, &usm->header_size, > >>>>>>>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); > >>>>>>>>> + if (!usm->header) > >>>>>>>>> + return AVERROR(ENOMEM); > >>>>>>>> > >>>>>>>> The bytestream2 API does not rely on the buffer being padded at > all. > >>>>>>>> > >>>>>>>>> > >>>>>>>>> + bytestream2_skip(&sgb, string_offset); > >>>>>>>> > >>>>>>>> This is unnecessary, because you seek with an absolute offset > >> lateron > >>>>>>>> anyway before using sgb. > >>>>>>>> > >>>>>>>>> > >>>>>>>>> + bytestream2_seek(&sgb, string_offset + offset, > SEEK_SET); > >>>>>>>>> + while (bytestream2_get_bytes_left(&sgb) > 0) { > >>>>>>>>> + key[n] = bytestream2_get_byte(&sgb); > >>>>>>>>> + if (!key[n]) > >>>>>>>>> + break; > >>>>>>>>> + if (n >= sizeof(key) - 1) > >>>>>>>>> + break; > >>>>>>>>> + n++; > >>>>>>>>> + } > >>>>>>>>> + key[n] = '\0'; > >>>>>>>> > >>>>>>>> IMO this would be easier with strnlen(), avoiding sgb altogether. > >>>>>>>> You would of course need to explicitly check that you are not > >>>>>>>> overreading, but that is good practice anyway. > >>>>>>>> > >>>>>>>>> > >>>>>>>>> + chunk_start = avio_tell(pb); > >>>>>>>>> + avio_skip(pb, 1); > >>>>>>>>> + payload_offset = avio_r8(pb); > >>>>>>>>> + padding_size = avio_rb16(pb); > >>>>>>>>> + stream_index = avio_r8(pb); > >>>>>>>>> + avio_skip(pb, 2); > >>>>>>>>> + payload_type = avio_r8(pb); > >>>>>>>>> + frame_time = avio_rb32(pb); > >>>>>>>>> + frame_rate = avio_rb32(pb); > >>>>>>>>> + avio_skip(pb, 8); > >>>>>>>> > >>>>>>>> payload_offset and frame_time are set-but-unused; this might lead > to > >>>>>>>> compiler warnings. > >>>>>>>> > >>>>>>>>> + if (usm->ch[is_audio][stream_index].used == 1) { > >>>>>>>>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) - > >>>>>>>> chunk_start); > >>>>>>>>> + > >>>>>>>> > >>>>>>>> This is unnecessary: Unless we already had a read error, pkt_size > is > >>>>>>>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). > >>>>>>>> > >>>>>>>> (Notice that in case padding_size is > 0, it will be part of the > >>>> packet > >>>>>>>> with the current code; not sure if that is an issue.) > >>>>>>>> > >>>>>>>>> > >>>>>>>>> + > >>>>>>>>> + avio_skip(pb, padding_size); > >>>>>>>>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); > >>>>>>>>> + > >>>>>>>> > >>>>>>>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, > >> SEEK_SET); > >>>>>>>> > >>>>>>> > >>>>>>> But input might not be seekable. > >>>>>>> > >>>>>> > >>>>>> And? You know that avio_skip(pb, offset) is just avio_seek(pb, > offset, > >>>>>> SEEK_CUR)? > >>>>>> > >>>>> > >>>>> And? Do you know that SEEK_SET is different from SEEK_CUR with > positive > >>>>> argument. > >>>>> > >>>> > >>>> You are using SEEK_CUR with -avio_tell(pb), which effectively makes it > >>>> an absolute seek. > >>>> > >>> > >>> Nope, I skip data only, not seeking backward ever. > >>> > >> > >> avio_seek() internally translates any avio_seek() with SEEK_CUR to an > >> absolute seek (as required by the seek callbacks) and does not care > >> about whether it is seeking forward or backward. > >> > > > > Irrelevant. Seeking needs enough cache to work on non-seekable input. > > > > Given that avio_skip() is just a wrapper around avio_seek(), the > behaviour will not change even when the input is non-seekable. > What you are trying to sell? That seeking works with unseekable input? > > - Andreas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-10 12:29 ` Paul B Mahol @ 2023-09-10 12:55 ` Andreas Rheinhardt 2023-09-10 13:16 ` Paul B Mahol 0 siblings, 1 reply; 17+ messages in thread From: Andreas Rheinhardt @ 2023-09-10 12:55 UTC (permalink / raw) To: ffmpeg-devel Paul B Mahol: > On Sun, Sep 10, 2023 at 2:18 PM Andreas Rheinhardt < > andreas.rheinhardt@outlook.com> wrote: > >> Paul B Mahol: >>> On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt < >>> andreas.rheinhardt@outlook.com> wrote: >>> >>>> Paul B Mahol: >>>>> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt < >>>>> andreas.rheinhardt@outlook.com> wrote: >>>>> >>>>>> Paul B Mahol: >>>>>>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt < >>>>>>> andreas.rheinhardt@outlook.com> wrote: >>>>>>> >>>>>>>> Paul B Mahol: >>>>>>>>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < >>>>>>>>> andreas.rheinhardt@outlook.com> wrote: >>>>>>>>> >>>>>>>>>> Paul B Mahol: >>>>>>>>>>> >>>>>>>>>>> + chunk_type = avio_rb32(pb); >>>>>>>>>>> + chunk_size = avio_rb32(pb); >>>>>>>>>> >>>>>>>>>> You are not checking whether the chunk here exceeds its containing >>>>>>>> chunk. >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> + av_fast_malloc(&usm->header, &usm->header_size, >>>>>>>>>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); >>>>>>>>>>> + if (!usm->header) >>>>>>>>>>> + return AVERROR(ENOMEM); >>>>>>>>>> >>>>>>>>>> The bytestream2 API does not rely on the buffer being padded at >> all. >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> + bytestream2_skip(&sgb, string_offset); >>>>>>>>>> >>>>>>>>>> This is unnecessary, because you seek with an absolute offset >>>> lateron >>>>>>>>>> anyway before using sgb. >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> + bytestream2_seek(&sgb, string_offset + offset, >> SEEK_SET); >>>>>>>>>>> + while (bytestream2_get_bytes_left(&sgb) > 0) { >>>>>>>>>>> + key[n] = bytestream2_get_byte(&sgb); >>>>>>>>>>> + if (!key[n]) >>>>>>>>>>> + break; >>>>>>>>>>> + if (n >= sizeof(key) - 1) >>>>>>>>>>> + break; >>>>>>>>>>> + n++; >>>>>>>>>>> + } >>>>>>>>>>> + key[n] = '\0'; >>>>>>>>>> >>>>>>>>>> IMO this would be easier with strnlen(), avoiding sgb altogether. >>>>>>>>>> You would of course need to explicitly check that you are not >>>>>>>>>> overreading, but that is good practice anyway. >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> + chunk_start = avio_tell(pb); >>>>>>>>>>> + avio_skip(pb, 1); >>>>>>>>>>> + payload_offset = avio_r8(pb); >>>>>>>>>>> + padding_size = avio_rb16(pb); >>>>>>>>>>> + stream_index = avio_r8(pb); >>>>>>>>>>> + avio_skip(pb, 2); >>>>>>>>>>> + payload_type = avio_r8(pb); >>>>>>>>>>> + frame_time = avio_rb32(pb); >>>>>>>>>>> + frame_rate = avio_rb32(pb); >>>>>>>>>>> + avio_skip(pb, 8); >>>>>>>>>> >>>>>>>>>> payload_offset and frame_time are set-but-unused; this might lead >> to >>>>>>>>>> compiler warnings. >>>>>>>>>> >>>>>>>>>>> + if (usm->ch[is_audio][stream_index].used == 1) { >>>>>>>>>>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) - >>>>>>>>>> chunk_start); >>>>>>>>>>> + >>>>>>>>>> >>>>>>>>>> This is unnecessary: Unless we already had a read error, pkt_size >> is >>>>>>>>>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). >>>>>>>>>> >>>>>>>>>> (Notice that in case padding_size is > 0, it will be part of the >>>>>> packet >>>>>>>>>> with the current code; not sure if that is an issue.) >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> + >>>>>>>>>>> + avio_skip(pb, padding_size); >>>>>>>>>>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); >>>>>>>>>>> + >>>>>>>>>> >>>>>>>>>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, >>>> SEEK_SET); >>>>>>>>>> >>>>>>>>> >>>>>>>>> But input might not be seekable. >>>>>>>>> >>>>>>>> >>>>>>>> And? You know that avio_skip(pb, offset) is just avio_seek(pb, >> offset, >>>>>>>> SEEK_CUR)? >>>>>>>> >>>>>>> >>>>>>> And? Do you know that SEEK_SET is different from SEEK_CUR with >> positive >>>>>>> argument. >>>>>>> >>>>>> >>>>>> You are using SEEK_CUR with -avio_tell(pb), which effectively makes it >>>>>> an absolute seek. >>>>>> >>>>> >>>>> Nope, I skip data only, not seeking backward ever. >>>>> >>>> >>>> avio_seek() internally translates any avio_seek() with SEEK_CUR to an >>>> absolute seek (as required by the seek callbacks) and does not care >>>> about whether it is seeking forward or backward. >>>> >>> >>> Irrelevant. Seeking needs enough cache to work on non-seekable input. >>> >> >> Given that avio_skip() is just a wrapper around avio_seek(), the >> behaviour will not change even when the input is non-seekable. >> > > What you are trying to sell? > > That seeking works with unseekable input? > Seeking forward works for unseekable input by reading data and discarding it (that is how avio_seek() works). But actually we don't need that, all we need is that avio_skip() is just a wrapper around avio_seek(), so that the two forms are equivalent. - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-10 12:55 ` Andreas Rheinhardt @ 2023-09-10 13:16 ` Paul B Mahol 2023-09-10 13:14 ` Andreas Rheinhardt 0 siblings, 1 reply; 17+ messages in thread From: Paul B Mahol @ 2023-09-10 13:16 UTC (permalink / raw) To: FFmpeg development discussions and patches On Sun, Sep 10, 2023 at 2:54 PM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > Paul B Mahol: > > On Sun, Sep 10, 2023 at 2:18 PM Andreas Rheinhardt < > > andreas.rheinhardt@outlook.com> wrote: > > > >> Paul B Mahol: > >>> On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt < > >>> andreas.rheinhardt@outlook.com> wrote: > >>> > >>>> Paul B Mahol: > >>>>> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt < > >>>>> andreas.rheinhardt@outlook.com> wrote: > >>>>> > >>>>>> Paul B Mahol: > >>>>>>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt < > >>>>>>> andreas.rheinhardt@outlook.com> wrote: > >>>>>>> > >>>>>>>> Paul B Mahol: > >>>>>>>>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < > >>>>>>>>> andreas.rheinhardt@outlook.com> wrote: > >>>>>>>>> > >>>>>>>>>> Paul B Mahol: > >>>>>>>>>>> > >>>>>>>>>>> + chunk_type = avio_rb32(pb); > >>>>>>>>>>> + chunk_size = avio_rb32(pb); > >>>>>>>>>> > >>>>>>>>>> You are not checking whether the chunk here exceeds its > containing > >>>>>>>> chunk. > >>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> + av_fast_malloc(&usm->header, &usm->header_size, > >>>>>>>>>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); > >>>>>>>>>>> + if (!usm->header) > >>>>>>>>>>> + return AVERROR(ENOMEM); > >>>>>>>>>> > >>>>>>>>>> The bytestream2 API does not rely on the buffer being padded at > >> all. > >>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> + bytestream2_skip(&sgb, string_offset); > >>>>>>>>>> > >>>>>>>>>> This is unnecessary, because you seek with an absolute offset > >>>> lateron > >>>>>>>>>> anyway before using sgb. > >>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> + bytestream2_seek(&sgb, string_offset + offset, > >> SEEK_SET); > >>>>>>>>>>> + while (bytestream2_get_bytes_left(&sgb) > 0) { > >>>>>>>>>>> + key[n] = bytestream2_get_byte(&sgb); > >>>>>>>>>>> + if (!key[n]) > >>>>>>>>>>> + break; > >>>>>>>>>>> + if (n >= sizeof(key) - 1) > >>>>>>>>>>> + break; > >>>>>>>>>>> + n++; > >>>>>>>>>>> + } > >>>>>>>>>>> + key[n] = '\0'; > >>>>>>>>>> > >>>>>>>>>> IMO this would be easier with strnlen(), avoiding sgb > altogether. > >>>>>>>>>> You would of course need to explicitly check that you are not > >>>>>>>>>> overreading, but that is good practice anyway. > >>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> + chunk_start = avio_tell(pb); > >>>>>>>>>>> + avio_skip(pb, 1); > >>>>>>>>>>> + payload_offset = avio_r8(pb); > >>>>>>>>>>> + padding_size = avio_rb16(pb); > >>>>>>>>>>> + stream_index = avio_r8(pb); > >>>>>>>>>>> + avio_skip(pb, 2); > >>>>>>>>>>> + payload_type = avio_r8(pb); > >>>>>>>>>>> + frame_time = avio_rb32(pb); > >>>>>>>>>>> + frame_rate = avio_rb32(pb); > >>>>>>>>>>> + avio_skip(pb, 8); > >>>>>>>>>> > >>>>>>>>>> payload_offset and frame_time are set-but-unused; this might > lead > >> to > >>>>>>>>>> compiler warnings. > >>>>>>>>>> > >>>>>>>>>>> + if (usm->ch[is_audio][stream_index].used == 1) { > >>>>>>>>>>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) - > >>>>>>>>>> chunk_start); > >>>>>>>>>>> + > >>>>>>>>>> > >>>>>>>>>> This is unnecessary: Unless we already had a read error, > pkt_size > >> is > >>>>>>>>>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). > >>>>>>>>>> > >>>>>>>>>> (Notice that in case padding_size is > 0, it will be part of the > >>>>>> packet > >>>>>>>>>> with the current code; not sure if that is an issue.) > >>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> + > >>>>>>>>>>> + avio_skip(pb, padding_size); > >>>>>>>>>>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); > >>>>>>>>>>> + > >>>>>>>>>> > >>>>>>>>>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, > >>>> SEEK_SET); > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> But input might not be seekable. > >>>>>>>>> > >>>>>>>> > >>>>>>>> And? You know that avio_skip(pb, offset) is just avio_seek(pb, > >> offset, > >>>>>>>> SEEK_CUR)? > >>>>>>>> > >>>>>>> > >>>>>>> And? Do you know that SEEK_SET is different from SEEK_CUR with > >> positive > >>>>>>> argument. > >>>>>>> > >>>>>> > >>>>>> You are using SEEK_CUR with -avio_tell(pb), which effectively makes > it > >>>>>> an absolute seek. > >>>>>> > >>>>> > >>>>> Nope, I skip data only, not seeking backward ever. > >>>>> > >>>> > >>>> avio_seek() internally translates any avio_seek() with SEEK_CUR to an > >>>> absolute seek (as required by the seek callbacks) and does not care > >>>> about whether it is seeking forward or backward. > >>>> > >>> > >>> Irrelevant. Seeking needs enough cache to work on non-seekable input. > >>> > >> > >> Given that avio_skip() is just a wrapper around avio_seek(), the > >> behaviour will not change even when the input is non-seekable. > >> > > > > What you are trying to sell? > > > > That seeking works with unseekable input? > > > > Seeking forward works for unseekable input by reading data and > discarding it (that is how avio_seek() works). But actually we don't > need that, all we need is that avio_skip() is just a wrapper around > avio_seek(), so that the two forms are equivalent. > Patches welcome, just do not break unseekable demuxing. > > - Andreas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-10 13:16 ` Paul B Mahol @ 2023-09-10 13:14 ` Andreas Rheinhardt 2023-09-10 13:24 ` Paul B Mahol 0 siblings, 1 reply; 17+ messages in thread From: Andreas Rheinhardt @ 2023-09-10 13:14 UTC (permalink / raw) To: ffmpeg-devel Paul B Mahol: > On Sun, Sep 10, 2023 at 2:54 PM Andreas Rheinhardt < > andreas.rheinhardt@outlook.com> wrote: > >> Paul B Mahol: >>> On Sun, Sep 10, 2023 at 2:18 PM Andreas Rheinhardt < >>> andreas.rheinhardt@outlook.com> wrote: >>> >>>> Paul B Mahol: >>>>> On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt < >>>>> andreas.rheinhardt@outlook.com> wrote: >>>>> >>>>>> Paul B Mahol: >>>>>>> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt < >>>>>>> andreas.rheinhardt@outlook.com> wrote: >>>>>>> >>>>>>>> Paul B Mahol: >>>>>>>>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt < >>>>>>>>> andreas.rheinhardt@outlook.com> wrote: >>>>>>>>> >>>>>>>>>> Paul B Mahol: >>>>>>>>>>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < >>>>>>>>>>> andreas.rheinhardt@outlook.com> wrote: >>>>>>>>>>> >>>>>>>>>>>> Paul B Mahol: >>>>>>>>>>>>> >>>>>>>>>>>>> + chunk_type = avio_rb32(pb); >>>>>>>>>>>>> + chunk_size = avio_rb32(pb); >>>>>>>>>>>> >>>>>>>>>>>> You are not checking whether the chunk here exceeds its >> containing >>>>>>>>>> chunk. >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> + av_fast_malloc(&usm->header, &usm->header_size, >>>>>>>>>>>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE); >>>>>>>>>>>>> + if (!usm->header) >>>>>>>>>>>>> + return AVERROR(ENOMEM); >>>>>>>>>>>> >>>>>>>>>>>> The bytestream2 API does not rely on the buffer being padded at >>>> all. >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> + bytestream2_skip(&sgb, string_offset); >>>>>>>>>>>> >>>>>>>>>>>> This is unnecessary, because you seek with an absolute offset >>>>>> lateron >>>>>>>>>>>> anyway before using sgb. >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> + bytestream2_seek(&sgb, string_offset + offset, >>>> SEEK_SET); >>>>>>>>>>>>> + while (bytestream2_get_bytes_left(&sgb) > 0) { >>>>>>>>>>>>> + key[n] = bytestream2_get_byte(&sgb); >>>>>>>>>>>>> + if (!key[n]) >>>>>>>>>>>>> + break; >>>>>>>>>>>>> + if (n >= sizeof(key) - 1) >>>>>>>>>>>>> + break; >>>>>>>>>>>>> + n++; >>>>>>>>>>>>> + } >>>>>>>>>>>>> + key[n] = '\0'; >>>>>>>>>>>> >>>>>>>>>>>> IMO this would be easier with strnlen(), avoiding sgb >> altogether. >>>>>>>>>>>> You would of course need to explicitly check that you are not >>>>>>>>>>>> overreading, but that is good practice anyway. >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> + chunk_start = avio_tell(pb); >>>>>>>>>>>>> + avio_skip(pb, 1); >>>>>>>>>>>>> + payload_offset = avio_r8(pb); >>>>>>>>>>>>> + padding_size = avio_rb16(pb); >>>>>>>>>>>>> + stream_index = avio_r8(pb); >>>>>>>>>>>>> + avio_skip(pb, 2); >>>>>>>>>>>>> + payload_type = avio_r8(pb); >>>>>>>>>>>>> + frame_time = avio_rb32(pb); >>>>>>>>>>>>> + frame_rate = avio_rb32(pb); >>>>>>>>>>>>> + avio_skip(pb, 8); >>>>>>>>>>>> >>>>>>>>>>>> payload_offset and frame_time are set-but-unused; this might >> lead >>>> to >>>>>>>>>>>> compiler warnings. >>>>>>>>>>>> >>>>>>>>>>>>> + if (usm->ch[is_audio][stream_index].used == 1) { >>>>>>>>>>>>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) - >>>>>>>>>>>> chunk_start); >>>>>>>>>>>>> + >>>>>>>>>>>> >>>>>>>>>>>> This is unnecessary: Unless we already had a read error, >> pkt_size >>>> is >>>>>>>>>>>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). >>>>>>>>>>>> >>>>>>>>>>>> (Notice that in case padding_size is > 0, it will be part of the >>>>>>>> packet >>>>>>>>>>>> with the current code; not sure if that is an issue.) >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> + >>>>>>>>>>>>> + avio_skip(pb, padding_size); >>>>>>>>>>>>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start)); >>>>>>>>>>>>> + >>>>>>>>>>>> >>>>>>>>>>>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, >>>>>> SEEK_SET); >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> But input might not be seekable. >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> And? You know that avio_skip(pb, offset) is just avio_seek(pb, >>>> offset, >>>>>>>>>> SEEK_CUR)? >>>>>>>>>> >>>>>>>>> >>>>>>>>> And? Do you know that SEEK_SET is different from SEEK_CUR with >>>> positive >>>>>>>>> argument. >>>>>>>>> >>>>>>>> >>>>>>>> You are using SEEK_CUR with -avio_tell(pb), which effectively makes >> it >>>>>>>> an absolute seek. >>>>>>>> >>>>>>> >>>>>>> Nope, I skip data only, not seeking backward ever. >>>>>>> >>>>>> >>>>>> avio_seek() internally translates any avio_seek() with SEEK_CUR to an >>>>>> absolute seek (as required by the seek callbacks) and does not care >>>>>> about whether it is seeking forward or backward. >>>>>> >>>>> >>>>> Irrelevant. Seeking needs enough cache to work on non-seekable input. >>>>> >>>> >>>> Given that avio_skip() is just a wrapper around avio_seek(), the >>>> behaviour will not change even when the input is non-seekable. >>>> >>> >>> What you are trying to sell? >>> >>> That seeking works with unseekable input? >>> >> >> Seeking forward works for unseekable input by reading data and >> discarding it (that is how avio_seek() works). But actually we don't >> need that, all we need is that avio_skip() is just a wrapper around >> avio_seek(), so that the two forms are equivalent. >> > > Patches welcome, just do not break unseekable demuxing. > ? You want me to send a separate patch avoiding the unnecessary avio_seek() and switching to avio_seek() from avio_skip()? Why not apply it now? - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-10 13:14 ` Andreas Rheinhardt @ 2023-09-10 13:24 ` Paul B Mahol 0 siblings, 0 replies; 17+ messages in thread From: Paul B Mahol @ 2023-09-10 13:24 UTC (permalink / raw) To: FFmpeg development discussions and patches On Sun, Sep 10, 2023 at 3:13 PM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > Paul B Mahol: > > On Sun, Sep 10, 2023 at 2:54 PM Andreas Rheinhardt < > > andreas.rheinhardt@outlook.com> wrote: > > > >> Paul B Mahol: > >>> On Sun, Sep 10, 2023 at 2:18 PM Andreas Rheinhardt < > >>> andreas.rheinhardt@outlook.com> wrote: > >>> > >>>> Paul B Mahol: > >>>>> On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt < > >>>>> andreas.rheinhardt@outlook.com> wrote: > >>>>> > >>>>>> Paul B Mahol: > >>>>>>> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt < > >>>>>>> andreas.rheinhardt@outlook.com> wrote: > >>>>>>> > >>>>>>>> Paul B Mahol: > >>>>>>>>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt < > >>>>>>>>> andreas.rheinhardt@outlook.com> wrote: > >>>>>>>>> > >>>>>>>>>> Paul B Mahol: > >>>>>>>>>>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt < > >>>>>>>>>>> andreas.rheinhardt@outlook.com> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> Paul B Mahol: > >>>>>>>>>>>>> > >>>>>>>>>>>>> + chunk_type = avio_rb32(pb); > >>>>>>>>>>>>> + chunk_size = avio_rb32(pb); > >>>>>>>>>>>> > >>>>>>>>>>>> You are not checking whether the chunk here exceeds its > >> containing > >>>>>>>>>> chunk. > >>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> + av_fast_malloc(&usm->header, &usm->header_size, > >>>>>>>>>>>>> + chunk_size + > AV_INPUT_BUFFER_PADDING_SIZE); > >>>>>>>>>>>>> + if (!usm->header) > >>>>>>>>>>>>> + return AVERROR(ENOMEM); > >>>>>>>>>>>> > >>>>>>>>>>>> The bytestream2 API does not rely on the buffer being padded > at > >>>> all. > >>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> + bytestream2_skip(&sgb, string_offset); > >>>>>>>>>>>> > >>>>>>>>>>>> This is unnecessary, because you seek with an absolute offset > >>>>>> lateron > >>>>>>>>>>>> anyway before using sgb. > >>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> + bytestream2_seek(&sgb, string_offset + offset, > >>>> SEEK_SET); > >>>>>>>>>>>>> + while (bytestream2_get_bytes_left(&sgb) > 0) { > >>>>>>>>>>>>> + key[n] = bytestream2_get_byte(&sgb); > >>>>>>>>>>>>> + if (!key[n]) > >>>>>>>>>>>>> + break; > >>>>>>>>>>>>> + if (n >= sizeof(key) - 1) > >>>>>>>>>>>>> + break; > >>>>>>>>>>>>> + n++; > >>>>>>>>>>>>> + } > >>>>>>>>>>>>> + key[n] = '\0'; > >>>>>>>>>>>> > >>>>>>>>>>>> IMO this would be easier with strnlen(), avoiding sgb > >> altogether. > >>>>>>>>>>>> You would of course need to explicitly check that you are not > >>>>>>>>>>>> overreading, but that is good practice anyway. > >>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> + chunk_start = avio_tell(pb); > >>>>>>>>>>>>> + avio_skip(pb, 1); > >>>>>>>>>>>>> + payload_offset = avio_r8(pb); > >>>>>>>>>>>>> + padding_size = avio_rb16(pb); > >>>>>>>>>>>>> + stream_index = avio_r8(pb); > >>>>>>>>>>>>> + avio_skip(pb, 2); > >>>>>>>>>>>>> + payload_type = avio_r8(pb); > >>>>>>>>>>>>> + frame_time = avio_rb32(pb); > >>>>>>>>>>>>> + frame_rate = avio_rb32(pb); > >>>>>>>>>>>>> + avio_skip(pb, 8); > >>>>>>>>>>>> > >>>>>>>>>>>> payload_offset and frame_time are set-but-unused; this might > >> lead > >>>> to > >>>>>>>>>>>> compiler warnings. > >>>>>>>>>>>> > >>>>>>>>>>>>> + if (usm->ch[is_audio][stream_index].used == 1) { > >>>>>>>>>>>>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) > - > >>>>>>>>>>>> chunk_start); > >>>>>>>>>>>>> + > >>>>>>>>>>>> > >>>>>>>>>>>> This is unnecessary: Unless we already had a read error, > >> pkt_size > >>>> is > >>>>>>>>>>>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8). > >>>>>>>>>>>> > >>>>>>>>>>>> (Notice that in case padding_size is > 0, it will be part of > the > >>>>>>>> packet > >>>>>>>>>>>> with the current code; not sure if that is an issue.) > >>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> + > >>>>>>>>>>>>> + avio_skip(pb, padding_size); > >>>>>>>>>>>>> + avio_skip(pb, chunk_size - (avio_tell(pb) - > chunk_start)); > >>>>>>>>>>>>> + > >>>>>>>>>>>> > >>>>>>>>>>>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, > >>>>>> SEEK_SET); > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> But input might not be seekable. > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> And? You know that avio_skip(pb, offset) is just avio_seek(pb, > >>>> offset, > >>>>>>>>>> SEEK_CUR)? > >>>>>>>>>> > >>>>>>>>> > >>>>>>>>> And? Do you know that SEEK_SET is different from SEEK_CUR with > >>>> positive > >>>>>>>>> argument. > >>>>>>>>> > >>>>>>>> > >>>>>>>> You are using SEEK_CUR with -avio_tell(pb), which effectively > makes > >> it > >>>>>>>> an absolute seek. > >>>>>>>> > >>>>>>> > >>>>>>> Nope, I skip data only, not seeking backward ever. > >>>>>>> > >>>>>> > >>>>>> avio_seek() internally translates any avio_seek() with SEEK_CUR to > an > >>>>>> absolute seek (as required by the seek callbacks) and does not care > >>>>>> about whether it is seeking forward or backward. > >>>>>> > >>>>> > >>>>> Irrelevant. Seeking needs enough cache to work on non-seekable input. > >>>>> > >>>> > >>>> Given that avio_skip() is just a wrapper around avio_seek(), the > >>>> behaviour will not change even when the input is non-seekable. > >>>> > >>> > >>> What you are trying to sell? > >>> > >>> That seeking works with unseekable input? > >>> > >> > >> Seeking forward works for unseekable input by reading data and > >> discarding it (that is how avio_seek() works). But actually we don't > >> need that, all we need is that avio_skip() is just a wrapper around > >> avio_seek(), so that the two forms are equivalent. > >> > > > > Patches welcome, just do not break unseekable demuxing. > > > > ? You want me to send a separate patch avoiding the unnecessary > avio_seek() and switching to avio_seek() from avio_skip()? Why not apply > it now? > I prefer avio_skip() solution. And would prefer to stay that way, It is irrelevant how underlying code call is implemented. Otherwise I'm not code nazi. > > - Andreas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-05 21:37 [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer Paul B Mahol 2023-09-06 9:27 ` Andreas Rheinhardt @ 2023-09-10 12:02 ` Paul B Mahol 2023-09-16 12:19 ` Paul B Mahol 1 sibling, 1 reply; 17+ messages in thread From: Paul B Mahol @ 2023-09-10 12:02 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1: Type: text/plain, Size: 320 bytes --] New version attached: - fixed VP9 demuxing - added support for alpha streams - added support for subtitle streams - numerous fixes and improvements Can't get seeking to behave correctly with ADPCM_ADX audio streams. Once one seek to start of file audio is no longer demuxed and video packets are filling all queue. [-- Attachment #2: 0001-avformat-add-CRI-USM-demuxer.patch --] [-- Type: text/x-patch, Size: 13469 bytes --] From 5c32d4a9edb4f87ac2909909c732841bb6871e48 Mon Sep 17 00:00:00 2001 From: Paul B Mahol <onemda@gmail.com> Date: Tue, 5 Sep 2023 16:53:32 +0200 Subject: [PATCH] avformat: add CRI USM demuxer Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavformat/Makefile | 1 + libavformat/allformats.c | 1 + libavformat/usmdec.c | 379 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 381 insertions(+) create mode 100644 libavformat/usmdec.c diff --git a/libavformat/Makefile b/libavformat/Makefile index cc1b12360a..329055ccfd 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -588,6 +588,7 @@ OBJS-$(CONFIG_TTY_DEMUXER) += tty.o sauce.o OBJS-$(CONFIG_TY_DEMUXER) += ty.o OBJS-$(CONFIG_TXD_DEMUXER) += txd.o OBJS-$(CONFIG_UNCODEDFRAMECRC_MUXER) += uncodedframecrcenc.o framehash.o +OBJS-$(CONFIG_USM_DEMUXER) += usmdec.o OBJS-$(CONFIG_V210_DEMUXER) += rawvideodec.o OBJS-$(CONFIG_V210X_DEMUXER) += rawvideodec.o OBJS-$(CONFIG_VAG_DEMUXER) += vag.o diff --git a/libavformat/allformats.c b/libavformat/allformats.c index f4210e4932..d4b505a5a3 100644 --- a/libavformat/allformats.c +++ b/libavformat/allformats.c @@ -471,6 +471,7 @@ extern const AVInputFormat ff_txd_demuxer; extern const AVInputFormat ff_tty_demuxer; extern const AVInputFormat ff_ty_demuxer; extern const FFOutputFormat ff_uncodedframecrc_muxer; +extern const AVInputFormat ff_usm_demuxer; extern const AVInputFormat ff_v210_demuxer; extern const AVInputFormat ff_v210x_demuxer; extern const AVInputFormat ff_vag_demuxer; diff --git a/libavformat/usmdec.c b/libavformat/usmdec.c new file mode 100644 index 0000000000..f8bfe1a123 --- /dev/null +++ b/libavformat/usmdec.c @@ -0,0 +1,379 @@ +/* + * USM demuxer + * Copyright (c) 2023 Paul B Mahol + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "libavutil/intreadwrite.h" +#include "libavcodec/bytestream.h" + +#include "avformat.h" +#include "internal.h" + +#define VIDEOI 0 +#define AUDIOI 1 +#define ALPHAI 2 +#define SUBTTI 3 + +typedef struct USMChannel { + int index; + int used; +} USMChannel; + +typedef struct USMDemuxContext { + USMChannel ch[4][256]; + int nb_channels[4]; + uint8_t *header; + unsigned header_size; +} USMDemuxContext; + +static int usm_probe(const AVProbeData *p) +{ + if (AV_RL32(p->buf) != MKTAG('C','R','I','D')) + return 0; + + if (AV_RN32(p->buf + 4) == 0) + return 0; + + return AVPROBE_SCORE_MAX / 3; +} + +static int usm_read_header(AVFormatContext *s) +{ + s->ctx_flags |= AVFMTCTX_NOHEADER; + return 0; +} + +static int parse_utf(AVFormatContext *s, AVIOContext *pb, + AVStream *st, AVCodecParameters *par, int ch_type, + uint32_t parent_chunk_size) +{ + USMDemuxContext *usm = s->priv_data; + GetByteContext gb, ugb, sgb; + uint32_t chunk_type, chunk_size, offset; + uint32_t unique_offset, string_offset; + int nb_items, unique_size, nb_dictionaries; + AVRational fps = { 0 }; + int type; + + chunk_type = avio_rb32(pb); + chunk_size = avio_rb32(pb); + + if (chunk_type != MKBETAG('@','U','T','F')) + return AVERROR_INVALIDDATA; + + if (!chunk_size || chunk_size >= parent_chunk_size) + return AVERROR_INVALIDDATA; + + av_fast_malloc(&usm->header, &usm->header_size, chunk_size); + if (!usm->header) + return AVERROR(ENOMEM); + + if (avio_read(pb, usm->header, chunk_size) != chunk_size) + return AVERROR_EOF; + + bytestream2_init(&gb, usm->header, chunk_size); + ugb = gb; + sgb = gb; + unique_offset = bytestream2_get_be32(&gb); + string_offset = bytestream2_get_be32(&gb); + /*byte_offset =*/ bytestream2_get_be32(&gb); + /*payload_name_offset =*/ bytestream2_get_be32(&gb); + nb_items = bytestream2_get_be16(&gb); + unique_size = bytestream2_get_be16(&gb); + nb_dictionaries = bytestream2_get_be32(&gb); + if (nb_dictionaries == 0) + return AVERROR_INVALIDDATA; + + bytestream2_skip(&ugb, unique_offset); + if (bytestream2_get_bytes_left(&ugb) < unique_size) + return AVERROR_INVALIDDATA; + bytestream2_init(&ugb, ugb.buffer, unique_size); + + bytestream2_skip(&sgb, string_offset); + + for (int i = 0; i < nb_items; i++) { + GetByteContext *xgb; + uint8_t key[256]; + int64_t value; + int n = 0; + + type = bytestream2_get_byte(&gb); + offset = bytestream2_get_be32(&gb); + + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET); + while (bytestream2_get_bytes_left(&sgb) > 0) { + key[n] = bytestream2_get_byte(&sgb); + if (!key[n]) + break; + if (n >= sizeof(key) - 1) + break; + n++; + } + key[n] = '\0'; + + if ((type >> 5) == 1) + xgb = &gb; + else + xgb = &ugb; + + switch (type & 0x1F) { + case 0x10: + case 0x11: + value = bytestream2_get_byte(xgb); + break; + case 0x12: + case 0x13: + value = bytestream2_get_be16(xgb); + break; + case 0x14: + case 0x15: + value = bytestream2_get_be32(xgb); + break; + case 0x16: + case 0x17: + value = bytestream2_get_be64(xgb); + break; + case 0x18: + value = av_int2float(bytestream2_get_be32(xgb)); + break; + case 0x19: + value = av_int2double(bytestream2_get_be64(xgb)); + break; + case 0x1A: + break; + } + + if (ch_type == AUDIOI) { + if (!strcmp(key, "sampling_rate")) { + par->sample_rate = value; + avpriv_set_pts_info(st, 64, 1, value); + } else if (!strcmp(key, "num_channels")) { + par->ch_layout.nb_channels = value; + } else if (!strcmp(key, "total_samples")) { + st->duration = value; + } else if (!strcmp(key, "audio_codec")) { + switch (value) { + case 2: + par->codec_id = AV_CODEC_ID_ADPCM_ADX; + break; + case 4: + par->codec_id = AV_CODEC_ID_HCA; + break; + default: + av_log(s, AV_LOG_ERROR, "unsupported audio: %d\n", (int)value); + break; + } + } + } else if (ch_type == VIDEOI || ch_type == ALPHAI) { + if (!strcmp(key, "width")) { + par->width = value; + } else if (!strcmp(key, "height")) { + par->height = value; + } else if (!strcmp(key, "total_frames")) { + st->nb_frames = value; + } else if (!strcmp(key, "framerate_n")) { + fps.num = value; + } else if (!strcmp(key, "framerate_d")) { + fps.den = value; + } else if (!strcmp(key, "mpeg_codec")) { + switch (value) { + case 1: + par->codec_id = AV_CODEC_ID_MPEG1VIDEO; + break; + case 5: + par->codec_id = AV_CODEC_ID_H264; + break; + case 9: + par->codec_id = AV_CODEC_ID_VP9; + break; + default: + av_log(s, AV_LOG_ERROR, "unsupported video: %d\n", (int)value); + break; + } + } + } + } + + if (ch_type == VIDEOI && fps.num && fps.den) + avpriv_set_pts_info(st, 64, fps.den, fps.num); + + return 0; +} + +static int64_t parse_chunk(AVFormatContext *s, AVIOContext *pb, + uint32_t chunk_type, uint32_t chunk_size, + AVPacket *pkt) +{ + const int is_audio = chunk_type == MKBETAG('@','S','F','A'); + const int is_alpha = chunk_type == MKBETAG('@','A','L','P'); + const int is_subtt = chunk_type == MKBETAG('@','S','B','T'); + USMDemuxContext *usm = s->priv_data; + int64_t chunk_start, ret; + int stream_index, payload_type; + int payload_offset; + int frame_rate, padding_size; + const int ch_type = is_subtt ? SUBTTI : is_audio ? AUDIOI : is_alpha ? ALPHAI : VIDEOI; + + ret = avio_tell(pb); + if (ret < 0) + return ret; + chunk_start = ret; + avio_skip(pb, 1); + payload_offset = avio_r8(pb); + padding_size = avio_rb16(pb); + stream_index = avio_r8(pb); + avio_skip(pb, 2); + payload_type = avio_r8(pb); + /*frame_time =*/ avio_rb32(pb); + frame_rate = avio_rb32(pb); + avio_skip(pb, 8); + ret = avio_tell(pb); + if (ret < 0) + return ret; + ret = avio_skip(pb, FFMAX(0, (ret - chunk_start) - payload_offset)); + if (ret < 0) + return ret; + + if (payload_type == 1) { + if (usm->ch[ch_type][stream_index].used == 0) { + AVStream *st = avformat_new_stream(s, NULL); + AVCodecParameters *par; + if (!st) + return AVERROR(ENOMEM); + + par = st->codecpar; + switch (ch_type) { + case ALPHAI: + case VIDEOI: + par->codec_type = AVMEDIA_TYPE_VIDEO; + break; + case AUDIOI: + par->codec_type = AVMEDIA_TYPE_AUDIO; + break; + case SUBTTI: + par->codec_type = AVMEDIA_TYPE_SUBTITLE; + break; + } + st->start_time = 0; + + usm->ch[ch_type][stream_index].index = st->index; + usm->ch[ch_type][stream_index].used = 1; + usm->nb_channels[ch_type]++; + + ret = parse_utf(s, pb, st, par, ch_type, chunk_size); + if (ret < 0) + return ret; + + if (!st->time_base.num || !st->time_base.den) + avpriv_set_pts_info(st, 64, 100, frame_rate); + ffstream(st)->need_parsing = AVSTREAM_PARSE_TIMESTAMPS; + } + } else if (payload_type == 0) { + if (usm->ch[ch_type][stream_index].used == 1) { + uint32_t pkt_size; + + ret = avio_tell(pb); + if (ret < 0) + return ret; + + pkt_size = chunk_size - (ret - chunk_start) - padding_size; + ret = av_get_packet(pb, pkt, pkt_size); + if (ret < 0) + return ret; + + pkt->stream_index = usm->ch[ch_type][stream_index].index; + + avio_skip(pb, padding_size); + + if (ret != pkt_size) + return AVERROR_EOF; + return ret; + } + } + + ret = avio_tell(pb); + if (ret < 0) + return ret; + ret = avio_skip(pb, FFMAX(0, chunk_size - (ret - chunk_start))); + if (ret < 0) + return ret; + return 0; +} + +static int usm_read_packet(AVFormatContext *s, AVPacket *pkt) +{ + AVIOContext *pb = s->pb; + int64_t ret = AVERROR_EOF; + + while (!avio_feof(pb)) { + uint32_t chunk_type, chunk_size; + int got_packet = 0; + int64_t pos; + + pos = avio_tell(pb); + if (pos < 0) + return pos; + chunk_type = avio_rb32(pb); + chunk_size = avio_rb32(pb); + if (!chunk_size) + return AVERROR_INVALIDDATA; + + switch (chunk_type) { + case MKBETAG('C','R','I','D'): + default: + ret = avio_skip(pb, chunk_size); + break; + case MKBETAG('@','A','L','P'): + case MKBETAG('@','S','B','T'): + case MKBETAG('@','S','F','A'): + case MKBETAG('@','S','F','V'): + ret = parse_chunk(s, pb, chunk_type, chunk_size, pkt); + got_packet = ret > 0; + break; + } + + if (got_packet) + pkt->pos = pos; + + if (got_packet || ret < 0) + break; + } + + return ret; +} + +static int usm_read_close(AVFormatContext *s) +{ + USMDemuxContext *usm = s->priv_data; + av_freep(&usm->header); + usm->header_size = 0; + return 0; +} + +const AVInputFormat ff_usm_demuxer = { + .name = "usm", + .long_name = NULL_IF_CONFIG_SMALL("CRI USM"), + .priv_data_size = sizeof(USMDemuxContext), + .read_probe = usm_probe, + .read_header = usm_read_header, + .read_packet = usm_read_packet, + .read_close = usm_read_close, + .extensions = "usm", + .flags = AVFMT_GENERIC_INDEX | AVFMT_NO_BYTE_SEEK | AVFMT_NOBINSEARCH, +}; -- 2.39.1 [-- Attachment #3: Type: text/plain, Size: 251 bytes --] _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer 2023-09-10 12:02 ` Paul B Mahol @ 2023-09-16 12:19 ` Paul B Mahol 0 siblings, 0 replies; 17+ messages in thread From: Paul B Mahol @ 2023-09-16 12:19 UTC (permalink / raw) To: FFmpeg development discussions and patches Will push soon. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-09-16 12:12 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-09-05 21:37 [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer Paul B Mahol 2023-09-06 9:27 ` Andreas Rheinhardt 2023-09-06 9:57 ` Paul B Mahol 2023-09-06 11:31 ` Andreas Rheinhardt 2023-09-06 12:01 ` Paul B Mahol 2023-09-10 12:00 ` Andreas Rheinhardt 2023-09-10 12:10 ` Paul B Mahol 2023-09-10 12:07 ` Andreas Rheinhardt 2023-09-10 12:15 ` Paul B Mahol 2023-09-10 12:19 ` Andreas Rheinhardt 2023-09-10 12:29 ` Paul B Mahol 2023-09-10 12:55 ` Andreas Rheinhardt 2023-09-10 13:16 ` Paul B Mahol 2023-09-10 13:14 ` Andreas Rheinhardt 2023-09-10 13:24 ` Paul B Mahol 2023-09-10 12:02 ` Paul B Mahol 2023-09-16 12:19 ` Paul B Mahol
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git