From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <ffmpeg-devel-bounces@ffmpeg.org> Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 987C04DF1D for <ffmpegdev@gitmailbox.com>; Sat, 26 Apr 2025 11:52:57 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id EAFEA68AF22; Sat, 26 Apr 2025 14:52:51 +0300 (EEST) Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 39BCA687D5A for <ffmpeg-devel@ffmpeg.org>; Sat, 26 Apr 2025 14:52:45 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 6199843212 for <ffmpeg-devel@ffmpeg.org>; Sat, 26 Apr 2025 11:52:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1745668364; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=sa0oQa36mUGJHfJsm7fVhmWoT7dKtBFDOrY7fhjTJ7Y=; b=DdM1zWZHfzqmsw75ZMDJBjxNEDjBkR9ap0cSLbbBbOO+5u62mGnfJFelXISKs7Tv8joTv6 m/jveP5JV735qXSC7kmcdmSUOlrktOJMr/q/X1uxQfoueZnJjkGMUtS5jG/UonlXEEk03n bB5NsK4Tyqgh4szCuJ/v/9DrlbDRpviZXs4PG/US/oITQuMlHmAvGp5CfmzbU5MElXZaVU nPtJdCBne7niTn4E5vXy6js250L9pEo55+Ug1mdE0jea/rXe4eJOOPnvFYiirKO67bxENB jiVFwbg+KeUjYPWwU0sX1FP4unv+yWzJYc2Sti+fu/tgPOfsdoktQIrYo4ibfA== Date: Sat, 26 Apr 2025 13:52:43 +0200 From: Michael Niedermayer <michael@niedermayer.cc> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Message-ID: <20250426115243.GE4991@pb2> References: <20250422214408.10102-1-romain.beauxis@gmail.com> <20250422214408.10102-2-romain.beauxis@gmail.com> <20250426031123.GC4991@pb2> <f6e13b29-2fe4-48f0-9235-684c794a45ca@lynne.ee> MIME-Version: 1.0 In-Reply-To: <f6e13b29-2fe4-48f0-9235-684c794a45ca@lynne.ee> X-GND-State: clean X-GND-Score: -70 X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvheehuddvucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuifetpfffkfdpucggtfgfnhhsuhgsshgtrhhisggvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenfghrlhcuvffnffculdeftddmnecujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefoihgthhgrvghlucfpihgvuggvrhhmrgihvghruceomhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtqeenucggtffrrghtthgvrhhnpeeigeektdejudffjefhteegjedtgeettefggedthfejgfevhfetgeekjedtvdfhveenucfkphepgedurdeiiedrieejrdduudefnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepgedurdeiiedrieejrdduudefpdhhvghloheplhhotggrlhhhohhsthdpmhgrihhlfhhrohhmpehmihgthhgrvghlsehnihgvuggvrhhmrgihvghrrdgttgdpnhgspghrtghpthhtohepuddprhgtphhtthhopehffhhmphgvghdquggvvhgvlhesfhhfmhhpvghgrdhorhhg X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH 1/2] tests: Add stream dump test API util, use it to dump stream data for chained ogg/{vorbis, opus, flac} streams. X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches <ffmpeg-devel.ffmpeg.org> List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>, <mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe> List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel> List-Post: <mailto:ffmpeg-devel@ffmpeg.org> List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help> List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>, <mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe> Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Content-Type: multipart/mixed; boundary="===============0499712204868710091==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org> Archived-At: <https://master.gitmailbox.com/ffmpegdev/20250426115243.GE4991@pb2/> List-Archive: <https://master.gitmailbox.com/ffmpegdev/> List-Post: <mailto:ffmpegdev@gitmailbox.com> --===============0499712204868710091== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="xANaT5/XbCzG7bSN" Content-Disposition: inline --xANaT5/XbCzG7bSN Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi On Sat, Apr 26, 2025 at 12:45:52PM +0200, Lynne wrote: > On 26/04/2025 05:11, Michael Niedermayer wrote: > > On Tue, Apr 22, 2025 at 04:44:07PM -0500, Romain Beauxis wrote: > > > --- > > > tests/Makefile | 4 + > > > tests/api/Makefile | 2 +- > > > tests/api/api-dump-stream-meta-test.c | 177 ++++++++++++++++++= +++ > > > 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, 271 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 > > >=20 > > > diff --git a/tests/Makefile b/tests/Makefile > > > index f9f5fc07f3..75b9bcc729 100644 > > > --- a/tests/Makefile > > > +++ b/tests/Makefile > > > @@ -219,6 +219,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 > > > @@ -277,6 +280,7 @@ $(FATE_FFPROBE) $(FATE_FFMPEG_FFPROBE) $(FATE_SAM= PLES_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-tes= t$(EXESUF) > > > ifdef SAMPLES > > > FATE +=3D $(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) +=3D api-flac > > > APITESTPROGS-$(call DEMDEC, H264, H264) +=3D api-h264 > > > APITESTPROGS-$(call DEMDEC, H264, H264) +=3D api-h264-slice > > > -APITESTPROGS-yes +=3D api-seek > > > +APITESTPROGS-yes +=3D api-seek api-dump-stream-meta > > > APITESTPROGS-$(call DEMDEC, H263, H263) +=3D api-band > > > APITESTPROGS-$(HAVE_THREADS) +=3D api-threadmessage > > > APITESTPROGS +=3D $(APITESTPROGS-yes) > > > diff --git a/tests/api/api-dump-stream-meta-test.c b/tests/api/api-du= mp-stream-meta-test.c > > > new file mode 100644 > > > index 0000000000..bbfbd1f30b > > > --- /dev/null > > > +++ b/tests/api/api-dump-stream-meta-test.c > > > @@ -0,0 +1,177 @@ > > > +/* > > > + * Copyright (c) 2025 Romain Beauxis > > > + * > > > + * Permission is hereby granted, free of charge, to any person obtai= ning a copy > > > + * of this software and associated documentation files (the "Softwar= e"), 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 Softwar= e is > > > + * furnished to do so, subject to the following conditions: > > > + * > > > + * The above copyright notice and this permission notice shall be in= cluded in > > > + * all copies or substantial portions of the Software. > > > + * > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, E= XPRESS OR > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTA= BILITY, > > > + * 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, A= RISING FROM, > > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEA= LINGS 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 =3D NULL; > > > + AVPacket *pkt =3D NULL; > > > + AVFrame *fr =3D NULL; > > > + AVFormatContext *fmt_ctx =3D NULL; > > > + AVCodecContext *ctx =3D NULL; > > > + AVCodecParameters *origin_par =3D NULL; > > > + AVStream *st; > > > + int stream_idx =3D 0; > > > + int result; > > > + char *metadata; > > > + > > > + result =3D avformat_open_input(&fmt_ctx, input_filename, NULL, N= ULL); > > > + if (result < 0) { > > > + av_log(NULL, AV_LOG_ERROR, "Can't open file\n"); > > > + return result; > > > + } > > > + > > > + result =3D 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 in= put!\n"); > > > + goto end; > > > + } > > > + > > > + origin_par =3D fmt_ctx->streams[stream_idx]->codecpar; > > > + st =3D fmt_ctx->streams[stream_idx]; > > > + > > > + result =3D av_dict_get_string(st->metadata, &metadata, '=3D', ':= '); > > > + if (result < 0) > > > + goto end; > > > + > > > + printf("Stream ID: %d, codec name: %s, metadata: %s\n", stream_i= dx, > > > + avcodec_get_name(origin_par->codec_id), > > > + strlen(metadata) ? metadata : "N/A"); > > > + > > > + codec =3D avcodec_find_decoder(origin_par->codec_id); > > > + if (!codec) { > > > + av_log(NULL, AV_LOG_ERROR, "Can't find decoder\n"); > > > + result =3D AVERROR_DECODER_NOT_FOUND; > > > + goto end; > > > + } > > > + > > > + ctx =3D avcodec_alloc_context3(codec); > > > + if (!ctx) { > > > + av_log(NULL, AV_LOG_ERROR, "Can't allocate decoder context\n= "); > > > + result =3D AVERROR(ENOMEM); > > > + goto end; > > > + } > > > + > > > + result =3D avcodec_parameters_to_context(ctx, origin_par); > > > + if (result) { > > > + av_log(NULL, AV_LOG_ERROR, "Can't copy decoder context\n"); > > > + goto end; > > > + } > > > + > > > + result =3D avcodec_open2(ctx, codec, NULL); > > > + if (result < 0) { > > > + av_log(ctx, AV_LOG_ERROR, "Can't open decoder\n"); > > > + goto end; > > > + } > > > + > > > + pkt =3D av_packet_alloc(); > > > + if (!pkt) { > > > + av_log(NULL, AV_LOG_ERROR, "Cannot allocate packet\n"); > > > + result =3D AVERROR(ENOMEM); > > > + goto end; > > > + } > > > + > > > + fr =3D av_frame_alloc(); > > > + if (!fr) { > > > + av_log(NULL, AV_LOG_ERROR, "Can't allocate frame\n"); > > > + result =3D AVERROR(ENOMEM); > > > + goto end; > > > + } > > > + > > > + for (;;) { > > > + result =3D av_read_frame(fmt_ctx, pkt); > > > + if (result) > > > + goto end; > > > + > > > + if (pkt->stream_index !=3D 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 =3D av_dict_get_string(st->metadata, &metadata, '= =3D', ':'); > > > + if (result < 0) > > > + goto end; > > > + > > > + printf("Stream ID: %d, new metadata: %s\n", pkt->stream_= index, > > > + strlen(metadata) ? metadata : "N/A"); > > > + > > > + st->event_flags &=3D ~AVSTREAM_EVENT_FLAG_METADATA_UPDAT= ED; > > > + } > > > + > >=20 > > > + result =3D avcodec_send_packet(ctx, pkt); > > > + av_packet_unref(pkt); > > > + > > > + if (result < 0) > > > + goto end; > > > + > > > + result =3D avcodec_receive_frame(ctx, fr); > > > + if (result =3D=3D AVERROR_EOF) { > > > + result =3D 0; > > > + goto end; > > > + } > > > + > > > + if (result =3D=3D AVERROR(EAGAIN)) > > > + continue; > >=20 > > This code is not what the API docs suggest. > >=20 > > avcodec_receive_frame() should be called in a loop > >=20 > > there is not a guranteed 1:1 relation between packets and frames > > Even if thats the case most of the time > >=20 > > Or is this tool only supposed to work with some specific types > > of files ? (if so this should be documented) >=20 > I think for a test it's fine. For just a test, i agree but the following patches will add new API and functionality and starting out with a test thats "wrong"/incomplete then results in imple= mentation after implementation being actually wrong. I mean for ogg we may have a 1:1 relation between packets and frames (i am = not sure) but in general thats not true and our API is not designed that way also pts are not guranteed to be unique or even available at all, its not possible to use pts to bypass the API all these are "hacks", its not a correct implementation that fails in corne= r cases Worse yet, the very test for the new functionalty fails in the same places the implementations dont handle. For the test, fixing it should be very easy, just look at tools/decode_simple.c copy and paste the code surrounding avcodec_receive_frame() and avcodec_send_packet() and adapt as needed (or factorize) The code in decode_simple: static int decode_read(DecodeContext *dc, int flush) { const int ret_done =3D flush ? AVERROR_EOF : AVERROR(EAGAIN); int ret =3D 0; while (ret >=3D 0 && (dc->max_frames =3D=3D 0 || dc->decoder->frame_num < dc->max_fra= mes)) { ret =3D avcodec_receive_frame(dc->decoder, dc->frame); if (ret < 0) { if (ret =3D=3D AVERROR_EOF) { int err =3D dc->process_frame(dc, NULL); if (err < 0) return err; } return (ret =3D=3D ret_done) ? 0 : ret; } ret =3D dc->process_frame(dc, dc->frame); av_frame_unref(dc->frame); if (ret < 0) return ret; if (dc->max_frames && dc->decoder->frame_num =3D=3D dc->max_frames) return 1; } return (dc->max_frames =3D=3D 0 || dc->decoder->frame_num < dc->max_fra= mes) ? 0 : 1; } In this max_frames can be droped and maybe other things thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you fake or manipulate statistics in a paper in physics you will never get a job again. If you fake or manipulate statistics in a paper in medicin you will get a job for life at the pharma industry. --xANaT5/XbCzG7bSN Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCaAzJAQAKCRBhHseHBAsP q4m4AJ0WGeJaMqfvG9RG0zaBqRIZahU0YACaA014loPDI3l4bAhq0llYHAF5qSM= =WY2q -----END PGP SIGNATURE----- --xANaT5/XbCzG7bSN-- --===============0499712204868710091== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============0499712204868710091==--