On Wed, Aug 23, 2017 at 10:31:58AM +0200, Tomas Härdin wrote:
> On 2017-08-22 03:23, Tyler Jones wrote:
> > +static int create_residues(vorbis_enc_context *venc)
> > +{
> > + int res, ret;
> > + vorbis_enc_residue *rc;
> > +
> > + venc->nresidues = 2;
> > + venc->residues = av_malloc(sizeof(vorbis_enc_residue) *
> > venc->nresidues);
>
> av_malloc_array()? Applies to most av_malloc() in there
I can change it, but I don't feel that it helps readability in this specific
case above. As for the others that happen to show up in the diffs, I did not
want to make any unnecessary and unrelated functional changes. However, I'll
gladly to switch these cases to `av_malloc_array()` in a separate commit if
desired.
> > - // single mapping
> > - mc = &venc->mappings[0];
> > - mc->submaps = 1;
> > - mc->mux = av_malloc(sizeof(int) * venc->channels);
> > - if (!mc->mux)
> > - return AVERROR(ENOMEM);
> > - for (i = 0; i < venc->channels; i++)
> > - mc->mux[i] = 0;
> > - mc->floor = av_malloc(sizeof(int) * mc->submaps);
> > - mc->residue = av_malloc(sizeof(int) * mc->submaps);
> > - if (!mc->floor || !mc->residue)
> > - return AVERROR(ENOMEM);
> > - for (i = 0; i < mc->submaps; i++) {
> > - mc->floor[i] = 0;
> > - mc->residue[i] = 0;
> > - }
> > - mc->coupling_steps = venc->channels == 2 ? 1 : 0;
> > - mc->magnitude = av_malloc(sizeof(int) * mc->coupling_steps);
> > - mc->angle = av_malloc(sizeof(int) * mc->coupling_steps);
> > - if (!mc->magnitude || !mc->angle)
> > - return AVERROR(ENOMEM);
> > - if (mc->coupling_steps) {
> > - mc->magnitude[0] = 0;
> > - mc->angle[0] = 1;
> > + for (map = 0; map < venc->nmappings; map++) {
> > + mc = &venc->mappings[map];
> > + mc->submaps = 1;
> > + mc->mux = av_malloc(sizeof(int) * venc->channels);
> > + if (!mc->mux)
> > + return AVERROR(ENOMEM);
> > + for (i = 0; i < venc->channels; i++)
> > + mc->mux[i] = 0;
> > + mc->floor = av_malloc(sizeof(int) * mc->submaps);
> > + mc->residue = av_malloc(sizeof(int) * mc->submaps);
> > + if (!mc->floor || !mc->residue)
> > + return AVERROR(ENOMEM);
> > + for (i = 0; i < mc->submaps; i++) {
> > + mc->floor[i] = map;
> > + mc->residue[i] = map;
> > + }
> > + mc->coupling_steps = venc->channels == 2 ? 1 : 0;
> > + mc->magnitude = av_malloc(sizeof(int) * mc->coupling_steps);
> > + mc->angle = av_malloc(sizeof(int) * mc->coupling_steps);
> > + if (!mc->magnitude || !mc->angle)
> > + return AVERROR(ENOMEM);
> > + if (mc->coupling_steps) {
> > + mc->magnitude[0] = 0;
> > + mc->angle[0] = 1;
> > + }
> > }
>
> Maybe nitpicking, but it would be clearer what the changes are if you put
> the indentation change in a separate commit
No, you're right, and it's a good suggestion. I'll move the indentation to a
separate commit when enough other changes have been provided to warrant a new
version.
> > - move_audio(venc, avctx->frame_size);
> > + if (venc->transient < 0) {
> > + move_audio(venc, avctx->frame_size);
> > - for (ch = 0; ch < venc->channels; ch++) {
> > - float *scratch = venc->scratch + 2 * ch * frame_size + frame_size;
> > + for (ch = 0; ch < venc->channels; ch++) {
> > + float *scratch = venc->scratch + 2 * ch * long_win + long_win;
> > - if (!ff_psy_vorbis_block_frame(&venc->vpctx, scratch, ch,
> > - frame_size, block_size))
> > - curr_win = 0;
> > + if (!ff_psy_vorbis_block_frame(&venc->vpctx, scratch, ch,
> > + long_win, short_win))
> > + next_win = 0;
> > + }
> > }
>
> Same here
>
> /Tomas
>
I felt that separating this small amount of lines would just clutter the git
log history, but I'll move these along with the mapping indentations.
Thanks for taking a look,
Tyler Jones
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
