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

[asivery via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

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

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

On Friday, March 8th, 2024 at 1:06 PM, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:

> asivery via ffmpeg-devel:
> 
> > +typedef struct {
> > + int block_count;
> > +} AeaMuxerContext;
> > +
> > +static int aea_init(AVFormatContext *s)
> > +{
> > + AeaMuxerContext *c = s->priv_data;
> > + c->block_count = 0;
> 
> 
> Unnecessary: A muxer's priv_data is always pre-zeroed.
> 
> > +
> > + return 0;
> > +}
> > +
> > +static int aea_write_header(AVFormatContext *s)
> > +{
> > + AVDictionaryEntry *title_entry;
> 
> 
> const
> 
> > + int title_length = 0, i;
> > + char* title_contents;
> > + AVStream *st;
> > +
> > +
> > + if (s->nb_streams > 1) {
> > + av_log(s, AV_LOG_WARNING, "Got more than one stream to encode, they will be ignored.\n");
> 
> 
> We don't ignore extraneous streams, we normally error out if something
> like this happens.
> 
> > + }
> > +
> > + st = s->streams[0];
> > + 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);
> > + 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 = FFMIN(256, strlen(title_contents));
> 
> 
> Don't use strlen() in FFMIN (like many macros, it can evaluate its
> argument multiple times).
> 
> > + }
> > +
> > + if (title_length) {
> > + avio_write(s->pb, title_contents, title_length);
> > + }
> 
> 
> No need for this branch, just call this inside the "if (title_entry)"
> block above.
> 
> > +
> > + for (i = 0; i<(256 - title_length); i++) {
> > + avio_w8(s->pb, 0);
> > + }
> 
> 
> ffio_fill(). Same below.
> 
> > +
> > + /* 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);
> > +
> > + / Write flags (meaning unknown) /
> > + for(i = 0; i<11; i++) {
> > + avio_wl32(s->pb, 0);
> > + }
> > +
> > + / Pad the header to 2048 bytes */
> > + for(i = 0; i<1738; i++) avio_w8(s->pb, 0);
> > +
> > + return 0;
> > +}
> > +
> > +static int aea_write_packet(struct AVFormatContext *s, AVPacket *pkt)
> > +{
> > + AeaMuxerContext *c = s->priv_data;
> > + c->block_count++;
> > +
> > + avio_write(s->pb, pkt->data, pkt->size);
> > + return 0;
> 
> 
> You can avoid this and the whole muxer context by using
> ff_raw_write_packet() and relying on AVStream.nb_frames.
> 
> > +}
> > +
> > +static int aea_write_trailer(struct AVFormatContext *s)
> > +{
> > + AVIOContext *pb = s->pb;
> > + AeaMuxerContext c = s->priv_data;
> > + if (pb->seekable & AVIO_SEEKABLE_NORMAL) {
> > + / Seek to rewrite the block count. */
> > + avio_seek(pb, 260, SEEK_SET);
> > + avio_wl32(pb, c->block_count * s->streams[0]->codecpar->ch_layout.nb_channels);
> > + } else {
> > + av_log(s, AV_LOG_WARNING, "unable to rewrite AEA header.\n");
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +const FFOutputFormat ff_aea_muxer = {
> > + .p.name = "aea",
> > + .p.long_name = NULL_IF_CONFIG_SMALL("MD STUDIO audio"),
> > + .priv_data_size = sizeof(AeaMuxerContext),
> > + .p.extensions = "aea",
> > + .p.audio_codec = AV_CODEC_ID_ATRAC1,
> > + .init = aea_init,
> > +
> > + .write_header = aea_write_header,
> > + .write_packet = aea_write_packet,
> > + .write_trailer = aea_write_trailer,
> > + .p.flags = AVFMT_GENERIC_INDEX,
> 
> 
> Makes no sense for a muxer.
> 
> > +};
> 
> 
> _______________________________________________
> 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".

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-avformat-aea-Add-aea-muxer.patch --]
[-- Type: text/x-patch; name=0001-avformat-aea-Add-aea-muxer.patch, Size: 5622 bytes --]

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 +
 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");
+        return AVERROR(EINVAL);
+    }
+    
+    st = s->streams[0];
+    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);
+        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");
+    }
+
+    return 0;
+}
+
+const FFOutputFormat ff_aea_muxer = {
+    .p.name           = "aea",
+    .p.long_name      = NULL_IF_CONFIG_SMALL("MD STUDIO audio"),
+    .p.extensions     = "aea",
+    .p.audio_codec    = AV_CODEC_ID_ATRAC1,
+
+    .write_header     = aea_write_header,
+    .write_packet     = ff_raw_write_packet,
+    .write_trailer    = aea_write_trailer,
+};
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index 0a0e76138f..5639715104 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -47,6 +47,7 @@ extern const FFOutputFormat ff_adts_muxer;
 extern const FFInputFormat  ff_adx_demuxer;
 extern const FFOutputFormat ff_adx_muxer;
 extern const FFInputFormat  ff_aea_demuxer;
+extern const FFOutputFormat ff_aea_muxer;
 extern const FFInputFormat  ff_afc_demuxer;
 extern const FFInputFormat  ff_aiff_demuxer;
 extern const FFOutputFormat ff_aiff_muxer;
-- 
2.34.1


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