Re: [FFmpeg-devel] [PATCH 4/4] opus: Don't invert phase when downmixing to mono (per RFC8251)

[James Almer]

On 12/4/2017 4:39 AM, Rostislav Pehlivanov wrote:
> On 2 December 2017 at 17:46, Andrew D'Addesio  wrote:
> 
>> When decoding to downmixed mono, don't put the channels out of phase,
>> as they will cancel out and create audible artifacts. (See
>> RFC 8251 sec. 10.)
>>
>> Signed-off-by: Andrew D'Addesio 
>> ---
>>  libavcodec/opus_pvq.c | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/libavcodec/opus_pvq.c b/libavcodec/opus_pvq.c
>> index 2f7aa74..f18c010 100644
>> --- a/libavcodec/opus_pvq.c
>> +++ b/libavcodec/opus_pvq.c
>> @@ -643,7 +643,13 @@ static av_always_inline uint32_t
>> quant_band_template(CeltPVQ *pvq, CeltFrame *f,
>>  }
>>  } else {
>>  inv = (b > 2 << 3 && f->remaining2 > 2 << 3) ?
>> ff_opus_rc_dec_log(rc, 2) : 0;
>> +
>> +/* Don't put the channels out of phase if we are decoding
>> to downmixed
>> + * mono as this subjectively sounds bad (RFC 8251 section
>> 10). */
>> +if (f->channels == 1)
>> +inv = 0;
>>  }
>> +
>>  itheta = 0;
>>  }
>>  qalloc = opus_rc_tell_frac(rc) - tell;
>> --
>> 2.15.1.windows.2
>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> 
> RFC8251 made this optional, pushed a new version which added an option to
> toggle this.

Valgrind complains about your changes in 1b8649c2ce. Conditional jump or
move depends on uninitialised value(s).

http://fate.ffmpeg.org/report.cgi?time=20171204200723=x86_64-archlinux-gcc-valgrind
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 4/4] opus: Don't invert phase when downmixing to mono (per RFC8251)

[Rostislav Pehlivanov]

On 2 December 2017 at 17:46, Andrew D'Addesio  wrote:

> When decoding to downmixed mono, don't put the channels out of phase,
> as they will cancel out and create audible artifacts. (See
> RFC 8251 sec. 10.)
>
> Signed-off-by: Andrew D'Addesio 
> ---
>  libavcodec/opus_pvq.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/libavcodec/opus_pvq.c b/libavcodec/opus_pvq.c
> index 2f7aa74..f18c010 100644
> --- a/libavcodec/opus_pvq.c
> +++ b/libavcodec/opus_pvq.c
> @@ -643,7 +643,13 @@ static av_always_inline uint32_t
> quant_band_template(CeltPVQ *pvq, CeltFrame *f,
>  }
>  } else {
>  inv = (b > 2 << 3 && f->remaining2 > 2 << 3) ?
> ff_opus_rc_dec_log(rc, 2) : 0;
> +
> +/* Don't put the channels out of phase if we are decoding
> to downmixed
> + * mono as this subjectively sounds bad (RFC 8251 section
> 10). */
> +if (f->channels == 1)
> +inv = 0;
>  }
> +
>  itheta = 0;
>  }
>  qalloc = opus_rc_tell_frac(rc) - tell;
> --
> 2.15.1.windows.2
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

RFC8251 made this optional, pushed a new version which added an option to
toggle this.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 4/4] opus: Don't invert phase when downmixing to mono (per RFC8251)

[Andrew D'Addesio]

When decoding to downmixed mono, don't put the channels out of phase,
as they will cancel out and create audible artifacts. (See
RFC 8251 sec. 10.)

Signed-off-by: Andrew D'Addesio 
---
 libavcodec/opus_pvq.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavcodec/opus_pvq.c b/libavcodec/opus_pvq.c
index 2f7aa74..f18c010 100644
--- a/libavcodec/opus_pvq.c
+++ b/libavcodec/opus_pvq.c
@@ -643,7 +643,13 @@ static av_always_inline uint32_t 
quant_band_template(CeltPVQ *pvq, CeltFrame *f,
 }
 } else {
 inv = (b > 2 << 3 && f->remaining2 > 2 << 3) ? 
ff_opus_rc_dec_log(rc, 2) : 0;
+
+/* Don't put the channels out of phase if we are decoding to 
downmixed
+ * mono as this subjectively sounds bad (RFC 8251 section 10). 
*/
+if (f->channels == 1)
+inv = 0;
 }
+
 itheta = 0;
 }
 qalloc = opus_rc_tell_frac(rc) - tell;
-- 
2.15.1.windows.2

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

[FFmpeg-devel] [PATCH 4/4] opus: Don't invert phase when downmixing to mono (per RFC8251)

[Andrew D'Addesio]

When decoding to downmixed mono, don't put the channels out of phase,
as they will cancel out and create audible artifacts. (See
RFC 8251 sec. 10.)

Signed-off-by: Andrew D'Addesio 
---
 libavcodec/opus_pvq.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavcodec/opus_pvq.c b/libavcodec/opus_pvq.c
index 2f7aa74..f18c010 100644
--- a/libavcodec/opus_pvq.c
+++ b/libavcodec/opus_pvq.c
@@ -643,7 +643,13 @@ static av_always_inline uint32_t 
quant_band_template(CeltPVQ *pvq, CeltFrame *f,
 }
 } else {
 inv = (b > 2 << 3 && f->remaining2 > 2 << 3) ? 
ff_opus_rc_dec_log(rc, 2) : 0;
+
+/* Don't put the channels out of phase if we are decoding to 
downmixed
+ * mono as this subjectively sounds bad (RFC 8251 section 10). 
*/
+if (f->channels == 1)
+inv = 0;
 }
+
 itheta = 0;
 }
 qalloc = opus_rc_tell_frac(rc) - tell;
-- 
2.15.1.windows.2

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