On Tue, 2 Sep 2014 21:18:24 +0200 Reimar Döffinger <reimar.doeffin...@gmx.de> wrote:
> On Tue, Sep 02, 2014 at 08:58:50PM +0200, wm4 wrote: > > On Tue, 2 Sep 2014 20:32:57 +0200 > > Reimar Döffinger <reimar.doeffin...@gmx.de> wrote: > > > @@ -933,6 +938,7 @@ static av_cold int encode_init(AVCodecContext *avctx) > > > } > > > } > > > } > > > + av_freep(&best_state); > > > } > > > > > > if (s->version > 1) { > > > > Is there any specific purpose to these changes? Sure, 64KB stack is > > pushing a bit, but it should be fine on all normal systems. > > First, that is only true if you assume that the calling application > did not already use up all but 64 kB of stack. > As a library, every byte we use on stack is a byte the calling > application must ensure to leave free. > Plus, "normal" system depends a lot on your definition. > If you wanted to run 1000 threads on a 32 bit system, you certainly > can't use 8MB stacks like we might be used to on our systems. > Windows uses 1 MB stack by default. > Since we are a library, I think it is reasonable to say that we > definitely should not never grab more than 1/4th (and I find that rather > high personally). > That leaves us with 256kB. To keep a safety margin since we simply > don't test all paths, I would suggest testing at least for 128 kB max. > That does give us the freedom to allow 64 kB allocations if you should > find my patches too intrusive, it just feels cutting it a bit close > even on fairly common systems, not to mention that it might prevent > us from running at all on more obscure systems. > So I'd prefer to avoid it. However there is the question of which > code mess/benefit ratio we want to accept. I don't see anything wrong in the patch (well, maybe you should switch the code to the "goto fail;" idiom). I was just wondering whether there was a specific reason for this. Did it fail on a certain system? Anyway, I'm not opposed to these patches; just curious. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel