diff mbox series

[FFmpeg-devel] avcodec/on2avc: use the matching AVTX context for the 512 sized iMDCT

Message ID 20230805083259.24406-1-cus@passwd.hu
State Accepted
Commit f296c765905ca110a05d77a7e919d992800d2e11
Headers show
Series [FFmpeg-devel] avcodec/on2avc: use the matching AVTX context for the 512 sized iMDCT | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Marton Balint Aug. 5, 2023, 8:32 a.m. UTC
Improves the audio corruption regression caused by
6ba0aa1770ba29eb4126c6a706f6b0cd3809648f reported in ticket #10029.

There is still however a noticable audio glitch, so the FFT conversion to AVTX
probably also needs some modifications.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavcodec/on2avc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Lynne Aug. 5, 2023, 9:15 a.m. UTC | #1
Aug 5, 2023, 10:33 by cus@passwd.hu:

> Improves the audio corruption regression caused by
> 6ba0aa1770ba29eb4126c6a706f6b0cd3809648f reported in ticket #10029.
>
> There is still however a noticable audio glitch, so the FFT conversion to AVTX
> probably also needs some modifications.
>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavcodec/on2avc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c
> index 74be1dcb60..474adb149d 100644
> --- a/libavcodec/on2avc.c
> +++ b/libavcodec/on2avc.c
> @@ -700,7 +700,7 @@ static int on2avc_reconstruct_channel_ext(On2AVCContext *c, AVFrame *dst, int of
>  break;
>  case WINDOW_TYPE_EXT5:
>  c->wtf(c, buf, in, 512);
> -            c->mdct_half_fn(c->mdct, buf + 512, in + 512, sizeof(float));
> +            c->mdct_half_fn(c->mdct_half, buf + 512, in + 512, sizeof(float));
>  for (i = 0; i < 256; i++) {
>  FFSWAP(float, buf[i + 512], buf[1023 - i]);
>  }
>

LGTM
Thanks
Lynne Aug. 5, 2023, 9:38 a.m. UTC | #2
Aug 5, 2023, 11:15 by dev@lynne.ee:

> Aug 5, 2023, 10:33 by cus@passwd.hu:
>
>> Improves the audio corruption regression caused by
>> 6ba0aa1770ba29eb4126c6a706f6b0cd3809648f reported in ticket #10029.
>>
>> There is still however a noticable audio glitch, so the FFT conversion to AVTX
>> probably also needs some modifications.
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavcodec/on2avc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c
>> index 74be1dcb60..474adb149d 100644
>> --- a/libavcodec/on2avc.c
>> +++ b/libavcodec/on2avc.c
>> @@ -700,7 +700,7 @@ static int on2avc_reconstruct_channel_ext(On2AVCContext *c, AVFrame *dst, int of
>>  break;
>>  case WINDOW_TYPE_EXT5:
>>  c->wtf(c, buf, in, 512);
>> -            c->mdct_half_fn(c->mdct, buf + 512, in + 512, sizeof(float));
>> +            c->mdct_half_fn(c->mdct_half, buf + 512, in + 512, sizeof(float));
>>  for (i = 0; i < 256; i++) {
>>  FFSWAP(float, buf[i + 512], buf[1023 - i]);
>>  }
>>
>
> LGTM
> Thanks
>

Actually, looking at it, the WINDOW_TYPE_EXT7 branch, something
weird's going on there as well:

-            c->mdct.imdct_half(&c->mdct, buf, in);
+            c->mdct_fn(c->mdct, buf, in, sizeof(float));

The original misused the context. Maybe this
should be both half as well?
Marton Balint Aug. 5, 2023, 4:52 p.m. UTC | #3
On Sat, 5 Aug 2023, Lynne wrote:

> Aug 5, 2023, 11:15 by dev@lynne.ee:
>
>> Aug 5, 2023, 10:33 by cus@passwd.hu:
>>
>>> Improves the audio corruption regression caused by
>>> 6ba0aa1770ba29eb4126c6a706f6b0cd3809648f reported in ticket #10029.
>>>
>>> There is still however a noticable audio glitch, so the FFT conversion to AVTX
>>> probably also needs some modifications.
>>>
>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>> ---
>>>  libavcodec/on2avc.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c
>>> index 74be1dcb60..474adb149d 100644
>>> --- a/libavcodec/on2avc.c
>>> +++ b/libavcodec/on2avc.c
>>> @@ -700,7 +700,7 @@ static int on2avc_reconstruct_channel_ext(On2AVCContext *c, AVFrame *dst, int of
>>>  break;
>>>  case WINDOW_TYPE_EXT5:
>>>  c->wtf(c, buf, in, 512);
>>> -            c->mdct_half_fn(c->mdct, buf + 512, in + 512, sizeof(float));
>>> +            c->mdct_half_fn(c->mdct_half, buf + 512, in + 512, sizeof(float));
>>>  for (i = 0; i < 256; i++) {
>>>  FFSWAP(float, buf[i + 512], buf[1023 - i]);
>>>  }
>>>
>>
>> LGTM
>> Thanks
>>
>
> Actually, looking at it, the WINDOW_TYPE_EXT7 branch, something
> weird's going on there as well:
>
> -            c->mdct.imdct_half(&c->mdct, buf, in);
> +            c->mdct_fn(c->mdct, buf, in, sizeof(float));
>
> The original misused the context. Maybe this
> should be both half as well?

I thought that is just the old API where you have to use the imdct_half 
function to get the useful half-sized IMDCT. But the old code really means 
to use the 11-bit context to calculate 1024 samples not the 10-bit one 
confusingly called c->mdct_half to calculate 512 samples.

Regards,
Marton
Marton Balint Aug. 8, 2023, 5:35 p.m. UTC | #4
On Sat, 5 Aug 2023, Lynne wrote:

> Aug 5, 2023, 10:33 by cus@passwd.hu:
>
>> Improves the audio corruption regression caused by
>> 6ba0aa1770ba29eb4126c6a706f6b0cd3809648f reported in ticket #10029.
>>
>> There is still however a noticable audio glitch, so the FFT conversion to AVTX
>> probably also needs some modifications.
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavcodec/on2avc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c
>> index 74be1dcb60..474adb149d 100644
>> --- a/libavcodec/on2avc.c
>> +++ b/libavcodec/on2avc.c
>> @@ -700,7 +700,7 @@ static int on2avc_reconstruct_channel_ext(On2AVCContext *c, AVFrame *dst, int of
>>  break;
>>  case WINDOW_TYPE_EXT5:
>>  c->wtf(c, buf, in, 512);
>> -            c->mdct_half_fn(c->mdct, buf + 512, in + 512, sizeof(float));
>> +            c->mdct_half_fn(c->mdct_half, buf + 512, in + 512, sizeof(float));
>>  for (i = 0; i < 256; i++) {
>>  FFSWAP(float, buf[i + 512], buf[1023 - i]);
>>  }
>>
>
> LGTM
> Thanks

Thanks, will apply.

Regards,
Marton
diff mbox series

Patch

diff --git a/libavcodec/on2avc.c b/libavcodec/on2avc.c
index 74be1dcb60..474adb149d 100644
--- a/libavcodec/on2avc.c
+++ b/libavcodec/on2avc.c
@@ -700,7 +700,7 @@  static int on2avc_reconstruct_channel_ext(On2AVCContext *c, AVFrame *dst, int of
             break;
         case WINDOW_TYPE_EXT5:
             c->wtf(c, buf, in, 512);
-            c->mdct_half_fn(c->mdct, buf + 512, in + 512, sizeof(float));
+            c->mdct_half_fn(c->mdct_half, buf + 512, in + 512, sizeof(float));
             for (i = 0; i < 256; i++) {
                 FFSWAP(float, buf[i + 512], buf[1023 - i]);
             }