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
atomnu...@gmail.com 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 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 klaussfre...@gmail.com wrote:

 On Fri, Jul 17, 2015 at 11:19 PM, Rostislav Pehlivanov
 atomnu...@gmail.com 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 Claudio Freire
On Fri, Jul 17, 2015 at 10:32 PM, Rostislav Pehlivanov
atomnu...@gmail.com 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 6:20 PM, Rostislav Pehlivanov
atomnu...@gmail.com 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


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 klaussfre...@gmail.com wrote:

 On Fri, Jul 17, 2015 at 6:20 PM, Rostislav Pehlivanov
 atomnu...@gmail.com 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