Hi, I've integrated the basic static memory allocation from your patch into my source and it seems to be working well. Nice job. I've decoded a few hundred audio files at lots of different rates and haven't found any instances where more memory has been needed. None of the files have metadata, FYI. It gets pretty close to the maximum but never hits it. I've also tried encoding some files with the AoTuV vorbis encoder (via oggdropXpd) and it doesn't seem to make larger files, though the allocations are different for the same input and quality level. I do have a general nagging concern about memory allocations with older and yet implemented ogg encoders which might have greater memory requests, but I'm not sure what can be done about that. Best you can do is test vigorously on a few popular encoders and exit as graceful as possible if the pseudo static allocation fails.<br>
<br>Is there any reason you changed the name of so many allocations in the source files rather than going with a #ifdef for _ogg_malloc and the other associated functions? Was this just a development/debug decision or something you feel is useful for the released code as well?<br>
<br>Lastly, this is a slightly more general question on the codec. There are a couple places where realloc is used in Tremor - this is definitely hard to implement with this sort of static memory allocation. However, after running all these tests I have yet to find a case where realloc is actually called - it only appears a couple times in the code. Can anyone say if this code is truly dead or is some fix to deal with realloc (most likely just allocating for the whole amount immediately) going to be necessary?<br>
<br>Ethan<br><br><div class="gmail_quote">On Tue, Nov 25, 2008 at 2:48 PM, Nicholas Vinen <span dir="ltr"><<a href="mailto:hb@x256.org">hb@x256.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="Ih2E3d">Andy Grundman wrote:<br>
> I tested your patch with one of my test files with a 100K artwork<br>
> comment, it's a -q9 file, and fails:<br>
><br>
> STATIC MEMORY ERROR: tmem.bufdata exhausted (9216/8192)<br>
><br>
> It also fails on a newly encoded -q9 file without any comments:<br>
><br>
> STATIC MEMORY ERROR: tmem.dectable exhausted (15844/14602)<br>
><br>
> This file is here: <a href="http://bugs.slimdevices.com/attachment.cgi?id=3261" target="_blank">http://bugs.slimdevices.com/attachment.cgi?id=3261</a><br>
> _______________________________________________<br>
> Tremor mailing list<br>
> <a href="mailto:Tremor@xiph.org">Tremor@xiph.org</a><br>
> <a href="http://lists.xiph.org/mailman/listinfo/tremor" target="_blank">http://lists.xiph.org/mailman/listinfo/tremor</a><br>
><br>
<br>
</div>It's because different allocations take up different amounts of space as<br>
the quality level changes. My latest patch allocates pretty much<br>
everything out of a single piece of memory, so if one thing grows and<br>
another shrinks it doesn't fail like that. Try this one:<br>
<br>
<a href="http://x256.org/%7Ehb/Tremor3.patch" target="_blank">http://x256.org/~hb/Tremor3.patch</a><br>
<br>
I'm still a bit uncertain about the comment truncation code; it works on<br>
all my test files now (1k, 2k, 3k, 4k, 5k and 40k comments) but I'm<br>
really not sure that it won't break on some other size. Unfortunately I<br>
find the way the code parses the file rather mysterious and complex and<br>
I have too many other things to work out now to work it all out. I think<br>
the general approach is sound - ignore and discard the latter pages of<br>
large comment fields as they're being read in - but I've implemented it<br>
in a rather clumsy way.<br>
<font color="#888888"><br>
<br>
<br>
Nicholas<br>
</font><div><div></div><div class="Wj3C7c"><br>
_______________________________________________<br>
Tremor mailing list<br>
<a href="mailto:Tremor@xiph.org">Tremor@xiph.org</a><br>
<a href="http://lists.xiph.org/mailman/listinfo/tremor" target="_blank">http://lists.xiph.org/mailman/listinfo/tremor</a><br>
</div></div></blockquote></div><br>