[FFmpeg-devel] [PATCH] Add libmodplug support.

[Reimar Döffinger]

On Tue, Oct 04, 2011 at 09:11:36PM +0200, Clément Bœsch wrote:
> +    int sz = avio_read(pb, mikmod->buf, sizeof mikmod->buf);

In AVInputFormat you use sizeof with (), consistent would be good IMO.
Also, any chance of getting a probe function?

> +static int mikmod_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    int ret, n;
> +    MikModContext *mikmod = s->priv_data;
> +    uint8_t buf[512];
> +
> +    n = ModPlug_Read(mikmod->f, buf, sizeof buf);
> +    if (n <= 0)
> +        return AVERROR(EIO);
> +
> +    ret = av_new_packet(pkt, n);
> +    if (ret)
> +        return ret;

Only < 0 is really supposed to indicate an error - and even
if not you should not return > 0 like this here.

> +    pkt->pts  = pkt->dts = AV_NOPTS_VALUE;
> +    pkt->size = n;

I don't think setting the size is necessary if it's the
same value as in av_new_packet.

> +    memcpy(pkt->data, buf, n);

And what don't you just do ModPlug_Read directly into
pkt->data?

> +static int mikmod_read_seek(AVFormatContext *s, int stream_idx, int64_t ts, int flags)
> +{
> +    const MikModContext *mikmod = s->priv_data;
> +    ModPlug_Seek(mikmod->f, (int)ts);

What is the point of the cast? And does ModPlug_Seek really take
the position in samples?