[FFmpeg-devel] LRC (music lyrics) muxer and demuxer

Aurelien Jacobs aurel
Sun Dec 26 22:00:59 CET 2010


On Sun, Dec 26, 2010 at 11:45:50PM +1100, Peter Ross wrote:
> Typical sample: http://lrc.awardspace.us/lrc/Evanescence-Missing.lrc

First some general comments.

Using CODEC_ID_TEXT don't seem appropriate.
We need to be able to decode the "Simple format extended" (gender
specifier) and the "Enhanced format" (Word Time Tag).
See : http://en.wikipedia.org/wiki/LRC_%28file_format%29
So for this to work, we need a specific CODEC_ID_LRC (and we will need
to implement an encoder and a decoder, but that's another story).

It seems you also need to support multiple timestamps on a single line
(allows to avoid duplicating the same text several times).
See : http://www.mobile-mir.com/en/HowToLRC.php
And for a real world sample file, see:
http://www.lyrdb.com/karaoke/18374.htm

Also, I wonder how we will generate AVSubtitle.end_display_time in the
decoder ? In LRC, the start time of the next frame is the end time of
current frame. The decoder could wait for the next frame before
returning current frame, adding a 1 frame delay, but with subtitles, the
time between two frames can be really long and the video would continue
to decode while the subtitle is waiting for next frame to appear to know
the end time of the frame which is supposed to be displayed now. So I
guess this wouldn't really work.
Any idea about how we could handle this ?

Now about the code itself.

> [...]
> +const AVMetadataConv ff_lrc_metadata_conv[] = {
> +    { "al", "album"      },
> +    { "ar", "artist"     },
> +    { "au", "composer"   },
> +    { "by", "encoded_by" },
> +    { "la", "language"   },
> +    { "re", "encoder"    },
> +    { "ti", "title"      },
> +    { 0 },
> +};

You may want to add:
 +    { "ve", "encoder_version" },

> +static int probe(AVProbeData *p)
> +{
> +    const AVMetadataConv * c;
> +    if (p->buf[0] != '[')
> +        return 0;
> +    for (c = ff_lrc_metadata_conv; c->native; c++) {
> +        int len = strlen(c->native);
> +        if (p->buf_size >= len + 2 && !memcmp(p->buf + 1, c->native, len) &&
> +            p->buf[len + 1]==':')
> +            return AVPROBE_SCORE_MAX/2 + 1;
> +    }
> +    return 0;
> +}

This probe function is a bit week. Maybe you could test that the line
correctly ends with a ']'. And you could also check that the second line
is also correct.
Also this autodetect fonction won't work for files with no metadata.

> +static int read_header(AVFormatContext *s, AVFormatParameters *ap)
> +{
> +    AVStream *st = av_new_stream(s, 0);
> +    if (!st)
> +        return -1;
> +    st->codec->codec_type = AVMEDIA_TYPE_SUBTITLE;
> +    st->codec->codec_id   = CODEC_ID_TEXT;
> +    st->start_time        = 0;
> +    av_set_pts_info(st, 64, 1, 100);

> +    ff_metadata_conv_ctx(s, ff_lrc_metadata_conv, NULL);

What is this ff_metadata_conv_ctx() good for ?

> [...]
> +            split = strchr(start, ':');
> +            if (!split)
> +                 continue;
> +            end = strchr(split, ']');
> +            if (!end)
> +                 continue;

Could be simplified this way (at least I find it simpler):

+            if (!(split = strchr(start, ':')) ||
+                !(end   = strchr(split, ']')))
+                 continue;

> [...]
> +            for (c = ff_lrc_metadata_conv; c->native; c++) {
> +                int len = split - start;
> +                if (strlen(c->native) == len && !memcmp(start, c->native, len))
> +                    av_metadata_set2(&s->metadata, c->native, split + 1, 0);
> +            }

I might simplify your code if you use ff_metadata_conv() ?

> [...]
> +
> +    end = strchr(start, '\n');
> +    av_new_packet(pkt, end ? end - start : strlen(start));

It seems you drop the final '\n' (but not the '\r'). Why ?
Other subtitles demuxers are outputing \r\n as part of the packet.

> [...]
> +AVInputFormat lrc_demuxer = {
> +    .name           = "lrc",

> +    .long_name      = NULL_IF_CONFIG_SMALL("LRC"),

Not a very descriptive long name...

> +static int write_header(AVFormatContext *s)
> +{
> +    AVMetadataTag *t = NULL;
> +    if (s->nb_streams != 1)
> +        return -1;

Here you should also check that codec_id is the one supported.

> [...]
> +
> +AVOutputFormat lrc_muxer = {
> +    .name           = "lrc",
> +    .long_name      = NULL_IF_CONFIG_SMALL("LRC"),
> +    .extensions     = "lrc",
> +    .subtitle_codec = CODEC_ID_TEXT,
> +    .write_header   = write_header,
> +    .write_packet   = write_packet,

> +    .flags          = AVFMT_GLOBALHEADER,

Is this really needed/useful/appropriate ?
This flag supposedly mean that the muxer expect some extradata to be
available, but I don't think it tells anything about metadata.

Aurel



More information about the ffmpeg-devel mailing list