On 08/07/2011 13:38, Kostya wrote:
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?
In the decoder, I can indeed use initialized_for_bits everywhere. I'll
make that change. In the encoder, I just don't have it.
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).
I'll take a look at it.
--
Joseph Artsimovich
Software Developer
MirriAd Ltd
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
