Re: [FFmpeg-devel] [PATCH 3/3] aaccoder: Improve IS phase rejection
>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
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
>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
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
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
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
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