On Wed, Jul 29, 2015 at 1:44 AM, Rostislav Pehlivanov
<atomnu...@gmail.com> wrote:
> + if (cpe->ms_mode)
> + phase = 1 - 2 * cpe->ms_mask[w*16+g];
Shouldn't it be ?
phase *= 1 - ... ;
phase is an argument, the original code would step on it, with a value
that doesn't depend on phase, so it would fail to evaluate both
phases. Using phase *= would make sure to test both phases.
Well, that's the general idea, except it breaks the phase assigned to
the struct. Something like the following does work though:
ephase = phase;
if (cpe->ms_mode)
ephase *= 1 - 2 * cpe->ms_mask[w*16+g];
and then change all uses of phase into ephase, except the last that remains:
is_error.phase = phase; // original phase
is_error.pass = dist2 <= dist1;
On Wed, Jul 29, 2015 at 1:44 AM, Rostislav Pehlivanov
<atomnu...@gmail.com> wrote:
> for (w = 0; w < 128; w++)
> if (sce1->band_type[w] >= INTENSITY_BT2)
> sce1->band_type[w] = 0;
>
> - if (!cpe->common_window)
> - return;
> - for (w = 0; w < sce0->ics.num_windows; w += sce0->ics.group_len[w]) {
> - start = 0;
> - for (g = 0; g < sce0->ics.num_swb; g++) {
> - if (start*freq_mult > INT_STEREO_LOW_LIMIT*(lambda/170.0f) &&
> - cpe->ch[0].band_type[w*16+g] != NOISE_BT &&
> !cpe->ch[0].zeroes[w*16+g] &&
> - cpe->ch[1].band_type[w*16+g] != NOISE_BT &&
> !cpe->ch[1].zeroes[w*16+g]) {
> - int phase = 0;
> - float ener0 = 0.0f, ener1 = 0.0f, ener01 = 0.0f;
> - float dist1 = 0.0f, dist2 = 0.0f;
> + if (!cpe->common_window)
> + return;
> + for (w = 0; w < sce0->ics.num_windows; w += sce0->ics.group_len[w]) {
> + start = 0;
> + for (g = 0; g < sce0->ics.num_swb; g++) {
> + if (start*freq_mult >
> INT_STEREO_LOW_LIMIT*(s->lambda/170.0f) &&
This looks strange. As it is that patch, it ends up with code like:
> for (w = 0; w < 128; w++)
> if (sce1->band_type[w] >= INTENSITY_BT2)
> sce1->band_type[w] = 0;
>
> if (!cpe->common_window)
> return;
> for (w = 0; w < sce0->ics.num_windows; w += sce0->ics.group_len[w]) {
> start = 0;
> for (g = 0; g < sce0->ics.num_swb; g++) {
Which looks wrong. Bad indentation right?
I think you meant:
for (w = 0; w < 128; w++)
if (sce1->band_type[w] >= INTENSITY_BT2)
sce1->band_type[w] = 0;
if (!cpe->common_window)
return;
for (w = 0; w < sce0->ics.num_windows; w += sce0->ics.group_len[w]) {
start = 0;
for (g = 0; g < sce0->ics.num_swb; g++) {
A big part of the diff in that hunk is reindent, so I believe if you
fix that indentation snafu the patch will shrink.
On Wed, Jul 29, 2015 at 1:44 AM, Rostislav Pehlivanov
<atomnu...@gmail.com> wrote:
> for (w2 = 0; w2 < sce0->ics.group_len[w]; w2++) {
> + abs_pow34_v(L34, sce0->coeffs+start+(w+w2)*128,
> sce0->ics.swb_sizes[g]);
> + abs_pow34_v(R34, sce1->coeffs+start+(w+w2)*128,
> sce0->ics.swb_sizes[g]);
> for (i = 0; i < sce0->ics.swb_sizes[g]; i++) {
> float coef0 = sce0->pcoeffs[start+(w+w2)*128+i];
> float coef1 = sce1->pcoeffs[start+(w+w2)*128+i];
> - phase += coef0*coef1 >= 0.0f ? 1 : -1;
> ener0 += coef0*coef0;
> ener1 += coef1*coef1;
> ener01 += (coef0 + coef1)*(coef0 + coef1);
> }
> }
Careful, you're stepping on L34 and R34 on eight_short_window, and
passing the last results only to calc_encoding_err_is.
In fact, I'm thinking I may have induced you to make that mistake when
I suggested not to compute R34 / L34 twice (once for each phase),
since L34 and R34 only have room for one window, and
calc_encoding_err_is needs to process a whole window group.
I think you'll have to move it back to inside calc_encoing_err_is and
just compute it twice. Redundant, but at least it's correct.
Also, you should use pcoeffs (coeffs will have M/S applied to it when ms_mask).
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
