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
> > >
> > > 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_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..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 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;
> > > +
> > > + result = avcodec_receive_frame(ctx, fr);
> > > + if (result == AVERROR_EOF) {
> > > + result = 0;
> > > + goto end;
> > > + }
> > > +
> > > + if (result == AVERROR(EAGAIN))
> > > + continue;
> >
> > This code is not what the API docs suggest.
> >
> > avcodec_receive_frame() should be called in a loop
> >
> > there is not a guranteed 1:1 relation between packets and frames
> > Even if thats the case most of the time
> >
> > Or is this tool only supposed to work with some specific types
> > of files ? (if so this should be documented)
>
> 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
implementation
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 corner
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 = flush ? AVERROR_EOF : AVERROR(EAGAIN);
int ret = 0;
while (ret >= 0 &&
(dc->max_frames == 0 || dc->decoder->frame_num < dc->max_frames)) {
ret = avcodec_receive_frame(dc->decoder, dc->frame);
if (ret < 0) {
if (ret == AVERROR_EOF) {
int err = dc->process_frame(dc, NULL);
if (err < 0)
return err;
}
return (ret == ret_done) ? 0 : ret;
}
ret = dc->process_frame(dc, dc->frame);
av_frame_unref(dc->frame);
if (ret < 0)
return ret;
if (dc->max_frames && dc->decoder->frame_num == dc->max_frames)
return 1;
}
return (dc->max_frames == 0 || dc->decoder->frame_num < dc->max_frames) ? 0
: 1;
}
In this max_frames can be droped and maybe other things
thx
[...]
--
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.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
