Re: [FFmpeg-devel] [TC] "Future Log Output Default"

["softworkz ." <softworkz-at-hotmail.com@ffmpeg.org>]

[-- Attachment #1: Type: text/plain, Size: 2270 bytes --]



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Michael Niedermayer
> Sent: Samstag, 26. April 2025 17:10
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: [FFmpeg-devel] [TC] "Future Log Output Default"
> 
> Hi all
> 
> This is just an announcement that the TC has been asked to look into
>     avutil/log: Add log flag to control printing of memory addresses
>     GitHub:    https://github.com/ffstaging/FFmpeg/pull/59
>     Patchwork:
> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=14094
>     ...
> 
> and the disagreement between people about it.
> 
> So far, Niklas, Martin and myself have commented, there have been no
> formal
> decissions and no votes, we just since yesterday send some comments.
> 
> From these to me it seems the TC members who spoke so far seem to
> agree
> that the addresses in the log are "mostly noise".
> 
> I wonder a bit if the TC discussions should/could be more public but
> thats
> a subject for a different thread. Myself summarizing will surely miss
> some
> fine points but iam trying anyway.
> 
> Niklas also brought up that item_name() is used in filters to
> disambiguate
> instances and that we maybe could add a const char *name and have
> fftools
> genrate a unique ID for each
> 
> Either way none of this is a TC decission or binding its just a
> partial
> summary of the things that TC members said
> 
> thx
> 
> --
> Michael     GnuPG fingerprint:



Thanks Michael,
hello everybody,

I would like to note that my reason for handing this over to the TC was not primarily about conflict resolution, it's rather that I feel that this decision is at least one size too big for me standing right in the front line.

Ideally, I think, it would be great when it would be backed by a majority, but I wouldn't know how to determine that other than a vote. The TC in turn has already been elected for making those decisions and so that seems to be the most natural way. I'm fine with any outcome, also open to do once another variant of implementation if that would be desired, or just keep the default to current behavior if that's the verdict.

Thanks
sw



[-- Attachment #2: Type: message/rfc822, Size: 19229 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 8675 bytes --]

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)

thx

[...]
--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott


[-- Attachment #2.1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 201 bytes --]

[-- Attachment #2.1.3: ATT00001.txt --]
[-- Type: text/plain, Size: 258 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".

[-- 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".