Re: [FFmpeg-devel] [PATCH 3/3] aaccoder: Improve IS phase rejection

2015-07-20 Thread Rostislav Pehlivanov
>An alternative technique that may be better in that regard, then,
>would be to measure distortion with both phases, and pick the phase
>that yields the lowest distortion?
That's a very good suggestion.
Since the main problem with M/S and IS was phasing this might be a way to
solve that too.
I'll give it a try.

On 21 July 2015 at 02:02, Claudio Freire  wrote:

> On Fri, Jul 17, 2015 at 11:19 PM, Rostislav Pehlivanov
>  wrote:
> >>But even if not used for avoding I/S, it can be used to pick whether
> >>to invert the phases, where it was clearly more stable.
> > In case the phase is very clearly wrong then there will be an increase in
> > the distortion which should cause the dist2 <= dist1 check to fail and
> thus
> > not use IS. So the phase isn't even very important. The whole point of
> the
> > phase check is to pick out the obviously wrong phases and save time by
> not
> > having to calculate the error of the spectral coefficients. And this
> works
> > better when you individually pick out a majority rather than just summing
> > them up and then doing the decisions.
>
> All the more reason to use energy then.
>
> If you use this sign-difference count, low-energy noise may change the
> count while being inaudible, yielding an incorrect phase value. After
> that, distortion will be huge and the band won't be I/S encoded, but
> it might if the phase value was otherwise.
>
> In general, this algorithm seems fragile (ie: not robust in the
> presence of noise).
>
> Why not forego the optimization and optimize efficiency instead? (it's
> the priority now, running time optimization should be done after
> exploiting the technique to increase quality as much as posibble).
>
> An alternative technique that may be better in that regard, then,
> would be to measure distortion with both phases, and pick the phase
> that yields the lowest distortion?
>
> Give it a try.
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/3] aaccoder: Improve IS phase rejection

2015-07-20 Thread Claudio Freire
On Fri, Jul 17, 2015 at 11:19 PM, Rostislav Pehlivanov
 wrote:
>>But even if not used for avoding I/S, it can be used to pick whether
>>to invert the phases, where it was clearly more stable.
> In case the phase is very clearly wrong then there will be an increase in
> the distortion which should cause the dist2 <= dist1 check to fail and thus
> not use IS. So the phase isn't even very important. The whole point of the
> phase check is to pick out the obviously wrong phases and save time by not
> having to calculate the error of the spectral coefficients. And this works
> better when you individually pick out a majority rather than just summing
> them up and then doing the decisions.

All the more reason to use energy then.

If you use this sign-difference count, low-energy noise may change the
count while being inaudible, yielding an incorrect phase value. After
that, distortion will be huge and the band won't be I/S encoded, but
it might if the phase value was otherwise.

In general, this algorithm seems fragile (ie: not robust in the
presence of noise).

Why not forego the optimization and optimize efficiency instead? (it's
the priority now, running time optimization should be done after
exploiting the technique to increase quality as much as posibble).

An alternative technique that may be better in that regard, then,
would be to measure distortion with both phases, and pick the phase
that yields the lowest distortion?

Give it a try.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/3] aaccoder: Improve IS phase rejection

2015-07-17 Thread Rostislav Pehlivanov
>From my earlier testing, it's a bit too conservative, and if you make
>it even more conservative, it may end up reducing the effectiveness of
>I/S.
Yes, that's what I remember too from testing it too.

>But even if not used for avoding I/S, it can be used to pick whether
>to invert the phases, where it was clearly more stable.
In case the phase is very clearly wrong then there will be an increase in
the distortion which should cause the dist2 <= dist1 check to fail and thus
not use IS. So the phase isn't even very important. The whole point of the
phase check is to pick out the obviously wrong phases and save time by not
having to calculate the error of the spectral coefficients. And this works
better when you individually pick out a majority rather than just summing
them up and then doing the decisions.

Reverse the phase and you'll observe tons and tons of distortions. It's why
M/S is turned off when IS is used.

