FYI I'm also looking at this problem right now. I too need to be able to ignore comments of arbitrarily large size. The problem that I have is that I've currently statically allocated for a maximum of 7 ogg buffers in _fetch_buffer(). This works fine for small comments (and all decoding needs I've run into thus far) but large comment headers make additional requests to get more ogg buffers. I think that in the comment decode phase Ogg will allocate buffers continuously to hold all the metadata, which I can't handle.<br>
<br>My thought was to short-circuit this behavior so that, if we're in the comment gathering phase, all data reads go to a dummy ogg_buffer. I would probably add the code in ogg_buffer_alloc() to check if this is a real request for a new buffer or if we should map to the dummy buffer/reference. I'm pretty sure I can safely find the start and end points of this phase by setting a flag right after _vorbis_unpack_info finishes and clearing the flag after _vorbis_unpack_comment finishes. This is a pretty big kludge but I think it would work.<br>
<br>Thoughts?<br><br><div class="gmail_quote">On Thu, Jan 22, 2009 at 10:28 AM, Andy Grundman <span dir="ltr"><<a href="mailto:andy@slimdevices.com">andy@slimdevices.com</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;">
OK, I need to make some more test files and see if any of them can be<br>
read properly. I may use your patch anyway because it at least avoids<br>
allocating tons of memory and crashing. Even if the file doesn't play<br>
it will fail more gracefully than it currently does.<br>
<div><div></div><div class="Wj3C7c"><br>
On Jan 22, 2009, at 10:18 AM, Nicholas Vinen wrote:<br>
<br>
><br>
> Unfortunately while in many cases the patch works, I've found that it<br>
> has problems with certain files. It's a bit of a hack and I don't yet<br>
> fully understand why it doesn't work.<br>
><br>
> A much better solution would be to examine the header of any packet<br>
> which exceeds a certain size and is not yet fully read in yet, and<br>
> determine the type. If it's the comment field, it should be possible<br>
> to<br>
> process what's already been read in and sync up to the next packet<br>
> without reading the rest in at all, rather than what I did which is to<br>
> prevent the current packet in memory growing once it reaches a<br>
> certain size.<br>
><br>
> So in summary I think somebody needs to come up with a better<br>
> solution.<br>
> I will do it at some point but I don't know when that will be, it<br>
> could<br>
> be a while. Someone who understands the Ogg packet code better would<br>
> likely do a good job. I don't yet understand it fully.<br>
><br>
> You may be able to get my code to work but as it is unfortunately I<br>
> don't think it's 100%. And yes you may be right, the later patch may<br>
> have broken the large comment skipping.<br>
><br>
><br>
> Nicholas<br>
><br>
><br>
><br>
> Andy Grundman wrote:<br>
>> Hi Nicholas,<br>
>><br>
>> I'm working on implementing your comment truncation patch into our<br>
>> Tremor code. I have a few questions about it.<br>
>><br>
>> Comparing Tremor3.patch [3] and your original patch [1], #3 seems to<br>
>> have removed the extra code in _vorbis_unpack_comment that breaks out<br>
>> of the comment reading loop if the data was truncated. Using patch<br>
>> #3<br>
>> this loop always tries to allocate space for the truncated comment<br>
>> and<br>
>> fails, so I think what you did in #1 is the right solution. However,<br>
>> using #1 I end up failing on my test file with ENOTVORBIS because the<br>
>> check for the "vorbis" string in vorbis_dsp_headerin fails when<br>
>> looking for the codebooks header.<br>
>><br>
>> Thanks,<br>
>> -Andy<br>
>><br>
>> [1] <a href="http://x256.org/%7Ehb/Tremor.patch" target="_blank">http://x256.org/~hb/Tremor.patch</a><br>
>> [2] <a href="http://x256.org/%7Ehb/Tremor3.patch" target="_blank">http://x256.org/~hb/Tremor3.patch</a><br>
<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>