[FFmpeg-devel] Request for review
Roman V. Shaposhnik
rvs
Tue Nov 18 22:57:44 CET 2008
Hi Michael!
Thanks for reviewing it so quickly!
On Tue, 2008-11-18 at 14:32 +0100, Michael Niedermayer wrote:
> > + DVVideoContext *s = (DVVideoContext *)avctx->priv_data;
>
> i think the cast is unneeded
Agreed.
> except that first patch ok
Committed.
> > + const uint8_t off[] = { 2, 6, 8, 0, 4 };
>
> should be const static to make sure stupid compilers dont generate these
> tables in each function call on the stack
Agreed.
> except that, second patch ok
Committed.
> > - if (d->work_chunks[dv_work_pool_size(d)-1].buf_offset)
> > - return 0;
> > -
> > + if (!d->work_chunks[dv_work_pool_size(d)-1].buf_offset) {
> > p = i = 0;
> > for (c=0; c<d->n_difchan; c++) {
> > for (s=0; s<d->difseg_size; s++) {
> > @@ -227,42 +225,45 @@ static int dv_init_dynamic_tables(const DVprofile *d)
> > }
> > }
> > }
> > - return 0;
> > }
> >
>
> ok, though this seems unrelated and could be a seperate commit
Do you mean that something like I've attached should be a separate
commit?
> > DVwork_chunk *work_chunks; /* each thread gets its own chunk of frame to work on */
>
> > + uint32_t *idct_factor; /* FIXME */
>
> why "FIXME" ?
I thought I had ways of optimizing it somewhat even further, but
I don't think that anymore. Besides, a comment like that has nothing
to do with the published history.
Thanks for spotting it -- I'll try to be more careful in the future.
Thanks,
Roman.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1.patch
Type: text/x-patch
Size: 1500 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081118/f3623291/attachment.bin>
More information about the ffmpeg-devel
mailing list