On Tue, Oct 07, 2014 at 05:41:09PM +1100, Peter Ross wrote: > Thanks for the review (also thanks compn). > I will address all your comments -- some further questions below:
[...] > > > + unsigned int half_prob[DST_MAX_CHANNELS]; > > > + unsigned int map_ch_to_felem[DST_MAX_CHANNELS]; > > > + unsigned int map_ch_to_pelem[DST_MAX_CHANNELS]; > > > + DECLARE_ALIGNED(16, uint8_t, status)[DST_MAX_CHANNELS][16]; > > > + DECLARE_ALIGNED(16, int16_t, filter)[DST_MAX_ELEMENTS][16][256]; > > > > At least the last one is quite large I think. > > Other structures (not shown above) are also large, so I have moved these into > the context. > > > Please consider putting it in the context or so instead. > > That's also less problematic with alignment. > > The 'filter' array is ~100 kilobytes ((6ch x 2) x 16 x 256 x sizeof(int16_t)). > It is a lookup table, and is used _a lot_. > When I move this into the context struct, decoder performance drops 15%. it might be worth taking a quick look at te generated asm code, for the 2 cases maybe gcc does something silly > > What is a reasonable size for on-stack variables? > > > > + if ((ret = init_get_bits8(&gb, avpkt->data, avpkt->size)) < 0) > > > + if ((ret = read_map(&gb, &fsets, map_ch_to_felem, avctx->channels)) > > > < 0) > > > + if ((ret = read_map(&gb, &probs, map_ch_to_pelem, > > > avctx->channels)) < 0) > > > > I still think putting assignments into an if is really bad idea all > > around. > > No exception for simple error return codes? if ((ret = func()) < 0) is used alot in existing code so IMHO theres no problem, the main risk is getting the () wrong, which did happens a few times over the years ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
