[FFmpeg-devel] [PATCH] libgme support

[Stefano Sabatini]

On date Tuesday 2013-07-02 01:34:48 +0200, wm4 encoded:
> On Mon, 1 Jul 2013 10:33:02 +0200
> Stefano Sabatini <stefasab at gmail.com> wrote:
> 
> > > I would prefer to add the tracks as streams, but since you can play
> > > only one track at a time, this is perhaps not possible. I wonder if
> > > there is some way to change the current track at runtime? The GME
> > > API allows it.
> > 
> > I don't think so. You could check the track option at each iteration,
> > and change stream if the option was set to a new value. Not very clean
> > but at the moment we lack a different system. A possibility would be
> > to add a command API (like the one in libavfilter).
> 
> It would also be possible to keep a Music_Emu instance for each track,
> but that would require loading the file multiple times.
> 
> Yes, a command API would be nice. Polling the option could also work.
> But I think the required logic for this is a bit too fragile and
> annoying (e.g. what happens if setting the new track fails), so I'll
> stay away from it for now.
> 
> > > +static void add_meta(AVFormatContext *s, const char *name, const
> > > char *value)
> > 
> > nit: you could inline this
> 
> I'd prefer not to, because it gets a bit repetitious. But if you think
> it should be changed, I can do that.
> 
> > > +    int n_tracks = gme_track_count(gme->music_emu);
> > 
> > nit++: int track_count for consistency
> 
> OK. I also decided to remove the n_tracks > 1 check before adding it
> as meta data; this seemed silly.
> 
> 

> From 3c96903f90016adb29b6d10d5da06cf4bf0d3243 Mon Sep 17 00:00:00 2001
> From: wm4 <nfxjfg at googlemail.com>
> Date: Fri, 28 Jun 2013 10:36:00 +0200
> Subject: [PATCH] lavf: add support for libgme
> 
> ---
>  configure                |   4 +
>  doc/demuxers.texi        |  13 +++
>  libavformat/Makefile     |   1 +
>  libavformat/allformats.c |   1 +
>  libavformat/libgme.c     | 200 +++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 219 insertions(+)
>  create mode 100644 libavformat/libgme.c
[...]
> +static int read_header_gme(AVFormatContext *s)
> +{
> +    AVStream *st;
> +    AVIOContext *pb = s->pb;
> +    GMEContext *gme = s->priv_data;
> +    int64_t sz = avio_size(pb);
> +    char *buf;
> +    char dummy;
> +
> +    if (sz < 0) {
> +        av_log(s, AV_LOG_WARNING, "Could not determine file size\n");
> +        sz = gme->max_size;
> +    } else if (gme->max_size && sz > gme->max_size) {
> +        sz = gme->max_size;
> +    }
> +
> +    buf = av_malloc(sz);
> +    if (!buf)
> +        return AVERROR(ENOMEM);
> +    sz = avio_read(pb, buf, sz);
> +
> +    // Data left means our buffer (the max_size option) is too small
> +    if (avio_read(pb, &dummy, 1) == 1) {

> +        av_log(s, AV_LOG_WARNING, "File size is larger than max_size option, "
> +               "consider increasing the max_size option\n");

Slightly better:
        av_log(s, AV_LOG_WARNING, "File size is larger than max_size option value %d, "
               "consider increasing the max_size value\n");

Also maybe you should use AV_LOG_ERROR.

> +        return AVERROR_BUFFER_TOO_SMALL;
> +    }
> +

[...]

No more comments from me, thanks.
-- 
FFmpeg = Freak & Fundamentalist Magic Perfectionist Extroverse Game