On Fri, Jul 08, 2011 at 01:21:01PM +0100, Joseph Artsimovich wrote: > On 08/07/2011 11:47, Kostya wrote: > >... > >In general code looks good to me, but tables are horrible. Can you please > >format them a bit (at least to fit into 80-character lines)? > I can, but that would be a separate patch, as the existing table > also have very long lines.
Please do. > >Another nit is context->initialized_for_bits variable name. Wouldn't > >output_bits suffice? Also why do you use it in some places and retrieve > >ctx->cid_table->bit_depth in other? > initialized_for_bits is in the decoder. Initially it's 0 which > means not initialized at all. The number of bits is a frame > attribute, so it's not known beforehand. Theoretically, there may > be a mixture of 8-bit and 10-bit frames. And yes, I could be > checking for ctx->cid_table->bit_depth, making sure ctx->cid_table > is not null first. It's just more convenient to have a separate > variable for that. So why not use that variable everywhere instead? > >Additionally, why can't you select proper transform in dsputil without > >overriding avctx->idct* stuff? > I have to override it because the 8-bit DNxHD code uses idct_put() > which implies 8-bit samples by "uint8_t *dest" parameter. You can do that inside dsputil.c like H.264 decoder currently does (by checking avctx->bits_per_raw_sample, maybe Ronald Bultje will give some insight on it). _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
