Re: [FFmpeg-devel] [PATCH 10/11] aaccoder: implement intensity stereo

2015-06-30 Thread Rostislav Pehlivanov
The problem was that ms_mask altered the phase during distortion
calculations but did not alter it again during codebook selection (e.g.
INTENSITY_BT or INTENSITY_BT2), so the phases got swapped around and the
artifacts appeared. M/S is off by default so the artifacts were only
present if -stereo_mode was set to something other than ms_off.
I've improved phase determination by using a separate variable and adding
up all the phases for every loop around w2. That way ms_mask will affect
the overall phase and distortion calculations will be more correct (using
the current overall phase instead of the local phase for a single w2 loop
will make some spectral coefficients have the wrong phase since they are a
minority and thus increase the error).

On 30 June 2015 at 05:42, Rostislav Pehlivanov  wrote:

> >cpe->ms_mask[w*16+g] = 0;
> This defeats the purpose of changing the phase of the spectral
> coefficients if ms_mask has been set by search_for_ms. If is_mask[idx] is 1
> then ms_mask is only used to alter the phase of the spectral coefficients,
> so probably the phase gets altered incorrectly. The reason why ms_mask is
> even considered is only because the decoder uses it in that way and because
> the specifications mention that IS and M/S are exclusive yet related. Maybe
> there might be something wrong with how the search_for_ms flags bands so it
> inverts the phase of IS incorrectly. Or maybe the code to determine the
> phase of the spectral coefficients might be wrong in search_for_is. I'll
> play around with how the phase is set and maybe in case nothing works out
> I'll unflag ms_mask[].
>
> On 30 June 2015 at 02:35, Claudio Freire  wrote:
>
>> On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
>>  wrote:
>> > +if (dist2 <= dist1) {
>> > +cpe->is_mask[w*16+g] = 1;
>> > +cpe->ch[0].is_ener[w*16+g] = ener1/ener01;
>> > +cpe->ch[1].is_ener[w*16+g] = ener0/ener1;
>> > +if (s_coef0*s_coef1 >= 0.0f)
>> > +cpe->ch[1].band_type[w*16+g] = INTENSITY_BT;
>> > +else
>> > +cpe->ch[1].band_type[w*16+g] = INTENSITY_BT2;
>> > +count++;
>> > +}
>>
>>
>> If you don't add an:
>>
>> cpe->ms_mask[w*16+g] = 0;
>>
>> In there, you get horrible artifacts. Tested it quite a bit already.
>>
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 10/11] aaccoder: implement intensity stereo

2015-06-29 Thread Rostislav Pehlivanov
>cpe->ms_mask[w*16+g] = 0;
This defeats the purpose of changing the phase of the spectral coefficients
if ms_mask has been set by search_for_ms. If is_mask[idx] is 1 then ms_mask
is only used to alter the phase of the spectral coefficients, so probably
the phase gets altered incorrectly. The reason why ms_mask is even
considered is only because the decoder uses it in that way and because the
specifications mention that IS and M/S are exclusive yet related. Maybe
there might be something wrong with how the search_for_ms flags bands so it
inverts the phase of IS incorrectly. Or maybe the code to determine the
phase of the spectral coefficients might be wrong in search_for_is. I'll
play around with how the phase is set and maybe in case nothing works out
I'll unflag ms_mask[].

On 30 June 2015 at 02:35, Claudio Freire  wrote:

> On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
>  wrote:
> > +if (dist2 <= dist1) {
> > +cpe->is_mask[w*16+g] = 1;
> > +cpe->ch[0].is_ener[w*16+g] = ener1/ener01;
> > +cpe->ch[1].is_ener[w*16+g] = ener0/ener1;
> > +if (s_coef0*s_coef1 >= 0.0f)
> > +cpe->ch[1].band_type[w*16+g] = INTENSITY_BT;
> > +else
> > +cpe->ch[1].band_type[w*16+g] = INTENSITY_BT2;
> > +count++;
> > +}
>
>
> If you don't add an:
>
> cpe->ms_mask[w*16+g] = 0;
>
> In there, you get horrible artifacts. Tested it quite a bit already.
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 10/11] aaccoder: implement intensity stereo

2015-06-29 Thread Claudio Freire
On Fri, Jun 26, 2015 at 5:16 PM, Rostislav Pehlivanov
 wrote:
> +if (dist2 <= dist1) {
> +cpe->is_mask[w*16+g] = 1;
> +cpe->ch[0].is_ener[w*16+g] = ener1/ener01;
> +cpe->ch[1].is_ener[w*16+g] = ener0/ener1;
> +if (s_coef0*s_coef1 >= 0.0f)
> +cpe->ch[1].band_type[w*16+g] = INTENSITY_BT;
> +else
> +cpe->ch[1].band_type[w*16+g] = INTENSITY_BT2;
> +count++;
> +}


If you don't add an:

cpe->ms_mask[w*16+g] = 0;

In there, you get horrible artifacts. Tested it quite a bit already.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel