[FFmpeg-devel] [PATCH 2/2] libmodplug: add an option to enlarge the max supported file size.

[Michael Niedermayer]

On Thu, Oct 06, 2011 at 01:36:32AM +0200, Clément Bœsch wrote:
> On Thu, Oct 06, 2011 at 01:24:21AM +0200, Clément Bœsch wrote:
> > On Thu, Oct 06, 2011 at 12:35:38AM +0200, Michael Niedermayer wrote:
> > > On Thu, Oct 06, 2011 at 12:30:53AM +0200, Clément Bœsch wrote:
> > > > On Thu, Oct 06, 2011 at 12:22:34AM +0200, Michael Niedermayer wrote:
> > > > [...]
> > > > > > @@ -67,8 +74,23 @@ static int modplug_read_header(AVFormatContext *s, AVFormatParameters *ap)
> > > > > >      AVIOContext *pb = s->pb;
> > > > > >      ModPlug_Settings settings;
> > > > > >      ModPlugContext *modplug = s->priv_data;
> > > > > > +    int sz = avio_size(pb);
> > > > > >  
> > > > > > -    int sz = avio_read(pb, modplug->buf, sizeof(modplug->buf));
> > > > > > +    if (sz < 0) {
> > > > > > +        av_log(s, AV_LOG_WARNING, "Could not determine file size\n");
> > > > > 
> > > > > > +        if (modplug->max_size != FF_MODPLUG_DEF_FILE_SIZE)
> > > > > > +            sz = modplug->max_size;
> > > > > > +        else
> > > > > > +            sz = FF_MODPLUG_DEF_FILE_SIZE;
> > > > > 
> > > > > sz = modplug->max_size;
> > > > > 
> > > > 
> > > > Indeed…
> > > > 
> > > > > 
> > > > > > +    } else if (modplug->max_size && sz > modplug->max_size) {
> > > > > > +        sz = modplug->max_size;
> > > > > > +        av_log(s, AV_LOG_WARNING, "Max file size reach%s, allocating %dB\n",
> > > > > > +               sz == FF_MODPLUG_DEF_FILE_SIZE ? " (see -max_size)" : "", sz);
> > > > > > +    }
> > > > > 
> > > > > does this work for any file ?
> > > > 
> > > > No it's likely to fail every time unfortunately.
> > > > 
> > > > > i would have thought many mod file formats are sensitiv to being
> > > > > truncated
> > > > 
> > > > Yes, it was the case with all the samples I tested (it's the reason the
> > > > message mentions the -max_size option to workaround the issue), but I
> > > > believe at least one will be OK, so it doesn't fail immediately.
> > > > 
> > > > Maybe you would prefer an extra warning saying it will likely fail?
> > > 
> > > yes
> > > 
> > 
> > Added "but demuxing is likely to fail due to incomplete buffer".
> > 
> > [...]
> > 
> 
> And with the other changes too…
> 
> -- 
> Clément B.

>  libmodplug.c |   24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> e55b879b4e667f27d1be6f1c57e2d62d6652039e  0002-libmodplug-add-an-option-to-enlarge-the-max-supporte.patch
> From a3978f80d5e43480491f2bfb7b0c143025819fab Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> Date: Wed, 5 Oct 2011 22:47:46 +0200
> Subject: [PATCH 2/3] libmodplug: add an option to enlarge the max supported
>  file size.

LGTM

[..]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111006/d3eb4728/attachment.asc>