[Ffmpeg-devel] [PATCH] flacenc - lpc and options
Michael Niedermayer
michaelni
Sat Jul 1 12:20:29 CEST 2006
Hi
On Fri, Jun 30, 2006 at 09:27:02PM -0400, Justin Ruggles wrote:
[...]
> >
> > [...]
> >
> >
> >>+ for(i=0; i<sub->order; i++) {
> >>+ sub->coefs[i] = coefs[sub->order-1][i];
> >> }
> >>- return bits[sub->order];
> >>+ porder = get_max_p_order(max_porder, n, sub->order);
> >>+ encode_residual_lpc(res, smp, n, sub->order, sub->coefs, sub->shift);
> >
> >
> > hmm, coeffs is copied into sub->coefs and then used, why isnt it used withot
> > copying?
> >
>
> The local coefs array is a 2-D array of all the coefs for all orders,
> and it is allocated on the stack. The codec context just has a 1-D
> array of the coefs of the selected order and is in memory allocated
> during flac_encode_init(). The local coeffs are copied into the encoder
> context because they have to be accessed later in order to write them on
> the output stream. I could have used the local array to encode the
> residual, but I still would have to copy them to the context.
>
> Another solution to this could be to store the full 2-D array in the
> encoder context. I just thought it was a cleaner solution to discard
> the information for the orders which are not used.
ok, leave the code as is, if the copy is needed iam fine with it, its
just a few bytes anyway
>
> > [...]
> >
> >>+ /**
> >>+ * sets audio frame size using frame duration in milliseconds
> >>+ * - encoding: set by user.
> >>+ * - decoding: unused.
> >>+ */
> >>+ int block_time_ms;
> >
> >
> > iam a little unsure about this, maybe you could leave this one out for now
> > and set it just based on compression_level
> >
> > [...]
> >
>
> This was to give the user the option to set the block size. Do you think
> the user should instead be able to set AVCodecContext.frame_size
> explicitly before calling encode_init()? Currently it is set by the
> encoder only, but i see no reason not to let the user set it. The
> encoders currently don't check the value anyway and would just
> over-write it with what it's supposed to be. Would there be anything
> wrong with some encoders checking first for a user-preferred value? I
> could change the comment to something like:
> /**
> * samples per packet.
> * - encoding: set by user, but may be changed by 'init'.
> * - decoding: set by lavc.
> */
> int frame_size;
>
> or would it be better to let the user set it and return an error if it
> is an invalid value for that codec, like is done with most of the other
> parameters?
hmm, iam more in favor of check & error
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is
More information about the ffmpeg-devel
mailing list