Re: [FFmpeg-devel] [PATCH] avformat/aea: Add aea muxer

[Stefano Sabatini <stefasab@gmail.com>]

On date Friday 2024-03-08 13:50:59 +0000, ffmpeg-devel Mailing List wrote:
> Thank you for your incredibly thorough and fast response. I've applied all the corrections you requested. Please let me know if there's anything else wrong with my patch, and thank you for your time.
> 
> - asivery

[...]

> From d2163e7d943a5ba53148aa73a813a28d346124a4 Mon Sep 17 00:00:00 2001
> From: asivery <asivery@protonmail.com>
> Date: Fri, 8 Mar 2024 14:45:02 +0100
> Subject: [PATCH] avformat/aea: Add aea muxer
> 
> Signed-off-by: asivery <asivery@protonmail.com>
> ---
>  libavformat/Makefile            |   3 +-
>  libavformat/{aea.c => aeadec.c} |   0
>  libavformat/aeaenc.c            | 103 ++++++++++++++++++++++++++++++++
>  libavformat/allformats.c        |   1 +

Please add some information in doc/muxers.texi, even if just a short
notice about where this format is expected to be used or a quick
historical intro. Also you might mention that the title metadata is
used to fill the container title.

>  4 files changed, 106 insertions(+), 1 deletion(-)
>  rename libavformat/{aea.c => aeadec.c} (100%)
>  create mode 100644 libavformat/aeaenc.c
> 
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 8811a0ffc9..70d56f391f 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -91,7 +91,8 @@ OBJS-$(CONFIG_ADTS_MUXER)                += adtsenc.o apetag.o img2.o \
>                                              id3v2enc.o
>  OBJS-$(CONFIG_ADX_DEMUXER)               += adxdec.o
>  OBJS-$(CONFIG_ADX_MUXER)                 += rawenc.o
> -OBJS-$(CONFIG_AEA_DEMUXER)               += aea.o pcm.o
> +OBJS-$(CONFIG_AEA_DEMUXER)               += aeadec.o pcm.o
> +OBJS-$(CONFIG_AEA_MUXER)                 += aeaenc.o rawenc.o
>  OBJS-$(CONFIG_AFC_DEMUXER)               += afc.o
>  OBJS-$(CONFIG_AIFF_DEMUXER)              += aiffdec.o aiff.o pcm.o \
>                                              mov_chan.o replaygain.o
> diff --git a/libavformat/aea.c b/libavformat/aeadec.c
> similarity index 100%
> rename from libavformat/aea.c
> rename to libavformat/aeadec.c
> diff --git a/libavformat/aeaenc.c b/libavformat/aeaenc.c
> new file mode 100644
> index 0000000000..e01fabcace
> --- /dev/null
> +++ b/libavformat/aeaenc.c
> @@ -0,0 +1,103 @@
> +/*
> + * MD STUDIO audio muxer
> + *
> + * Copyright (c) 2024 asivery
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "libavutil/channel_layout.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/avstring.h"
> +#include "avformat.h"
> +#include "avio_internal.h"
> +#include "rawenc.h"
> +#include "mux.h"
> +
> +static int aea_write_header(AVFormatContext *s)
> +{
> +    const AVDictionaryEntry *title_entry;
> +    int title_length = 0;
> +    char* title_contents;
> +    AVStream *st;
> +
> +    if (s->nb_streams > 1) {

> +        av_log(s, AV_LOG_WARNING, "Got more than one stream to encode. This is not supported.\n");

ERROR

> +        return AVERROR(EINVAL);
> +    }
> +    

> +    st = s->streams[0];

do you also need to check the stream type (probably this is handled by
the librarye already)?

> +    if (st->codecpar->ch_layout.nb_channels != 1 && st->codecpar->ch_layout.nb_channels != 2) {

> +        av_log(s, AV_LOG_ERROR, "Invalid amount of channels to mux (%d).\n", st->codecpar->ch_layout.nb_channels);

this can be made more explicit to aid users:
Only maximum 2 channels are supported in the audio stream, %d channels were found

> +        return AVERROR(EINVAL);
> +    }
> +
> +    if (st->codecpar->sample_rate != 44100) {
> +        av_log(s, AV_LOG_ERROR, "Invalid sample rate (%d) AEA only supports 44.1kHz.\n", st->codecpar->sample_rate);
> +        return AVERROR(EINVAL);
> +    }
> +
> +    /* Write magic */
> +    avio_wl32(s->pb, 2048);
> +    
> +    /* Write AEA title */
> +    title_entry = av_dict_get(st->metadata, "title", NULL, 0);
> +    if (title_entry) {
> +        title_contents = title_entry->value;
> +        title_length = strlen(title_contents);
> +        title_length = FFMIN(256, title_length);
> +        avio_write(s->pb, title_contents, title_length);
> +    }
> +
> +    ffio_fill(s->pb, 0, 256 - title_length);
> +
> +    /* Write number of frames (zero at header-writing time, will seek later), number of channels */
> +    avio_wl32(s->pb, 0);
> +    avio_w8(s->pb, st->codecpar->ch_layout.nb_channels);
> +    avio_w8(s->pb, 0);
> +
> +    /* Pad the header to 2048 bytes */
> +    ffio_fill(s->pb, 0, 1782);
> +
> +    return 0;
> +}
> +
> +static int aea_write_trailer(struct AVFormatContext *s)
> +{
> +    AVIOContext *pb = s->pb;
> +    AVStream *st = s->streams[0];
> +    if (pb->seekable & AVIO_SEEKABLE_NORMAL) {
> +        /* Seek to rewrite the block count. */
> +        avio_seek(pb, 260, SEEK_SET);
> +        avio_wl32(pb, st->nb_frames * st->codecpar->ch_layout.nb_channels);
> +    } else {

> +        av_log(s, AV_LOG_WARNING, "unable to rewrite AEA header.\n");

nit: Unable
_______________________________________________
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".