>The spike will be very audible though, and it should be ok if it
>dominates. I don't remember any case where it was mistaken when I was
>testing.
The spike is still audiable and dominates. Remember, the phase isn't even
half of the story. The energy still plays a role because it's in the
equation for setting the left channel's spectral coefficients and the
balance (encoded as a sf_idx).

On 18 July 2015 at 02:57, Claudio Freire  wrote:

> On Fri, Jul 17, 2015 at 10:32 PM, Rostislav Pehlivanov
>  wrote:
> > That previous idea discriminated way too many bands for it to be actually
> > useful. And it would require special cases for coefficients which 'blow
> up'
> > and have an insane value.
> ...
> >> What happened to the idea of comparing the energies of the addition
> >> and diferrence and deciding on that?
> >>
> >> It looked better at rejecting these cases than this one when we talked
> >> about it.
>
> From my earlier testing, it's a bit too conservative, and if you make
> it even more conservative, it may end up reducing the effectiveness of
> I/S.
>
> But even if not used for avoding I/S, it can be used to pick whether
> to invert the phases, where it was clearly more stable.
>
> > This method is naive but it handles spikes better since a single spike in
> > one channel will only cause a single phase to switch among a reasonably
> > sized number of coefficients summed over.
>
> The spike will be very audible though, and it should be ok if it
> dominates. I don't remember any case where it was mistaken when I was
> testing.
>
> Do you have a sample that exhibits this?
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/3] aaccoder: Improve IS phase rejection

2015-07-17 Thread Claudio Freire
On Fri, Jul 17, 2015 at 10:32 PM, Rostislav Pehlivanov
 wrote:
> That previous idea discriminated way too many bands for it to be actually
> useful. And it would require special cases for coefficients which 'blow up'
> and have an insane value.
...
>> What happened to the idea of comparing the energies of the addition
>> and diferrence and deciding on that?
>>
>> It looked better at rejecting these cases than this one when we talked
>> about it.

From my earlier testing, it's a bit too conservative, and if you make
it even more conservative, it may end up reducing the effectiveness of
I/S.

But even if not used for avoding I/S, it can be used to pick whether
to invert the phases, where it was clearly more stable.

> This method is naive but it handles spikes better since a single spike in
> one channel will only cause a single phase to switch among a reasonably
> sized number of coefficients summed over.

The spike will be very audible though, and it should be ok if it
dominates. I don't remember any case where it was mistaken when I was
testing.

Do you have a sample that exhibits this?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/3] aaccoder: Improve IS phase rejection

2015-07-17 Thread Rostislav Pehlivanov
That previous idea discriminated way too many bands for it to be actually
useful. And it would require special cases for coefficients which 'blow up'
and have an insane value.

This method is naive but it handles spikes better since a single spike in
one channel will only cause a single phase to switch among a reasonably
sized number of coefficients summed over.



On 18 July 2015 at 00:32, Claudio Freire  wrote:

