[FFmpeg-devel] [PATCH] lavf/aiffenc: ID3 tags support
Paul B Mahol
onemda at gmail.com
Fri Jan 11 09:50:34 CET 2013
On 1/11/13, Matthieu Bouron <matthieu.bouron at gmail.com> wrote:
> On Thu, Jan 10, 2013 at 04:43:47PM +0100, Cl??ment B??sch wrote:
>> On Mon, Jan 07, 2013 at 09:07:17PM +0100, Matthieu Bouron wrote:
>> [...]
>> > Patch updated.
>> > I also added an option to trigger ID3 tags writting (-write_id3v2) as
>> > Paul
>> > suggested.
>> > The covert arts are handled with the same stream based logic. Since
>> > mp3enc
>> > and wtvenc use this logic, i think it's ok to stick with it until a
>> > cleaner solution is found.
>> >
>> > Regards,
>> > Matthieu
>>
>> > From a83e6cb83e7a8705e6a1d0e34151a45aaf048245 Mon Sep 17 00:00:00 2001
>> > From: Matthieu Bouron <matthieu.bouron at gmail.com>
>> > Date: Fri, 4 Jan 2013 21:19:40 +0100
>> > Subject: [PATCH] lavf/aiffenc: ID3 tags support
>> >
>> > ---
>> > libavformat/Makefile | 2 +-
>> > libavformat/aiffenc.c | 157
>> > ++++++++++++++++++++++++++++++++++++++++++++++----
>> > 2 files changed, 147 insertions(+), 12 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..41ba761 100644
>> > --- a/libavformat/aiffenc.c
>> > +++ b/libavformat/aiffenc.c
>> > @@ -20,19 +20,62 @@
>> > */
>> >
>> > #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 {
>> > + const AVClass *class;
>> > int64_t form;
>> > int64_t frames;
>> > int64_t ssnd;
>> > + int audio_stream_idx;
>> > + AVPacketList *pict_list;
>>
>> > + ID3v2EncContext id3v2;
>>
>> Can be moved locally to put_id3v2_tags()
>
> Updated in new patch.
>
>>
>> > + int write_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;
>> > +
>>
>> if (s->metadata)?
>
> I remove this check since the user choose or not the write id3 tags even if
> there is no metadata available.
>
>>
>> > + 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 +96,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 +192,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 +201,54 @@ 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;
>> > +
>> > + /* warn only once for each stream */
>> > + if (s->streams[pkt->stream_index]->nb_frames == 1) {
>> > + av_log(s, AV_LOG_WARNING, "Got more than one picture in
>> > stream %d,"
>> > + " ignoring.\n", pkt->stream_index);
>> > + }
>> > + 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 av_copy_packet fails for some reason, you leak pict_list.
>
> Updated in new patch.
>
>>
>> > + 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 +259,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 +270,46 @@ static int aiff_write_trailer(AVFormatContext *s)
>> > /* return to the end */
>> > avio_seek(pb, end_size, SEEK_SET);
>> >
>> > + /* Write ID3 tags */
>> > + if (aiff->write_id3v2)
>> > + 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 ENC AV_OPT_FLAG_ENCODING_PARAM
>> > +static const AVOption options[] = {
>> > + { "write_id3v2", "Enable ID3 tags writing.",
>> > + OFFSET(write_id3v2), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, ENC },
>> > + { "id3v2_version", "Select ID3v2 version to write. Currently 3 and
>> > 4 are supported.",
>> > + OFFSET(id3v2_version), AV_OPT_TYPE_INT, {.i64 = 4}, 3, 4, ENC },
>> > + { 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 +317,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,
>> > };
>>
>> Rest LGTM.
>>
>
> New patch attached, thanks for the review !
Use git send-mail.
Is it ok to write empty id3 chunk?
More information about the ffmpeg-devel
mailing list