[FFmpeg-devel] [PATCH] lavf/aiffenc: ID3 tags support
Paul B Mahol
onemda at gmail.com
Mon Jan 7 10:37:54 CET 2013
On 1/5/13, Matthieu Bouron <matthieu.bouron at gmail.com> wrote:
> ---
> libavformat/Makefile | 2 +-
> libavformat/aiffenc.c | 151
> ++++++++++++++++++++++++++++++++++++++++++++++----
> tests/ref/lavf/aiff | 4 +-
> 3 files changed, 142 insertions(+), 15 deletions(-)
>
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 2266dee..a60a971 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -60,7 +60,7 @@ OBJS-$(CONFIG_AEA_DEMUXER) += aea.o pcm.o
> OBJS-$(CONFIG_AFC_DEMUXER) += afc.o
> OBJS-$(CONFIG_AIFF_DEMUXER) += aiffdec.o pcm.o isom.o \
> mov_chan.o
> -OBJS-$(CONFIG_AIFF_MUXER) += aiffenc.o isom.o rawenc.o
> +OBJS-$(CONFIG_AIFF_MUXER) += aiffenc.o isom.o id3v2enc.o
> OBJS-$(CONFIG_AMR_DEMUXER) += amr.o
> OBJS-$(CONFIG_AMR_MUXER) += amr.o
> OBJS-$(CONFIG_ANM_DEMUXER) += anm.o
> diff --git a/libavformat/aiffenc.c b/libavformat/aiffenc.c
> index 525bd49..0b360bc 100644
> --- a/libavformat/aiffenc.c
> +++ b/libavformat/aiffenc.c
> @@ -20,19 +20,61 @@
> */
>
> #include "libavutil/intfloat.h"
> +#include "libavutil/opt.h"
> #include "avformat.h"
> #include "internal.h"
> #include "aiff.h"
> #include "avio_internal.h"
> #include "isom.h"
> -#include "rawenc.h"
> +#include "id3v2.h"
>
> -typedef struct {
> +typedef struct AIFFOutputContext {
> + const AVClass *class;
> int64_t form;
> int64_t frames;
> int64_t ssnd;
> + int audio_stream_idx;
> + AVPacketList *pict_list;
> + ID3v2EncContext id3v2;
> + int id3v2_version;
> } AIFFOutputContext;
>
> +static int put_id3v2_tags(AVFormatContext *s, AIFFOutputContext *aiff)
> +{
> + int ret;
> + uint64_t pos, end;
> + AVIOContext *pb = s->pb;
> + AVPacketList *pict_list = aiff->pict_list;
> +
> + if (!pb->seekable)
> + return 0;
> +
> + if (!av_dict_count(s->metadata))
> + return 0;
> +
> + avio_wl32(pb, MKTAG('I', 'D', '3', ' '));
> + avio_wb32(pb, 0);
> + pos = avio_tell(pb);
> +
> + ff_id3v2_start(&aiff->id3v2, pb, aiff->id3v2_version,
> ID3v2_DEFAULT_MAGIC);
> + ff_id3v2_write_metadata(s, &aiff->id3v2);
> + while (pict_list) {
> + if ((ret = ff_id3v2_write_apic(s, &aiff->id3v2, &pict_list->pkt)) <
> 0)
> + return ret;
> + pict_list = pict_list->next;
> + }
> + ff_id3v2_finish(&aiff->id3v2, pb);
> +
> + end = avio_tell(pb);
> +
> + /* Update chunk size */
> + avio_seek(pb, pos - 4, SEEK_SET);
> + avio_wb32(pb, end - pos);
> + avio_seek(pb, end, SEEK_SET);
> +
> + return 0;
> +}
> +
> static void put_meta(AVFormatContext *s, const char *key, uint32_t id)
> {
> AVDictionaryEntry *tag;
> @@ -53,9 +95,26 @@ static int aiff_write_header(AVFormatContext *s)
> {
> AIFFOutputContext *aiff = s->priv_data;
> AVIOContext *pb = s->pb;
> - AVCodecContext *enc = s->streams[0]->codec;
> + AVCodecContext *enc;
> uint64_t sample_rate;
> - int aifc = 0;
> + int i, aifc = 0;
> +
> + aiff->audio_stream_idx = -1;
> + for (i = 0; i < s->nb_streams; i++) {
> + AVStream *st = s->streams[i];
> + if (aiff->audio_stream_idx < 0 && st->codec->codec_type ==
> AVMEDIA_TYPE_AUDIO) {
> + aiff->audio_stream_idx = i;
> + } else if (st->codec->codec_type != AVMEDIA_TYPE_VIDEO) {
> + av_log(s, AV_LOG_ERROR, "Only audio streams and pictures are
> allowed in AIFF.\n");
> + return AVERROR(EINVAL);
> + }
> + }
> + if (aiff->audio_stream_idx < 0) {
> + av_log(s, AV_LOG_ERROR, "No audio stream present.\n");
> + return AVERROR(EINVAL);
> + }
> +
> + enc = s->streams[aiff->audio_stream_idx]->codec;
>
> /* First verify if format is ok */
> if (!enc->codec_tag)
> @@ -132,7 +191,8 @@ static int aiff_write_header(AVFormatContext *s)
> avio_wb32(pb, 0); /* Data offset */
> avio_wb32(pb, 0); /* Block-size (block align) */
>
> - avpriv_set_pts_info(s->streams[0], 64, 1,
> s->streams[0]->codec->sample_rate);
> + avpriv_set_pts_info(s->streams[aiff->audio_stream_idx], 64, 1,
> +
> s->streams[aiff->audio_stream_idx]->codec->sample_rate);
>
> /* Data is starting here */
> avio_flush(pb);
> @@ -140,11 +200,50 @@ static int aiff_write_header(AVFormatContext *s)
> return 0;
> }
>
> +static int aiff_write_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> + AIFFOutputContext *aiff = s->priv_data;
> + AVIOContext *pb = s->pb;
> + if (pkt->stream_index == aiff->audio_stream_idx)
> + avio_write(pb, pkt->data, pkt->size);
> + else {
> + int ret;
> + AVPacketList *pict_list, *last;
> +
> + if (s->streams[pkt->stream_index]->codec->codec_type !=
> AVMEDIA_TYPE_VIDEO)
> + return 0;
> +
> + if (s->streams[pkt->stream_index]->nb_frames >= 1)
> + return 0;
> +
> + pict_list = av_mallocz(sizeof(AVPacketList));
> + if (!pict_list)
> + return AVERROR(ENOMEM);
> +
> + if ((ret = av_copy_packet(&pict_list->pkt, pkt)) < 0)
> + return ret;
> +
> + if (!aiff->pict_list)
> + aiff->pict_list = pict_list;
> + else {
> + last = aiff->pict_list;
> + while (last->next)
> + last = last->next;
> + last->next = pict_list;
> + }
> + }
> +
> + return 0;
> +}
> +
> +
> static int aiff_write_trailer(AVFormatContext *s)
> {
> + int ret;
> AVIOContext *pb = s->pb;
> AIFFOutputContext *aiff = s->priv_data;
> - AVCodecContext *enc = s->streams[0]->codec;
> + AVPacketList *pict_list = aiff->pict_list;
> + AVCodecContext *enc = s->streams[aiff->audio_stream_idx]->codec;
>
> /* Chunks sizes must be even */
> int64_t file_size, end_size;
> @@ -155,10 +254,6 @@ static int aiff_write_trailer(AVFormatContext *s)
> }
>
> if (s->pb->seekable) {
> - /* File length */
> - avio_seek(pb, aiff->form, SEEK_SET);
> - avio_wb32(pb, file_size - aiff->form - 4);
> -
> /* Number of sample frames */
> avio_seek(pb, aiff->frames, SEEK_SET);
> avio_wb32(pb, (file_size-aiff->ssnd-12)/enc->block_align);
> @@ -170,12 +265,43 @@ static int aiff_write_trailer(AVFormatContext *s)
> /* return to the end */
> avio_seek(pb, end_size, SEEK_SET);
>
> + /* Write ID3 tags */
> + if ((ret = put_id3v2_tags(s, aiff)) < 0)
> + return ret;
> +
> + /* File length */
> + file_size = avio_tell(pb);
> + avio_seek(pb, aiff->form, SEEK_SET);
> + avio_wb32(pb, file_size - aiff->form - 4);
> +
> avio_flush(pb);
> }
>
> + while (pict_list) {
> + AVPacketList *next = pict_list->next;
> + av_free_packet(&pict_list->pkt);
> + av_freep(&pict_list);
> + pict_list = next;
> + }
> +
> return 0;
> }
>
> +#define OFFSET(x) offsetof(AIFFOutputContext, x)
> +#define DEC AV_OPT_FLAG_DECODING_PARAM
> +static const AVOption options[] = {
> + { "id3v2_version", "Select ID3v2 version to write. Currently 3 and 4
> are supported.",
> + OFFSET(id3v2_version), AV_OPT_TYPE_INT, {.i64 = 4}, 3, 4, DEC },
> + { NULL },
> +};
> +
> +static const AVClass aiff_muxer_class = {
> + .class_name = "AIFF muxer",
> + .item_name = av_default_item_name,
> + .option = options,
> + .version = LIBAVUTIL_VERSION_INT,
> +};
> +
> AVOutputFormat ff_aiff_muxer = {
> .name = "aiff",
> .long_name = NULL_IF_CONFIG_SMALL("Audio IFF"),
> @@ -183,9 +309,10 @@ AVOutputFormat ff_aiff_muxer = {
> .extensions = "aif,aiff,afc,aifc",
> .priv_data_size = sizeof(AIFFOutputContext),
> .audio_codec = AV_CODEC_ID_PCM_S16BE,
> - .video_codec = AV_CODEC_ID_NONE,
> + .video_codec = AV_CODEC_ID_PNG,
> .write_header = aiff_write_header,
> - .write_packet = ff_raw_write_packet,
> + .write_packet = aiff_write_packet,
> .write_trailer = aiff_write_trailer,
> .codec_tag = (const AVCodecTag* const []){ ff_codec_aiff_tags,
> 0 },
> + .priv_class = &aiff_muxer_class,
> };
> diff --git a/tests/ref/lavf/aiff b/tests/ref/lavf/aiff
> index 2a5cd81..8b8c489 100644
> --- a/tests/ref/lavf/aiff
> +++ b/tests/ref/lavf/aiff
> @@ -1,3 +1,3 @@
> -b0d42747a6fc99a5cd1ab0e861671f3a *./tests/data/lavf/lavf.aif
> -90182 ./tests/data/lavf/lavf.aif
> +731d863ff976966827a166fa30b32400 *./tests/data/lavf/lavf.aif
> +90220 ./tests/data/lavf/lavf.aif
> ./tests/data/lavf/lavf.aif CRC=0xf1ae5536
> --
> 1.8.1
IMHO id3 tags should not be written by default, instead only when requested.
The code that adds cover art is ugly and bad, does it works/have been
tested at all?
I really dislike its current implementation, by abusing write_packet
to store picture in metadata. So I would prefer to not mux cover arts
until cleaner solution is available.
More information about the ffmpeg-devel
mailing list