> On Fri, Jul 17, 2015 at 6:20 PM, Rostislav Pehlivanov
>  wrote:
> > This commit adds a slightly more robust way of determining whether the
> phases
> > match or are too different for IS to be used.
> > ---
> >  libavcodec/aaccoder.c | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
> > index 17b14d6..bd232f6 100644
> > --- a/libavcodec/aaccoder.c
> > +++ b/libavcodec/aaccoder.c
> > @@ -56,6 +56,9 @@
> >  /** Frequency in Hz for lower limit of intensity stereo   **/
> >  #define INT_STEREO_LOW_LIMIT 6100
> >
> > +/** If less than this fraction of coeff phases agree disable IS for
> that band **/
> > +#define IS_PHASE_DIFF_LIM 0.11
> > +
> >  /** Total number of usable codebooks **/
> >  #define CB_TOT 12
> >
> > @@ -1218,9 +1221,9 @@ static void search_for_is(AACEncContext *s,
> AVCodecContext *avctx, ChannelElemen
> >  ener01 += (coef0 + coef1)*(coef0 + coef1);
> >  }
> >  }
> > -if (!phase) { /* Too much phase difference between
> channels */
> > +if
> (fabs(phase)/(sce0->ics.group_len[w]*sce0->ics.swb_sizes[g]) <
> IS_PHASE_DIFF_LIM) {
> >  start += sce0->ics.swb_sizes[g];
> > -continue;
> > +continue; /* Too much phase difference */
> >  }
> >  phase = av_clip(phase, -1, 1);
> >  for (w2 = 0; w2 < sce0->ics.group_len[w]; w2++) {
>
> What happened to the idea of comparing the energies of the addition
> and diferrence and deciding on that?
>
> It looked better at rejecting these cases than this one when we talked
> about it.
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/3] aaccoder: Improve IS phase rejection

2015-07-17 Thread Claudio Freire
On Fri, Jul 17, 2015 at 6:20 PM, Rostislav Pehlivanov
 wrote:
> This commit adds a slightly more robust way of determining whether the phases
> match or are too different for IS to be used.
> ---
>  libavcodec/aaccoder.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
> index 17b14d6..bd232f6 100644
> --- a/libavcodec/aaccoder.c
> +++ b/libavcodec/aaccoder.c
> @@ -56,6 +56,9 @@
>  /** Frequency in Hz for lower limit of intensity stereo   **/
>  #define INT_STEREO_LOW_LIMIT 6100
>
> +/** If less than this fraction of coeff phases agree disable IS for that 
> band **/
> +#define IS_PHASE_DIFF_LIM 0.11
> +
>  /** Total number of usable codebooks **/
>  #define CB_TOT 12
>
> @@ -1218,9 +1221,9 @@ static void search_for_is(AACEncContext *s, 
> AVCodecContext *avctx, ChannelElemen
>  ener01 += (coef0 + coef1)*(coef0 + coef1);
>  }
>  }
> -if (!phase) { /* Too much phase difference between channels 
> */
> +if 
> (fabs(phase)/(sce0->ics.group_len[w]*sce0->ics.swb_sizes[g]) < 
> IS_PHASE_DIFF_LIM) {
>  start += sce0->ics.swb_sizes[g];
> -continue;
> +continue; /* Too much phase difference */
>  }
>  phase = av_clip(phase, -1, 1);
>  for (w2 = 0; w2 < sce0->ics.group_len[w]; w2++) {

What happened to the idea of comparing the energies of the addition
and diferrence and deciding on that?

It looked better at rejecting these cases than this one when we talked about it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 3/3] aaccoder: Improve IS phase rejection

2015-07-17 Thread Rostislav Pehlivanov
This commit adds a slightly more robust way of determining whether the phases
match or are too different for IS to be used.
---
 libavcodec/aaccoder.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
index 17b14d6..bd232f6 100644
--- a/libavcodec/aaccoder.c
+++ b/libavcodec/aaccoder.c
@@ -56,6 +56,9 @@
 /** Frequency in Hz for lower limit of intensity stereo   **/
 #define INT_STEREO_LOW_LIMIT 6100
 
+/** If less than this fraction of coeff phases agree disable IS for that band 
**/
+#define IS_PHASE_DIFF_LIM 0.11
+
 /** Total number of usable codebooks **/
 #define CB_TOT 12
 
@@ -1218,9 +1221,9 @@ static void search_for_is(AACEncContext *s, 
AVCodecContext *avctx, ChannelElemen
 ener01 += (coef0 + coef1)*(coef0 + coef1);
 }
 }
-if (!phase) { /* Too much phase difference between channels */
+if 
(fabs(phase)/(sce0->ics.group_len[w]*sce0->ics.swb_sizes[g]) < 
IS_PHASE_DIFF_LIM) {
 start += sce0->ics.swb_sizes[g];
-continue;
+continue; /* Too much phase difference */
 }
 phase = av_clip(phase, -1, 1);
 for (w2 = 0; w2 < sce0->ics.group_len[w]; w2++) {
-- 
2.4.3.573.g4eafbef

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel