* [FFmpeg-devel] [PATCH v2 0/2] Remove chained ogg stream header packets from demuxer
@ 2025-04-28 23:31 Romain Beauxis
2025-04-28 23:31 ` [FFmpeg-devel] [PATCH v2 1/2] tests: Add stream dump test API util, use it to dump stream data for chained ogg/{vorbis, opus, flac} streams Romain Beauxis
2025-04-28 23:31 ` [FFmpeg-devel] [PATCH v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output Romain Beauxis
0 siblings, 2 replies; 9+ messages in thread
From: Romain Beauxis @ 2025-04-28 23:31 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: samples-request, Romain Beauxis
These patches remove the ogg header packets from secondary chainged ogg
streams from the demuxer.
First, a test utility is added to track what is currently happening with
chained streams.
Then the changes are introduced: the packet demuxing function is used to
explicitely tell the demuxer to skip header packets.
Also, the packet demuxing functions are adapted to properly copy extra
data from the new chained streams so that decoding can keep happening.
The diff from the test output makes it possible to follow what the
changes do to the extracted streams.
Test samples are available at: https://www.dropbox.com/scl/fo/xrtrna2rxr1j354hrtymq/AGwemlxHYecBLNmQ8Fsy--4?rlkey=lzilr4m9w4gfdqygoe172vvy8&dl=0
# Changes since last version:
* Fixed api-dump-stream-meta-test.c frame decoding logic
* Fixed oggparsevorbis.c packet skipping logic.
Romain Beauxis (2):
tests: Add stream dump test API util, use it to dump stream data for
chained ogg/{vorbis,opus,flac} streams.
ogg/{vorbis,flac,opus}: Remove header packets from subsequent ogg
streams from the demuxer output.
libavformat/oggdec.c | 26 +--
libavformat/oggdec.h | 6 +
libavformat/oggparseflac.c | 28 +++-
libavformat/oggparseopus.c | 12 ++
libavformat/oggparsevorbis.c | 12 +-
tests/Makefile | 4 +
tests/api/Makefile | 2 +-
tests/api/api-dump-stream-meta-test.c | 182 +++++++++++++++++++++
tests/fate/ogg-flac.mak | 11 ++
tests/fate/ogg-opus.mak | 11 ++
tests/fate/ogg-vorbis.mak | 11 ++
tests/ref/fate/ogg-flac-chained-meta.txt | 10 ++
tests/ref/fate/ogg-opus-chained-meta.txt | 26 +++
tests/ref/fate/ogg-vorbis-chained-meta.txt | 14 ++
14 files changed, 338 insertions(+), 17 deletions(-)
create mode 100644 tests/api/api-dump-stream-meta-test.c
create mode 100644 tests/fate/ogg-flac.mak
create mode 100644 tests/fate/ogg-opus.mak
create mode 100644 tests/fate/ogg-vorbis.mak
create mode 100644 tests/ref/fate/ogg-flac-chained-meta.txt
create mode 100644 tests/ref/fate/ogg-opus-chained-meta.txt
create mode 100644 tests/ref/fate/ogg-vorbis-chained-meta.txt
--
2.39.5 (Apple Git-154)
_______________________________________________
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] 9+ messages in thread* [FFmpeg-devel] [PATCH v2 1/2] tests: Add stream dump test API util, use it to dump stream data for chained ogg/{vorbis, opus, flac} streams. 2025-04-28 23:31 [FFmpeg-devel] [PATCH v2 0/2] Remove chained ogg stream header packets from demuxer Romain Beauxis @ 2025-04-28 23:31 ` Romain Beauxis 2025-04-29 21:25 ` Michael Niedermayer 2025-04-28 23:31 ` [FFmpeg-devel] [PATCH v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output Romain Beauxis 1 sibling, 1 reply; 9+ messages in thread From: Romain Beauxis @ 2025-04-28 23:31 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Romain Beauxis --- tests/Makefile | 4 + tests/api/Makefile | 2 +- tests/api/api-dump-stream-meta-test.c | 182 +++++++++++++++++++++ tests/fate/ogg-flac.mak | 11 ++ tests/fate/ogg-opus.mak | 11 ++ tests/fate/ogg-vorbis.mak | 11 ++ tests/ref/fate/ogg-flac-chained-meta.txt | 12 ++ tests/ref/fate/ogg-opus-chained-meta.txt | 27 +++ tests/ref/fate/ogg-vorbis-chained-meta.txt | 17 ++ 9 files changed, 276 insertions(+), 1 deletion(-) create mode 100644 tests/api/api-dump-stream-meta-test.c create mode 100644 tests/fate/ogg-flac.mak create mode 100644 tests/fate/ogg-opus.mak create mode 100644 tests/fate/ogg-vorbis.mak create mode 100644 tests/ref/fate/ogg-flac-chained-meta.txt create mode 100644 tests/ref/fate/ogg-opus-chained-meta.txt create mode 100644 tests/ref/fate/ogg-vorbis-chained-meta.txt diff --git a/tests/Makefile b/tests/Makefile index 0c08f68713..10871f28f8 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -220,6 +220,9 @@ include $(SRC_PATH)/tests/fate/mpeg4.mak include $(SRC_PATH)/tests/fate/mpegps.mak include $(SRC_PATH)/tests/fate/mpegts.mak include $(SRC_PATH)/tests/fate/mxf.mak +include $(SRC_PATH)/tests/fate/ogg-vorbis.mak +include $(SRC_PATH)/tests/fate/ogg-flac.mak +include $(SRC_PATH)/tests/fate/ogg-opus.mak include $(SRC_PATH)/tests/fate/oma.mak include $(SRC_PATH)/tests/fate/opus.mak include $(SRC_PATH)/tests/fate/pcm.mak @@ -278,6 +281,7 @@ $(FATE_FFPROBE) $(FATE_FFMPEG_FFPROBE) $(FATE_SAMPLES_FFPROBE) $(FATE_SAMPLES_FF $(FATE_SAMPLES_FASTSTART): tools/qt-faststart$(EXESUF) $(FATE_SAMPLES_DUMP_DATA) $(FATE_SAMPLES_DUMP_DATA-yes): tools/venc_data_dump$(EXESUF) $(FATE_SAMPLES_SCALE_SLICE): tools/scale_slice_test$(EXESUF) +$(FATE_SAMPLES_DUMP_STREAM_META): tests/api/api-dump-stream-meta-test$(EXESUF) ifdef SAMPLES FATE += $(FATE_EXTERN) diff --git a/tests/api/Makefile b/tests/api/Makefile index c96e636756..a2cb06a729 100644 --- a/tests/api/Makefile +++ b/tests/api/Makefile @@ -1,7 +1,7 @@ APITESTPROGS-$(call ENCDEC, FLAC, FLAC) += api-flac APITESTPROGS-$(call DEMDEC, H264, H264) += api-h264 APITESTPROGS-$(call DEMDEC, H264, H264) += api-h264-slice -APITESTPROGS-yes += api-seek +APITESTPROGS-yes += api-seek api-dump-stream-meta APITESTPROGS-$(call DEMDEC, H263, H263) += api-band APITESTPROGS-$(HAVE_THREADS) += api-threadmessage APITESTPROGS += $(APITESTPROGS-yes) diff --git a/tests/api/api-dump-stream-meta-test.c b/tests/api/api-dump-stream-meta-test.c new file mode 100644 index 0000000000..629b3a576a --- /dev/null +++ b/tests/api/api-dump-stream-meta-test.c @@ -0,0 +1,182 @@ +/* + * Copyright (c) 2025 Romain Beauxis + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +/** + * Dump stream metadata + */ + +#include "libavcodec/avcodec.h" +#include "libavformat/avformat.h" +#include "libavutil/timestamp.h" + +static int dump_stream_meta(const char *input_filename) { + const AVCodec *codec = NULL; + AVPacket *pkt = NULL; + AVFrame *fr = NULL; + AVFormatContext *fmt_ctx = NULL; + AVCodecContext *ctx = NULL; + AVCodecParameters *origin_par = NULL; + AVStream *st; + int stream_idx = 0; + int result; + char *metadata; + + result = avformat_open_input(&fmt_ctx, input_filename, NULL, NULL); + if (result < 0) { + av_log(NULL, AV_LOG_ERROR, "Can't open file\n"); + return result; + } + + result = avformat_find_stream_info(fmt_ctx, NULL); + if (result < 0) { + av_log(NULL, AV_LOG_ERROR, "Can't get stream info\n"); + goto end; + } + + if (fmt_ctx->nb_streams > 1) { + av_log(NULL, AV_LOG_ERROR, "More than one stream found in input!\n"); + goto end; + } + + origin_par = fmt_ctx->streams[stream_idx]->codecpar; + st = fmt_ctx->streams[stream_idx]; + + result = av_dict_get_string(st->metadata, &metadata, '=', ':'); + if (result < 0) + goto end; + + printf("Stream ID: %d, codec name: %s, metadata: %s\n", stream_idx, + avcodec_get_name(origin_par->codec_id), + strlen(metadata) ? metadata : "N/A"); + + codec = avcodec_find_decoder(origin_par->codec_id); + if (!codec) { + av_log(NULL, AV_LOG_ERROR, "Can't find decoder\n"); + result = AVERROR_DECODER_NOT_FOUND; + goto end; + } + + ctx = avcodec_alloc_context3(codec); + if (!ctx) { + av_log(NULL, AV_LOG_ERROR, "Can't allocate decoder context\n"); + result = AVERROR(ENOMEM); + goto end; + } + + result = avcodec_parameters_to_context(ctx, origin_par); + if (result) { + av_log(NULL, AV_LOG_ERROR, "Can't copy decoder context\n"); + goto end; + } + + result = avcodec_open2(ctx, codec, NULL); + if (result < 0) { + av_log(ctx, AV_LOG_ERROR, "Can't open decoder\n"); + goto end; + } + + pkt = av_packet_alloc(); + if (!pkt) { + av_log(NULL, AV_LOG_ERROR, "Cannot allocate packet\n"); + result = AVERROR(ENOMEM); + goto end; + } + + fr = av_frame_alloc(); + if (!fr) { + av_log(NULL, AV_LOG_ERROR, "Can't allocate frame\n"); + result = AVERROR(ENOMEM); + goto end; + } + + for (;;) { + result = av_read_frame(fmt_ctx, pkt); + if (result) + goto end; + + if (pkt->stream_index != stream_idx) { + av_packet_unref(pkt); + continue; + } + + printf("Stream ID: %d, packet PTS: %s, packet DTS: %s\n", + pkt->stream_index, av_ts2str(pkt->pts), av_ts2str(pkt->dts)); + + if (st->event_flags & AVSTREAM_EVENT_FLAG_METADATA_UPDATED) { + result = av_dict_get_string(st->metadata, &metadata, '=', ':'); + if (result < 0) + goto end; + + printf("Stream ID: %d, new metadata: %s\n", pkt->stream_index, + strlen(metadata) ? metadata : "N/A"); + + st->event_flags &= ~AVSTREAM_EVENT_FLAG_METADATA_UPDATED; + } + + result = avcodec_send_packet(ctx, pkt); + av_packet_unref(pkt); + + if (result < 0) + goto end; + + do { + result = avcodec_receive_frame(ctx, fr); + if (result == AVERROR_EOF) { + result = 0; + goto end; + } + + if (result == AVERROR(EAGAIN)) + break; + + if (result < 0) + goto end; + + result = av_dict_get_string(fr->metadata, &metadata, '=', ':'); + if (result < 0) + goto end; + + printf("Stream ID: %d, frame PTS: %s, metadata: %s\n", + pkt->stream_index, av_ts2str(fr->pts), + strlen(metadata) ? metadata : "N/A"); + } while (1); + } + +end: + av_packet_free(&pkt); + av_frame_free(&fr); + avformat_close_input(&fmt_ctx); + avcodec_free_context(&ctx); + return result; +} + +int main(int argc, char **argv) { + if (argc < 2) { + av_log(NULL, AV_LOG_ERROR, "Incorrect input\n"); + return 1; + } + + if (dump_stream_meta(argv[1]) != AVERROR_EOF) + return 1; + + return 0; +} diff --git a/tests/fate/ogg-flac.mak b/tests/fate/ogg-flac.mak new file mode 100644 index 0000000000..0d6a015161 --- /dev/null +++ b/tests/fate/ogg-flac.mak @@ -0,0 +1,11 @@ +FATE_OGG_FLAC += fate-ogg-flac-chained-meta +fate-ogg-flac-chained-meta: REF = $(SRC_PATH)/tests/ref/fate/ogg-flac-chained-meta.txt +fate-ogg-flac-chained-meta: CMD = run $(APITESTSDIR)/api-dump-stream-meta-test$(EXESUF) $(TARGET_SAMPLES)/ogg-flac/chained-meta.ogg + +FATE_OGG_FLAC-$(call DEMDEC, OGG, FLAC) += $(FATE_OGG_FLAC) + +FATE_SAMPLES_DUMP_STREAM_META += $(FATE_OGG_FLAC-yes) + +FATE_EXTERN += $(FATE_OGG_FLAC-yes) + +fate-ogg-flac: $(FATE_OGG_FLAC-yes) diff --git a/tests/fate/ogg-opus.mak b/tests/fate/ogg-opus.mak new file mode 100644 index 0000000000..54b6fbabde --- /dev/null +++ b/tests/fate/ogg-opus.mak @@ -0,0 +1,11 @@ +FATE_OGG_OPUS += fate-ogg-opus-chained-meta +fate-ogg-opus-chained-meta: REF = $(SRC_PATH)/tests/ref/fate/ogg-opus-chained-meta.txt +fate-ogg-opus-chained-meta: CMD = run $(APITESTSDIR)/api-dump-stream-meta-test$(EXESUF) $(TARGET_SAMPLES)/ogg-opus/chained-meta.ogg + +FATE_OGG_OPUS-$(call DEMDEC, OGG, OPUS) += $(FATE_OGG_OPUS) + +FATE_SAMPLES_DUMP_STREAM_META += $(FATE_OGG_OPUS-yes) + +FATE_EXTERN += $(FATE_OGG_OPUS-yes) + +fate-ogg-opus: $(FATE_OGG_OPUS-yes) diff --git a/tests/fate/ogg-vorbis.mak b/tests/fate/ogg-vorbis.mak new file mode 100644 index 0000000000..74805d591e --- /dev/null +++ b/tests/fate/ogg-vorbis.mak @@ -0,0 +1,11 @@ +FATE_OGG_VORBIS += fate-ogg-vorbis-chained-meta +fate-ogg-vorbis-chained-meta: REF = $(SRC_PATH)/tests/ref/fate/ogg-vorbis-chained-meta.txt +fate-ogg-vorbis-chained-meta: CMD = run $(APITESTSDIR)/api-dump-stream-meta-test$(EXESUF) $(TARGET_SAMPLES)/ogg-vorbis/chained-meta.ogg + +FATE_OGG_VORBIS-$(call DEMDEC, OGG, VORBIS) += $(FATE_OGG_VORBIS) + +FATE_SAMPLES_DUMP_STREAM_META += $(FATE_OGG_VORBIS-yes) + +FATE_EXTERN += $(FATE_OGG_VORBIS-yes) + +fate-ogg-vorbis: $(FATE_OGG_VORBIS-yes) diff --git a/tests/ref/fate/ogg-flac-chained-meta.txt b/tests/ref/fate/ogg-flac-chained-meta.txt new file mode 100644 index 0000000000..ad20ba935f --- /dev/null +++ b/tests/ref/fate/ogg-flac-chained-meta.txt @@ -0,0 +1,12 @@ +Stream ID: 0, codec name: flac, metadata: encoder=Lavc61.19.100 flac:title=First Stream +Stream ID: 0, packet PTS: 0, packet DTS: 0 +Stream ID: 0, new metadata: encoder=Lavc61.19.100 flac:title=First Stream +Stream ID: 0, frame PTS: 0, metadata: N/A +Stream ID: 0, packet PTS: 4608, packet DTS: 4608 +Stream ID: 0, frame PTS: 4608, metadata: N/A +Stream ID: 0, packet PTS: 0, packet DTS: 0 +Stream ID: 0, packet PTS: 0, packet DTS: 0 +Stream ID: 0, packet PTS: 0, packet DTS: 0 +Stream ID: 0, frame PTS: 0, metadata: N/A +Stream ID: 0, packet PTS: 4608, packet DTS: 4608 +Stream ID: 0, frame PTS: 4608, metadata: N/A diff --git a/tests/ref/fate/ogg-opus-chained-meta.txt b/tests/ref/fate/ogg-opus-chained-meta.txt new file mode 100644 index 0000000000..fc84b8b703 --- /dev/null +++ b/tests/ref/fate/ogg-opus-chained-meta.txt @@ -0,0 +1,27 @@ +Stream ID: 0, codec name: opus, metadata: encoder=Lavc61.19.100 libopus:title=First Stream +Stream ID: 0, packet PTS: -312, packet DTS: -312 +Stream ID: 0, new metadata: encoder=Lavc61.19.100 libopus:title=First Stream +Stream ID: 0, frame PTS: -312, metadata: N/A +Stream ID: 0, packet PTS: 648, packet DTS: 648 +Stream ID: 0, frame PTS: 648, metadata: N/A +Stream ID: 0, packet PTS: 1608, packet DTS: 1608 +Stream ID: 0, frame PTS: 1608, metadata: N/A +Stream ID: 0, packet PTS: 2568, packet DTS: 2568 +Stream ID: 0, frame PTS: 2568, metadata: N/A +Stream ID: 0, packet PTS: 3528, packet DTS: 3528 +Stream ID: 0, frame PTS: 3528, metadata: N/A +Stream ID: 0, packet PTS: 4488, packet DTS: 4488 +Stream ID: 0, frame PTS: 4488, metadata: N/A +Stream ID: 0, packet PTS: -312, packet DTS: -312 +Stream ID: 0, new metadata: encoder=Lavc61.19.100 libopus;Lavc61.19.100 libopus:title=First Stream;Second Stream +Stream ID: 0, frame PTS: -312, metadata: N/A +Stream ID: 0, packet PTS: 648, packet DTS: 648 +Stream ID: 0, frame PTS: 648, metadata: N/A +Stream ID: 0, packet PTS: 1608, packet DTS: 1608 +Stream ID: 0, frame PTS: 1608, metadata: N/A +Stream ID: 0, packet PTS: 2568, packet DTS: 2568 +Stream ID: 0, frame PTS: 2568, metadata: N/A +Stream ID: 0, packet PTS: 3528, packet DTS: 3528 +Stream ID: 0, frame PTS: 3528, metadata: N/A +Stream ID: 0, packet PTS: 4488, packet DTS: 4488 +Stream ID: 0, frame PTS: 4488, metadata: N/A diff --git a/tests/ref/fate/ogg-vorbis-chained-meta.txt b/tests/ref/fate/ogg-vorbis-chained-meta.txt new file mode 100644 index 0000000000..b7a97c90e2 --- /dev/null +++ b/tests/ref/fate/ogg-vorbis-chained-meta.txt @@ -0,0 +1,17 @@ +Stream ID: 0, codec name: vorbis, metadata: encoder=Lavc61.19.100 libvorbis:title=First Stream +Stream ID: 0, packet PTS: 0, packet DTS: 0 +Stream ID: 0, new metadata: encoder=Lavc61.19.100 libvorbis:title=First Stream +Stream ID: 0, packet PTS: 128, packet DTS: 128 +Stream ID: 0, frame PTS: 128, metadata: N/A +Stream ID: 0, packet PTS: 704, packet DTS: 704 +Stream ID: 0, frame PTS: 704, metadata: N/A +Stream ID: 0, packet PTS: 0, packet DTS: 0 +Stream ID: 0, packet PTS: 0, packet DTS: 0 +Stream ID: 0, new metadata: encoder=Lavc61.19.100 libvorbis:title=Second Stream +Stream ID: 0, packet PTS: 0, packet DTS: 0 +Stream ID: 0, packet PTS: 0, packet DTS: 0 +Stream ID: 0, frame PTS: 0, metadata: N/A +Stream ID: 0, packet PTS: 128, packet DTS: 128 +Stream ID: 0, frame PTS: 128, metadata: N/A +Stream ID: 0, packet PTS: 704, packet DTS: 704 +Stream ID: 0, frame PTS: 704, metadata: N/A -- 2.39.5 (Apple Git-154) _______________________________________________ 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] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 1/2] tests: Add stream dump test API util, use it to dump stream data for chained ogg/{vorbis, opus, flac} streams. 2025-04-28 23:31 ` [FFmpeg-devel] [PATCH v2 1/2] tests: Add stream dump test API util, use it to dump stream data for chained ogg/{vorbis, opus, flac} streams Romain Beauxis @ 2025-04-29 21:25 ` Michael Niedermayer 0 siblings, 0 replies; 9+ messages in thread From: Michael Niedermayer @ 2025-04-29 21:25 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 1320 bytes --] Hi Romain On Mon, Apr 28, 2025 at 06:31:35PM -0500, Romain Beauxis wrote: > --- > tests/Makefile | 4 + > tests/api/Makefile | 2 +- > tests/api/api-dump-stream-meta-test.c | 182 +++++++++++++++++++++ > tests/fate/ogg-flac.mak | 11 ++ > tests/fate/ogg-opus.mak | 11 ++ > tests/fate/ogg-vorbis.mak | 11 ++ > tests/ref/fate/ogg-flac-chained-meta.txt | 12 ++ > tests/ref/fate/ogg-opus-chained-meta.txt | 27 +++ > tests/ref/fate/ogg-vorbis-chained-meta.txt | 17 ++ > 9 files changed, 276 insertions(+), 1 deletion(-) > create mode 100644 tests/api/api-dump-stream-meta-test.c > create mode 100644 tests/fate/ogg-flac.mak > create mode 100644 tests/fate/ogg-opus.mak > create mode 100644 tests/fate/ogg-vorbis.mak > create mode 100644 tests/ref/fate/ogg-flac-chained-meta.txt > create mode 100644 tests/ref/fate/ogg-opus-chained-meta.txt > create mode 100644 tests/ref/fate/ogg-vorbis-chained-meta.txt LGTM will apply unless someone else is faster thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB While the State exists there can be no freedom; when there is freedom there will be no State. -- Vladimir Lenin [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] [-- Attachment #2: 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] 9+ messages in thread
* [FFmpeg-devel] [PATCH v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output. 2025-04-28 23:31 [FFmpeg-devel] [PATCH v2 0/2] Remove chained ogg stream header packets from demuxer Romain Beauxis 2025-04-28 23:31 ` [FFmpeg-devel] [PATCH v2 1/2] tests: Add stream dump test API util, use it to dump stream data for chained ogg/{vorbis, opus, flac} streams Romain Beauxis @ 2025-04-28 23:31 ` Romain Beauxis 2025-04-29 21:35 ` Michael Niedermayer 1 sibling, 1 reply; 9+ messages in thread From: Romain Beauxis @ 2025-04-28 23:31 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Romain Beauxis --- libavformat/oggdec.c | 26 ++++++++++---------- libavformat/oggdec.h | 6 +++++ libavformat/oggparseflac.c | 28 ++++++++++++++++++++-- libavformat/oggparseopus.c | 12 ++++++++++ libavformat/oggparsevorbis.c | 12 ++++++++-- tests/ref/fate/ogg-flac-chained-meta.txt | 2 -- tests/ref/fate/ogg-opus-chained-meta.txt | 1 - tests/ref/fate/ogg-vorbis-chained-meta.txt | 3 --- 8 files changed, 68 insertions(+), 22 deletions(-) diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c index 5339fdd32c..5557eb4a14 100644 --- a/libavformat/oggdec.c +++ b/libavformat/oggdec.c @@ -239,10 +239,6 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, os->start_trimming = 0; os->end_trimming = 0; - /* Chained files have extradata as a new packet */ - if (codec == &ff_opus_codec) - os->header = -1; - return i; } @@ -605,20 +601,26 @@ static int ogg_packet(AVFormatContext *s, int *sid, int *dstart, int *dsize, } else { os->pflags = 0; os->pduration = 0; + + ret = 0; if (os->codec && os->codec->packet) { if ((ret = os->codec->packet(s, idx)) < 0) { av_log(s, AV_LOG_ERROR, "Packet processing failed: %s\n", av_err2str(ret)); return ret; } } - if (sid) - *sid = idx; - if (dstart) - *dstart = os->pstart; - if (dsize) - *dsize = os->psize; - if (fpos) - *fpos = os->sync_pos; + + if (!ret) { + if (sid) + *sid = idx; + if (dstart) + *dstart = os->pstart; + if (dsize) + *dsize = os->psize; + if (fpos) + *fpos = os->sync_pos; + } + os->pstart += os->psize; os->psize = 0; if(os->pstart == os->bufpos) diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h index 43df23f4cb..09f698f99a 100644 --- a/libavformat/oggdec.h +++ b/libavformat/oggdec.h @@ -38,6 +38,12 @@ struct ogg_codec { * -1 if an error occurred or for unsupported stream */ int (*header)(AVFormatContext *, int); + /** + * Attempt to process a packet as a data packet + * @return 1 if the packet was a header from a chained bitstream. + * 0 if the packet was a regular data packet. + * -1 if an error occurred or for unsupported stream + */ int (*packet)(AVFormatContext *, int); /** * Translate a granule into a timestamp. diff --git a/libavformat/oggparseflac.c b/libavformat/oggparseflac.c index f25ed9cc15..d66b85b09e 100644 --- a/libavformat/oggparseflac.c +++ b/libavformat/oggparseflac.c @@ -27,6 +27,8 @@ #include "oggdec.h" #define OGG_FLAC_METADATA_TYPE_STREAMINFO 0x7F +#define OGG_FLAC_MAGIC "\177FLAC" +#define OGG_FLAC_MAGIC_SIZE sizeof(OGG_FLAC_MAGIC)-1 static int flac_header (AVFormatContext * s, int idx) @@ -78,6 +80,27 @@ flac_header (AVFormatContext * s, int idx) return 1; } +static int +flac_packet (AVFormatContext * s, int idx) +{ + struct ogg *ogg = s->priv_data; + struct ogg_stream *os = ogg->streams + idx; + + if (os->psize > OGG_FLAC_MAGIC_SIZE && + !memcmp( + os->buf + os->pstart, + OGG_FLAC_MAGIC, + OGG_FLAC_MAGIC_SIZE)) + return 1; + + if (os->psize > 0 && + ((os->buf[os->pstart] & 0x7F) == FLAC_METADATA_TYPE_VORBIS_COMMENT)) { + return 1; + } + + return 0; +} + static int old_flac_header (AVFormatContext * s, int idx) { @@ -127,10 +150,11 @@ fail: } const struct ogg_codec ff_flac_codec = { - .magic = "\177FLAC", - .magicsize = 5, + .magic = OGG_FLAC_MAGIC, + .magicsize = OGG_FLAC_MAGIC_SIZE, .header = flac_header, .nb_header = 2, + .packet = flac_packet, }; const struct ogg_codec ff_old_flac_codec = { diff --git a/libavformat/oggparseopus.c b/libavformat/oggparseopus.c index 218e9df581..07ddba8c82 100644 --- a/libavformat/oggparseopus.c +++ b/libavformat/oggparseopus.c @@ -81,6 +81,7 @@ static int opus_header(AVFormatContext *avf, int idx) if (priv->need_comments) { if (os->psize < 8 || memcmp(packet, "OpusTags", 8)) return AVERROR_INVALIDDATA; + ff_vorbis_stream_comment(avf, st, packet + 8, os->psize - 8); priv->need_comments--; return 1; @@ -125,6 +126,17 @@ static int opus_packet(AVFormatContext *avf, int idx) return AVERROR_INVALIDDATA; } + if (os->psize > 8 && !memcmp(packet, "OpusHead", 8)) { + if ((ret = ff_alloc_extradata(st->codecpar, os->psize)) < 0) + return ret; + + memcpy(st->codecpar->extradata, packet, os->psize); + return 1; + } + + if (os->psize > 8 && !memcmp(packet, "OpusTags", 8)) + return 1; + if ((!os->lastpts || os->lastpts == AV_NOPTS_VALUE) && !(os->flags & OGG_FLAG_EOS)) { int seg, d; int duration; diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c index 9f50ab9ffc..248ac4a416 100644 --- a/libavformat/oggparsevorbis.c +++ b/libavformat/oggparsevorbis.c @@ -418,6 +418,7 @@ static int vorbis_packet(AVFormatContext *s, int idx) struct ogg_stream *os = ogg->streams + idx; struct oggvorbis_private *priv = os->private; int duration, flags = 0; + int skip_packet = 0; if (!priv->vp) return AVERROR_INVALIDDATA; @@ -477,10 +478,17 @@ static int vorbis_packet(AVFormatContext *s, int idx) /* parse packet duration */ if (os->psize > 0) { duration = av_vorbis_parse_frame_flags(priv->vp, os->buf + os->pstart, 1, &flags); + if (duration < 0) { os->pflags |= AV_PKT_FLAG_CORRUPT; return 0; - } else if (flags & VORBIS_FLAG_COMMENT) { + } + + skip_packet = flags & VORBIS_FLAG_HEADER || + flags & VORBIS_FLAG_COMMENT || + flags & VORBIS_FLAG_SETUP; + + if (flags & VORBIS_FLAG_COMMENT) { vorbis_update_metadata(s, idx); flags = 0; } @@ -505,7 +513,7 @@ static int vorbis_packet(AVFormatContext *s, int idx) priv->final_duration += os->pduration; } - return 0; + return skip_packet; } const struct ogg_codec ff_vorbis_codec = { diff --git a/tests/ref/fate/ogg-flac-chained-meta.txt b/tests/ref/fate/ogg-flac-chained-meta.txt index ad20ba935f..28e22aa29e 100644 --- a/tests/ref/fate/ogg-flac-chained-meta.txt +++ b/tests/ref/fate/ogg-flac-chained-meta.txt @@ -5,8 +5,6 @@ Stream ID: 0, frame PTS: 0, metadata: N/A Stream ID: 0, packet PTS: 4608, packet DTS: 4608 Stream ID: 0, frame PTS: 4608, metadata: N/A Stream ID: 0, packet PTS: 0, packet DTS: 0 -Stream ID: 0, packet PTS: 0, packet DTS: 0 -Stream ID: 0, packet PTS: 0, packet DTS: 0 Stream ID: 0, frame PTS: 0, metadata: N/A Stream ID: 0, packet PTS: 4608, packet DTS: 4608 Stream ID: 0, frame PTS: 4608, metadata: N/A diff --git a/tests/ref/fate/ogg-opus-chained-meta.txt b/tests/ref/fate/ogg-opus-chained-meta.txt index fc84b8b703..addc41c1eb 100644 --- a/tests/ref/fate/ogg-opus-chained-meta.txt +++ b/tests/ref/fate/ogg-opus-chained-meta.txt @@ -13,7 +13,6 @@ Stream ID: 0, frame PTS: 3528, metadata: N/A Stream ID: 0, packet PTS: 4488, packet DTS: 4488 Stream ID: 0, frame PTS: 4488, metadata: N/A Stream ID: 0, packet PTS: -312, packet DTS: -312 -Stream ID: 0, new metadata: encoder=Lavc61.19.100 libopus;Lavc61.19.100 libopus:title=First Stream;Second Stream Stream ID: 0, frame PTS: -312, metadata: N/A Stream ID: 0, packet PTS: 648, packet DTS: 648 Stream ID: 0, frame PTS: 648, metadata: N/A diff --git a/tests/ref/fate/ogg-vorbis-chained-meta.txt b/tests/ref/fate/ogg-vorbis-chained-meta.txt index b7a97c90e2..1206f86c1f 100644 --- a/tests/ref/fate/ogg-vorbis-chained-meta.txt +++ b/tests/ref/fate/ogg-vorbis-chained-meta.txt @@ -6,10 +6,7 @@ Stream ID: 0, frame PTS: 128, metadata: N/A Stream ID: 0, packet PTS: 704, packet DTS: 704 Stream ID: 0, frame PTS: 704, metadata: N/A Stream ID: 0, packet PTS: 0, packet DTS: 0 -Stream ID: 0, packet PTS: 0, packet DTS: 0 Stream ID: 0, new metadata: encoder=Lavc61.19.100 libvorbis:title=Second Stream -Stream ID: 0, packet PTS: 0, packet DTS: 0 -Stream ID: 0, packet PTS: 0, packet DTS: 0 Stream ID: 0, frame PTS: 0, metadata: N/A Stream ID: 0, packet PTS: 128, packet DTS: 128 Stream ID: 0, frame PTS: 128, metadata: N/A -- 2.39.5 (Apple Git-154) _______________________________________________ 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] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output. 2025-04-28 23:31 ` [FFmpeg-devel] [PATCH v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output Romain Beauxis @ 2025-04-29 21:35 ` Michael Niedermayer 2025-04-29 22:42 ` Romain Beauxis 0 siblings, 1 reply; 9+ messages in thread From: Michael Niedermayer @ 2025-04-29 21:35 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 6763 bytes --] Hi On Mon, Apr 28, 2025 at 06:31:36PM -0500, Romain Beauxis wrote: > --- > libavformat/oggdec.c | 26 ++++++++++---------- > libavformat/oggdec.h | 6 +++++ > libavformat/oggparseflac.c | 28 ++++++++++++++++++++-- > libavformat/oggparseopus.c | 12 ++++++++++ > libavformat/oggparsevorbis.c | 12 ++++++++-- > tests/ref/fate/ogg-flac-chained-meta.txt | 2 -- > tests/ref/fate/ogg-opus-chained-meta.txt | 1 - > tests/ref/fate/ogg-vorbis-chained-meta.txt | 3 --- > 8 files changed, 68 insertions(+), 22 deletions(-) > > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c > index 5339fdd32c..5557eb4a14 100644 > --- a/libavformat/oggdec.c > +++ b/libavformat/oggdec.c > @@ -239,10 +239,6 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, > os->start_trimming = 0; > os->end_trimming = 0; > > - /* Chained files have extradata as a new packet */ > - if (codec == &ff_opus_codec) > - os->header = -1; > - > return i; > } > > @@ -605,20 +601,26 @@ static int ogg_packet(AVFormatContext *s, int *sid, int *dstart, int *dsize, > } else { > os->pflags = 0; > os->pduration = 0; > + > + ret = 0; > if (os->codec && os->codec->packet) { > if ((ret = os->codec->packet(s, idx)) < 0) { > av_log(s, AV_LOG_ERROR, "Packet processing failed: %s\n", av_err2str(ret)); > return ret; > } > } > - if (sid) > - *sid = idx; > - if (dstart) > - *dstart = os->pstart; > - if (dsize) > - *dsize = os->psize; > - if (fpos) > - *fpos = os->sync_pos; > + > + if (!ret) { > + if (sid) > + *sid = idx; > + if (dstart) > + *dstart = os->pstart; > + if (dsize) > + *dsize = os->psize; > + if (fpos) > + *fpos = os->sync_pos; > + } > + > os->pstart += os->psize; > os->psize = 0; > if(os->pstart == os->bufpos) > diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h > index 43df23f4cb..09f698f99a 100644 > --- a/libavformat/oggdec.h > +++ b/libavformat/oggdec.h > @@ -38,6 +38,12 @@ struct ogg_codec { > * -1 if an error occurred or for unsupported stream > */ > int (*header)(AVFormatContext *, int); > + /** > + * Attempt to process a packet as a data packet > + * @return 1 if the packet was a header from a chained bitstream. > + * 0 if the packet was a regular data packet. > + * -1 if an error occurred or for unsupported stream > + */ > int (*packet)(AVFormatContext *, int); > /** > * Translate a granule into a timestamp. ok, but seems unrelated > diff --git a/libavformat/oggparseflac.c b/libavformat/oggparseflac.c > index f25ed9cc15..d66b85b09e 100644 > --- a/libavformat/oggparseflac.c > +++ b/libavformat/oggparseflac.c > @@ -27,6 +27,8 @@ > #include "oggdec.h" > > #define OGG_FLAC_METADATA_TYPE_STREAMINFO 0x7F > +#define OGG_FLAC_MAGIC "\177FLAC" > +#define OGG_FLAC_MAGIC_SIZE sizeof(OGG_FLAC_MAGIC)-1 factoring this out could be a seperate patch > > static int > flac_header (AVFormatContext * s, int idx) > @@ -78,6 +80,27 @@ flac_header (AVFormatContext * s, int idx) > return 1; > } > > +static int > +flac_packet (AVFormatContext * s, int idx) > +{ > + struct ogg *ogg = s->priv_data; > + struct ogg_stream *os = ogg->streams + idx; > + > + if (os->psize > OGG_FLAC_MAGIC_SIZE && > + !memcmp( > + os->buf + os->pstart, > + OGG_FLAC_MAGIC, > + OGG_FLAC_MAGIC_SIZE)) > + return 1; > + > + if (os->psize > 0 && > + ((os->buf[os->pstart] & 0x7F) == FLAC_METADATA_TYPE_VORBIS_COMMENT)) { > + return 1; > + } > + > + return 0; > +} > + > static int > old_flac_header (AVFormatContext * s, int idx) > { > @@ -127,10 +150,11 @@ fail: > } > > const struct ogg_codec ff_flac_codec = { > - .magic = "\177FLAC", > - .magicsize = 5, > + .magic = OGG_FLAC_MAGIC, > + .magicsize = OGG_FLAC_MAGIC_SIZE, > .header = flac_header, > .nb_header = 2, > + .packet = flac_packet, > }; > > const struct ogg_codec ff_old_flac_codec = { > diff --git a/libavformat/oggparseopus.c b/libavformat/oggparseopus.c > index 218e9df581..07ddba8c82 100644 > --- a/libavformat/oggparseopus.c > +++ b/libavformat/oggparseopus.c > @@ -81,6 +81,7 @@ static int opus_header(AVFormatContext *avf, int idx) > if (priv->need_comments) { > if (os->psize < 8 || memcmp(packet, "OpusTags", 8)) > return AVERROR_INVALIDDATA; > + > ff_vorbis_stream_comment(avf, st, packet + 8, os->psize - 8); > priv->need_comments--; > return 1; stray change > @@ -125,6 +126,17 @@ static int opus_packet(AVFormatContext *avf, int idx) > return AVERROR_INVALIDDATA; > } > > + if (os->psize > 8 && !memcmp(packet, "OpusHead", 8)) { > + if ((ret = ff_alloc_extradata(st->codecpar, os->psize)) < 0) > + return ret; > + > + memcpy(st->codecpar->extradata, packet, os->psize); > + return 1; > + } > + > + if (os->psize > 8 && !memcmp(packet, "OpusTags", 8)) > + return 1; > + > if ((!os->lastpts || os->lastpts == AV_NOPTS_VALUE) && !(os->flags & OGG_FLAG_EOS)) { > int seg, d; > int duration; > diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c > index 9f50ab9ffc..248ac4a416 100644 > --- a/libavformat/oggparsevorbis.c > +++ b/libavformat/oggparsevorbis.c > @@ -418,6 +418,7 @@ static int vorbis_packet(AVFormatContext *s, int idx) > struct ogg_stream *os = ogg->streams + idx; > struct oggvorbis_private *priv = os->private; > int duration, flags = 0; > + int skip_packet = 0; > > if (!priv->vp) > return AVERROR_INVALIDDATA; > @@ -477,10 +478,17 @@ static int vorbis_packet(AVFormatContext *s, int idx) > /* parse packet duration */ > if (os->psize > 0) { > duration = av_vorbis_parse_frame_flags(priv->vp, os->buf + os->pstart, 1, &flags); > + > if (duration < 0) { stray change [...] thx -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I am the wisest man alive, for I know one thing, and that is that I know nothing. -- Socrates [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] [-- Attachment #2: 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] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output. 2025-04-29 21:35 ` Michael Niedermayer @ 2025-04-29 22:42 ` Romain Beauxis 2025-04-30 0:17 ` Michael Niedermayer 0 siblings, 1 reply; 9+ messages in thread From: Romain Beauxis @ 2025-04-29 22:42 UTC (permalink / raw) To: FFmpeg development discussions and patches Le mar. 29 avr. 2025 à 16:35, Michael Niedermayer <michael@niedermayer.cc> a écrit : > > Hi > > On Mon, Apr 28, 2025 at 06:31:36PM -0500, Romain Beauxis wrote: > > --- > > libavformat/oggdec.c | 26 ++++++++++---------- > > libavformat/oggdec.h | 6 +++++ > > libavformat/oggparseflac.c | 28 ++++++++++++++++++++-- > > libavformat/oggparseopus.c | 12 ++++++++++ > > libavformat/oggparsevorbis.c | 12 ++++++++-- > > tests/ref/fate/ogg-flac-chained-meta.txt | 2 -- > > tests/ref/fate/ogg-opus-chained-meta.txt | 1 - > > tests/ref/fate/ogg-vorbis-chained-meta.txt | 3 --- > > 8 files changed, 68 insertions(+), 22 deletions(-) > > > > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c > > index 5339fdd32c..5557eb4a14 100644 > > --- a/libavformat/oggdec.c > > +++ b/libavformat/oggdec.c > > @@ -239,10 +239,6 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, > > os->start_trimming = 0; > > os->end_trimming = 0; > > > > - /* Chained files have extradata as a new packet */ > > - if (codec == &ff_opus_codec) > > - os->header = -1; > > - > > return i; > > } > > > > @@ -605,20 +601,26 @@ static int ogg_packet(AVFormatContext *s, int *sid, int *dstart, int *dsize, > > } else { > > os->pflags = 0; > > os->pduration = 0; > > + > > + ret = 0; > > if (os->codec && os->codec->packet) { > > if ((ret = os->codec->packet(s, idx)) < 0) { > > av_log(s, AV_LOG_ERROR, "Packet processing failed: %s\n", av_err2str(ret)); > > return ret; > > } > > } > > - if (sid) > > - *sid = idx; > > - if (dstart) > > - *dstart = os->pstart; > > - if (dsize) > > - *dsize = os->psize; > > - if (fpos) > > - *fpos = os->sync_pos; > > + > > + if (!ret) { > > + if (sid) > > + *sid = idx; > > + if (dstart) > > + *dstart = os->pstart; > > + if (dsize) > > + *dsize = os->psize; > > + if (fpos) > > + *fpos = os->sync_pos; > > + } > > + > > os->pstart += os->psize; > > os->psize = 0; > > if(os->pstart == os->bufpos) > > > diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h > > index 43df23f4cb..09f698f99a 100644 > > --- a/libavformat/oggdec.h > > +++ b/libavformat/oggdec.h > > @@ -38,6 +38,12 @@ struct ogg_codec { > > * -1 if an error occurred or for unsupported stream > > */ > > int (*header)(AVFormatContext *, int); > > + /** > > + * Attempt to process a packet as a data packet > > + * @return 1 if the packet was a header from a chained bitstream. > > + * 0 if the packet was a regular data packet. > > + * -1 if an error occurred or for unsupported stream > > + */ > > int (*packet)(AVFormatContext *, int); > > /** > > * Translate a granule into a timestamp. > > ok, but seems unrelated This is a new convention to allow the parser to know when to skip a packet. Previous to that, return value of 1 did not have specific meaning. > > diff --git a/libavformat/oggparseflac.c b/libavformat/oggparseflac.c > > index f25ed9cc15..d66b85b09e 100644 > > --- a/libavformat/oggparseflac.c > > +++ b/libavformat/oggparseflac.c > > @@ -27,6 +27,8 @@ > > #include "oggdec.h" > > > > #define OGG_FLAC_METADATA_TYPE_STREAMINFO 0x7F > > +#define OGG_FLAC_MAGIC "\177FLAC" > > +#define OGG_FLAC_MAGIC_SIZE sizeof(OGG_FLAC_MAGIC)-1 > > factoring this out could be a seperate patch > > > > > > static int > > flac_header (AVFormatContext * s, int idx) > > @@ -78,6 +80,27 @@ flac_header (AVFormatContext * s, int idx) > > return 1; > > } > > > > +static int > > +flac_packet (AVFormatContext * s, int idx) > > +{ > > + struct ogg *ogg = s->priv_data; > > + struct ogg_stream *os = ogg->streams + idx; > > + > > + if (os->psize > OGG_FLAC_MAGIC_SIZE && > > + !memcmp( > > + os->buf + os->pstart, > > + OGG_FLAC_MAGIC, > > + OGG_FLAC_MAGIC_SIZE)) > > + return 1; > > + > > + if (os->psize > 0 && > > + ((os->buf[os->pstart] & 0x7F) == FLAC_METADATA_TYPE_VORBIS_COMMENT)) { > > + return 1; > > + } > > + > > + return 0; > > +} > > + > > static int > > old_flac_header (AVFormatContext * s, int idx) > > { > > @@ -127,10 +150,11 @@ fail: > > } > > > > const struct ogg_codec ff_flac_codec = { > > - .magic = "\177FLAC", > > - .magicsize = 5, > > + .magic = OGG_FLAC_MAGIC, > > + .magicsize = OGG_FLAC_MAGIC_SIZE, > > .header = flac_header, > > .nb_header = 2, > > + .packet = flac_packet, > > }; > > > > const struct ogg_codec ff_old_flac_codec = { > > > diff --git a/libavformat/oggparseopus.c b/libavformat/oggparseopus.c > > index 218e9df581..07ddba8c82 100644 > > --- a/libavformat/oggparseopus.c > > +++ b/libavformat/oggparseopus.c > > @@ -81,6 +81,7 @@ static int opus_header(AVFormatContext *avf, int idx) > > if (priv->need_comments) { > > if (os->psize < 8 || memcmp(packet, "OpusTags", 8)) > > return AVERROR_INVALIDDATA; > > + > > ff_vorbis_stream_comment(avf, st, packet + 8, os->psize - 8); > > priv->need_comments--; > > return 1; > > stray change > > > > @@ -125,6 +126,17 @@ static int opus_packet(AVFormatContext *avf, int idx) > > return AVERROR_INVALIDDATA; > > } > > > > + if (os->psize > 8 && !memcmp(packet, "OpusHead", 8)) { > > + if ((ret = ff_alloc_extradata(st->codecpar, os->psize)) < 0) > > + return ret; > > + > > + memcpy(st->codecpar->extradata, packet, os->psize); > > + return 1; > > + } > > + > > + if (os->psize > 8 && !memcmp(packet, "OpusTags", 8)) > > + return 1; > > + > > if ((!os->lastpts || os->lastpts == AV_NOPTS_VALUE) && !(os->flags & OGG_FLAG_EOS)) { > > int seg, d; > > int duration; > > diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c > > index 9f50ab9ffc..248ac4a416 100644 > > --- a/libavformat/oggparsevorbis.c > > +++ b/libavformat/oggparsevorbis.c > > @@ -418,6 +418,7 @@ static int vorbis_packet(AVFormatContext *s, int idx) > > struct ogg_stream *os = ogg->streams + idx; > > struct oggvorbis_private *priv = os->private; > > int duration, flags = 0; > > + int skip_packet = 0; > > > > if (!priv->vp) > > return AVERROR_INVALIDDATA; > > > @@ -477,10 +478,17 @@ static int vorbis_packet(AVFormatContext *s, int idx) > > /* parse packet duration */ > > if (os->psize > 0) { > > duration = av_vorbis_parse_frame_flags(priv->vp, os->buf + os->pstart, 1, &flags); > > + > > if (duration < 0) { > > stray change > > [...] Thanks for the review. You can find an updated commit here if you want to avoid another post to the ML: https://github.com/toots/FFmpeg/commit/140c4cafe660abed67212a4b4c128628b4dd01f5 > thx > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > I am the wisest man alive, for I know one thing, and that is that I know > nothing. -- Socrates > _______________________________________________ > 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] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output. 2025-04-29 22:42 ` Romain Beauxis @ 2025-04-30 0:17 ` Michael Niedermayer 2025-04-30 0:49 ` Romain Beauxis 0 siblings, 1 reply; 9+ messages in thread From: Michael Niedermayer @ 2025-04-30 0:17 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 4148 bytes --] Hi Romain On Tue, Apr 29, 2025 at 05:42:22PM -0500, Romain Beauxis wrote: > Le mar. 29 avr. 2025 à 16:35, Michael Niedermayer > <michael@niedermayer.cc> a écrit : > > > > Hi > > > > On Mon, Apr 28, 2025 at 06:31:36PM -0500, Romain Beauxis wrote: > > > --- > > > libavformat/oggdec.c | 26 ++++++++++---------- > > > libavformat/oggdec.h | 6 +++++ > > > libavformat/oggparseflac.c | 28 ++++++++++++++++++++-- > > > libavformat/oggparseopus.c | 12 ++++++++++ > > > libavformat/oggparsevorbis.c | 12 ++++++++-- > > > tests/ref/fate/ogg-flac-chained-meta.txt | 2 -- > > > tests/ref/fate/ogg-opus-chained-meta.txt | 1 - > > > tests/ref/fate/ogg-vorbis-chained-meta.txt | 3 --- > > > 8 files changed, 68 insertions(+), 22 deletions(-) > > > > > > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c > > > index 5339fdd32c..5557eb4a14 100644 > > > --- a/libavformat/oggdec.c > > > +++ b/libavformat/oggdec.c > > > @@ -239,10 +239,6 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, > > > os->start_trimming = 0; > > > os->end_trimming = 0; > > > > > > - /* Chained files have extradata as a new packet */ > > > - if (codec == &ff_opus_codec) > > > - os->header = -1; > > > - > > > return i; > > > } > > > > > > @@ -605,20 +601,26 @@ static int ogg_packet(AVFormatContext *s, int *sid, int *dstart, int *dsize, > > > } else { > > > os->pflags = 0; > > > os->pduration = 0; > > > + > > > + ret = 0; > > > if (os->codec && os->codec->packet) { > > > if ((ret = os->codec->packet(s, idx)) < 0) { > > > av_log(s, AV_LOG_ERROR, "Packet processing failed: %s\n", av_err2str(ret)); > > > return ret; > > > } > > > } > > > - if (sid) > > > - *sid = idx; > > > - if (dstart) > > > - *dstart = os->pstart; > > > - if (dsize) > > > - *dsize = os->psize; > > > - if (fpos) > > > - *fpos = os->sync_pos; > > > + > > > + if (!ret) { > > > + if (sid) > > > + *sid = idx; > > > + if (dstart) > > > + *dstart = os->pstart; > > > + if (dsize) > > > + *dsize = os->psize; > > > + if (fpos) > > > + *fpos = os->sync_pos; > > > + } > > > + > > > os->pstart += os->psize; > > > os->psize = 0; > > > if(os->pstart == os->bufpos) > > > > > diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h > > > index 43df23f4cb..09f698f99a 100644 > > > --- a/libavformat/oggdec.h > > > +++ b/libavformat/oggdec.h > > > @@ -38,6 +38,12 @@ struct ogg_codec { > > > * -1 if an error occurred or for unsupported stream > > > */ > > > int (*header)(AVFormatContext *, int); > > > + /** > > > + * Attempt to process a packet as a data packet > > > + * @return 1 if the packet was a header from a chained bitstream. > > > + * 0 if the packet was a regular data packet. > > > + * -1 if an error occurred or for unsupported stream > > > + */ > > > int (*packet)(AVFormatContext *, int); > > > /** > > > * Translate a granule into a timestamp. > > > > ok, but seems unrelated > > This is a new convention to allow the parser to know when to skip a > packet. Previous to that, return value of 1 did not have specific > meaning. Do you think this would merit a seperate patch ? I mean patch #1 changing the packet() return value and clearly stating that in the commit message and patch #2 using the new value ? I think it would make things clearer thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The bravest are surely those who have the clearest vision of what is before them, glory and danger alike, and yet notwithstanding go out to meet it. -- Thucydides [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] [-- Attachment #2: 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] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output. 2025-04-30 0:17 ` Michael Niedermayer @ 2025-04-30 0:49 ` Romain Beauxis 2025-05-02 20:28 ` Michael Niedermayer 0 siblings, 1 reply; 9+ messages in thread From: Romain Beauxis @ 2025-04-30 0:49 UTC (permalink / raw) To: FFmpeg development discussions and patches Le mar. 29 avr. 2025 à 19:17, Michael Niedermayer <michael@niedermayer.cc> a écrit : > > Hi Romain > > On Tue, Apr 29, 2025 at 05:42:22PM -0500, Romain Beauxis wrote: > > Le mar. 29 avr. 2025 à 16:35, Michael Niedermayer > > <michael@niedermayer.cc> a écrit : > > > > > > Hi > > > > > > On Mon, Apr 28, 2025 at 06:31:36PM -0500, Romain Beauxis wrote: > > > > --- > > > > libavformat/oggdec.c | 26 ++++++++++---------- > > > > libavformat/oggdec.h | 6 +++++ > > > > libavformat/oggparseflac.c | 28 ++++++++++++++++++++-- > > > > libavformat/oggparseopus.c | 12 ++++++++++ > > > > libavformat/oggparsevorbis.c | 12 ++++++++-- > > > > tests/ref/fate/ogg-flac-chained-meta.txt | 2 -- > > > > tests/ref/fate/ogg-opus-chained-meta.txt | 1 - > > > > tests/ref/fate/ogg-vorbis-chained-meta.txt | 3 --- > > > > 8 files changed, 68 insertions(+), 22 deletions(-) > > > > > > > > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c > > > > index 5339fdd32c..5557eb4a14 100644 > > > > --- a/libavformat/oggdec.c > > > > +++ b/libavformat/oggdec.c > > > > @@ -239,10 +239,6 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, > > > > os->start_trimming = 0; > > > > os->end_trimming = 0; > > > > > > > > - /* Chained files have extradata as a new packet */ > > > > - if (codec == &ff_opus_codec) > > > > - os->header = -1; > > > > - > > > > return i; > > > > } > > > > > > > > @@ -605,20 +601,26 @@ static int ogg_packet(AVFormatContext *s, int *sid, int *dstart, int *dsize, > > > > } else { > > > > os->pflags = 0; > > > > os->pduration = 0; > > > > + > > > > + ret = 0; > > > > if (os->codec && os->codec->packet) { > > > > if ((ret = os->codec->packet(s, idx)) < 0) { > > > > av_log(s, AV_LOG_ERROR, "Packet processing failed: %s\n", av_err2str(ret)); > > > > return ret; > > > > } > > > > } > > > > - if (sid) > > > > - *sid = idx; > > > > - if (dstart) > > > > - *dstart = os->pstart; > > > > - if (dsize) > > > > - *dsize = os->psize; > > > > - if (fpos) > > > > - *fpos = os->sync_pos; > > > > + > > > > + if (!ret) { > > > > + if (sid) > > > > + *sid = idx; > > > > + if (dstart) > > > > + *dstart = os->pstart; > > > > + if (dsize) > > > > + *dsize = os->psize; > > > > + if (fpos) > > > > + *fpos = os->sync_pos; > > > > + } > > > > + > > > > os->pstart += os->psize; > > > > os->psize = 0; > > > > if(os->pstart == os->bufpos) > > > > > > > diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h > > > > index 43df23f4cb..09f698f99a 100644 > > > > --- a/libavformat/oggdec.h > > > > +++ b/libavformat/oggdec.h > > > > @@ -38,6 +38,12 @@ struct ogg_codec { > > > > * -1 if an error occurred or for unsupported stream > > > > */ > > > > int (*header)(AVFormatContext *, int); > > > > + /** > > > > + * Attempt to process a packet as a data packet > > > > + * @return 1 if the packet was a header from a chained bitstream. > > > > + * 0 if the packet was a regular data packet. > > > > + * -1 if an error occurred or for unsupported stream > > > > + */ > > > > int (*packet)(AVFormatContext *, int); > > > > /** > > > > * Translate a granule into a timestamp. > > > > > > ok, but seems unrelated > > > > This is a new convention to allow the parser to know when to skip a > > packet. Previous to that, return value of 1 did not have specific > > meaning. > > Do you think this would merit a seperate patch ? > I mean patch #1 changing the packet() return value and clearly stating > that in the commit message > and patch #2 using the new value ? > > I think it would make things clearer Sure thing! libavformat/oggdec.c: Skip packets when packet function returns 1: https://github.com/toots/FFmpeg/commit/b78acea3b2840320bb68e065b2e712d753cb8d26 ogg/{vorbis,flac,opus}: Remove header packets from subsequent ogg streams from the demuxer output: https://github.com/toots/FFmpeg/commit/ee0b86aefb846baccdc7acc96aa3347689273949 > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The bravest are surely those who have the clearest vision > of what is before them, glory and danger alike, and yet > notwithstanding go out to meet it. -- Thucydides > _______________________________________________ > 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] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output. 2025-04-30 0:49 ` Romain Beauxis @ 2025-05-02 20:28 ` Michael Niedermayer 0 siblings, 0 replies; 9+ messages in thread From: Michael Niedermayer @ 2025-05-02 20:28 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 6076 bytes --] On Tue, Apr 29, 2025 at 07:49:54PM -0500, Romain Beauxis wrote: > Le mar. 29 avr. 2025 à 19:17, Michael Niedermayer <michael@niedermayer.cc> > a écrit : > > > > Hi Romain > > > > On Tue, Apr 29, 2025 at 05:42:22PM -0500, Romain Beauxis wrote: > > > Le mar. 29 avr. 2025 à 16:35, Michael Niedermayer > > > <michael@niedermayer.cc> a écrit : > > > > > > > > Hi > > > > > > > > On Mon, Apr 28, 2025 at 06:31:36PM -0500, Romain Beauxis wrote: > > > > > --- > > > > > libavformat/oggdec.c | 26 > ++++++++++---------- > > > > > libavformat/oggdec.h | 6 +++++ > > > > > libavformat/oggparseflac.c | 28 > ++++++++++++++++++++-- > > > > > libavformat/oggparseopus.c | 12 ++++++++++ > > > > > libavformat/oggparsevorbis.c | 12 ++++++++-- > > > > > tests/ref/fate/ogg-flac-chained-meta.txt | 2 -- > > > > > tests/ref/fate/ogg-opus-chained-meta.txt | 1 - > > > > > tests/ref/fate/ogg-vorbis-chained-meta.txt | 3 --- > > > > > 8 files changed, 68 insertions(+), 22 deletions(-) > > > > > > > > > > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c > > > > > index 5339fdd32c..5557eb4a14 100644 > > > > > --- a/libavformat/oggdec.c > > > > > +++ b/libavformat/oggdec.c > > > > > @@ -239,10 +239,6 @@ static int ogg_replace_stream(AVFormatContext > *s, uint32_t serial, char *magic, > > > > > os->start_trimming = 0; > > > > > os->end_trimming = 0; > > > > > > > > > > - /* Chained files have extradata as a new packet */ > > > > > - if (codec == &ff_opus_codec) > > > > > - os->header = -1; > > > > > - > > > > > return i; > > > > > } > > > > > > > > > > @@ -605,20 +601,26 @@ static int ogg_packet(AVFormatContext *s, int > *sid, int *dstart, int *dsize, > > > > > } else { > > > > > os->pflags = 0; > > > > > os->pduration = 0; > > > > > + > > > > > + ret = 0; > > > > > if (os->codec && os->codec->packet) { > > > > > if ((ret = os->codec->packet(s, idx)) < 0) { > > > > > av_log(s, AV_LOG_ERROR, "Packet processing failed: > %s\n", av_err2str(ret)); > > > > > return ret; > > > > > } > > > > > } > > > > > - if (sid) > > > > > - *sid = idx; > > > > > - if (dstart) > > > > > - *dstart = os->pstart; > > > > > - if (dsize) > > > > > - *dsize = os->psize; > > > > > - if (fpos) > > > > > - *fpos = os->sync_pos; > > > > > + > > > > > + if (!ret) { > > > > > + if (sid) > > > > > + *sid = idx; > > > > > + if (dstart) > > > > > + *dstart = os->pstart; > > > > > + if (dsize) > > > > > + *dsize = os->psize; > > > > > + if (fpos) > > > > > + *fpos = os->sync_pos; > > > > > + } > > > > > + > > > > > os->pstart += os->psize; > > > > > os->psize = 0; > > > > > if(os->pstart == os->bufpos) > > > > > > > > > diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h > > > > > index 43df23f4cb..09f698f99a 100644 > > > > > --- a/libavformat/oggdec.h > > > > > +++ b/libavformat/oggdec.h > > > > > @@ -38,6 +38,12 @@ struct ogg_codec { > > > > > * -1 if an error occurred or for unsupported stream > > > > > */ > > > > > int (*header)(AVFormatContext *, int); > > > > > + /** > > > > > + * Attempt to process a packet as a data packet > > > > > + * @return 1 if the packet was a header from a chained > bitstream. > > > > > + * 0 if the packet was a regular data packet. > > > > > + * -1 if an error occurred or for unsupported stream > > > > > + */ > > > > > int (*packet)(AVFormatContext *, int); > > > > > /** > > > > > * Translate a granule into a timestamp. > > > > > > > > ok, but seems unrelated > > > > > > This is a new convention to allow the parser to know when to skip a > > > packet. Previous to that, return value of 1 did not have specific > > > meaning. > > > > Do you think this would merit a seperate patch ? > > I mean patch #1 changing the packet() return value and clearly stating > > that in the commit message > > and patch #2 using the new value ? > > > > I think it would make things clearer > > Sure thing! > > libavformat/oggdec.c: Skip packets when packet function returns 1: > > https://github.com/toots/FFmpeg/commit/b78acea3b2840320bb68e065b2e712d753cb8d26 > commit 16f4a594af15aee1b68e87f17d00018592528ce6 > Author: Romain Beauxis <romain.beauxis@gmail.com> > Date: Mon Apr 28 18:08:58 2025 -0500 > > libavformat/oggdec.c: Skip packets when packet function returns 1. > ... > diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h > index 43df23f4cb9..09f698f99ab 100644 > --- a/libavformat/oggdec.h > +++ b/libavformat/oggdec.h > @@ -38,6 +38,12 @@ struct ogg_codec { > * -1 if an error occurred or for unsupported stream > */ > int (*header)(AVFormatContext *, int); > + /** > + * Attempt to process a packet as a data packet > + * @return 1 if the packet was a header from a chained bitstream. > + * 0 if the packet was a regular data packet. > + * -1 if an error occurred or for unsupported stream > + */ > int (*packet)(AVFormatContext *, int); > /** > * Translate a granule into a timestamp. The commit message and the change dont seem to fit together I would have expected the commit message to either say "Changing the packet() callback API/Return value" or "Documenting the packet() callback API/Return value" thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The smallest minority on earth is the individual. Those who deny individual rights cannot claim to be defenders of minorities. - Ayn Rand [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] [-- Attachment #2: 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] 9+ messages in thread
end of thread, other threads:[~2025-05-02 20:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-28 23:31 [FFmpeg-devel] [PATCH v2 0/2] Remove chained ogg stream header packets from demuxer Romain Beauxis
2025-04-28 23:31 ` [FFmpeg-devel] [PATCH v2 1/2] tests: Add stream dump test API util, use it to dump stream data for chained ogg/{vorbis, opus, flac} streams Romain Beauxis
2025-04-29 21:25 ` Michael Niedermayer
2025-04-28 23:31 ` [FFmpeg-devel] [PATCH v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output Romain Beauxis
2025-04-29 21:35 ` Michael Niedermayer
2025-04-29 22:42 ` Romain Beauxis
2025-04-30 0:17 ` Michael Niedermayer
2025-04-30 0:49 ` Romain Beauxis
2025-05-02 20:28 ` Michael Niedermayer
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