Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] libavfilter: Fix incorrect ebur128 peak calculation.
@ 2025-11-03 20:38 Carl Hetherington via ffmpeg-devel
  2025-11-04 19:52 ` [FFmpeg-devel] " Nicolas George via ffmpeg-devel
  0 siblings, 1 reply; 7+ messages in thread
From: Carl Hetherington via ffmpeg-devel @ 2025-11-03 20:38 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Carl Hetherington

Since 3b26b782eeded9b9ab7fac013cd1a83a30d68206 it would only look at the
first channel.

Signed-off-by: cth@carlh.net
---
 libavfilter/f_ebur128.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/f_ebur128.c b/libavfilter/f_ebur128.c
index a352f3831f..84d8e44035 100644
--- a/libavfilter/f_ebur128.c
+++ b/libavfilter/f_ebur128.c
@@ -657,7 +657,7 @@ double ff_ebur128_find_peak_c(double *restrict ch_peaks, const int nb_channels,
     for (int ch = 0; ch < nb_channels; ch++) {
         double ch_peak = ch_peaks[ch];
         for (int i = 0; i < nb_samples; i++) {
-            const double sample = fabs(samples[i * nb_channels]);
+            const double sample = fabs(samples[i * nb_channels + ch]);
             ch_peak = FFMAX(ch_peak, sample);
         }
         maxpeak = FFMAX(maxpeak, ch_peak);
-- 
2.51.0

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [FFmpeg-devel] Re: [PATCH] libavfilter: Fix incorrect ebur128 peak calculation.
  2025-11-03 20:38 [FFmpeg-devel] [PATCH] libavfilter: Fix incorrect ebur128 peak calculation Carl Hetherington via ffmpeg-devel
@ 2025-11-04 19:52 ` Nicolas George via ffmpeg-devel
  2025-11-04 21:34   ` Carl Hetherington via ffmpeg-devel
  2025-11-11  2:33   ` Michael Niedermayer via ffmpeg-devel
  0 siblings, 2 replies; 7+ messages in thread
From: Nicolas George via ffmpeg-devel @ 2025-11-04 19:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches
  Cc: Carl Hetherington, Nicolas George

Carl Hetherington via ffmpeg-devel (HE12025-11-03):
> Since 3b26b782eeded9b9ab7fac013cd1a83a30d68206 it would only look at the
> first channel.
> 
> Signed-off-by: cth@carlh.net
> ---
>  libavfilter/f_ebur128.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Hi. Thanks for the patch. I suspect the issue has been here for a longer
time, if only on one branch of the conditional build:

#if CONFIG_SWRESAMPLE
…
                const double sample = fabs(swr_samples[i * nb_channels]);
#endif

versus

            const double *restrict samples_ch = &samples[ch];
…
                const double sample = fabs(samples_ch[nb_channels * i]);

Anyway, the fix looks good to me.

Niklas, it has become mostly your code, what do you make of it?

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [FFmpeg-devel] Re: [PATCH] libavfilter: Fix incorrect ebur128 peak calculation.
  2025-11-04 19:52 ` [FFmpeg-devel] " Nicolas George via ffmpeg-devel
@ 2025-11-04 21:34   ` Carl Hetherington via ffmpeg-devel
  2025-11-11  2:33   ` Michael Niedermayer via ffmpeg-devel
  1 sibling, 0 replies; 7+ messages in thread
From: Carl Hetherington via ffmpeg-devel @ 2025-11-04 21:34 UTC (permalink / raw)
  To: Nicolas George via ffmpeg-devel; +Cc: Nicolas George, Carl Hetherington

On Tue, 4 Nov 2025, Nicolas George via ffmpeg-devel wrote:

> Carl Hetherington via ffmpeg-devel (HE12025-11-03):
> > Since 3b26b782eeded9b9ab7fac013cd1a83a30d68206 it would only look at the
> > first channel.
> >
> > Signed-off-by: cth@carlh.net
> > ---
> >  libavfilter/f_ebur128.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Hi. Thanks for the patch. I suspect the issue has been here for a longer
> time, if only on one branch of the conditional build:

[snip]

Ah, thanks for doing better git archaeology than me!

Best,
Carl
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [FFmpeg-devel] Re: [PATCH] libavfilter: Fix incorrect ebur128 peak calculation.
  2025-11-04 19:52 ` [FFmpeg-devel] " Nicolas George via ffmpeg-devel
  2025-11-04 21:34   ` Carl Hetherington via ffmpeg-devel
@ 2025-11-11  2:33   ` Michael Niedermayer via ffmpeg-devel
  2025-11-17 14:47     ` Tobias Rapp via ffmpeg-devel
  1 sibling, 1 reply; 7+ messages in thread
From: Michael Niedermayer via ffmpeg-devel @ 2025-11-11  2:33 UTC (permalink / raw)
  To: FFmpeg development discussions and patches, Niklas Haas
  Cc: Michael Niedermayer


[-- Attachment #1.1: Type: text/plain, Size: 1243 bytes --]

Hi

adding niklas to the CC so its not missed
but i agree the patch LGTM

On Tue, Nov 04, 2025 at 08:52:36PM +0100, Nicolas George via ffmpeg-devel wrote:
> Carl Hetherington via ffmpeg-devel (HE12025-11-03):
> > Since 3b26b782eeded9b9ab7fac013cd1a83a30d68206 it would only look at the
> > first channel.
> > 
> > Signed-off-by: cth@carlh.net
> > ---
> >  libavfilter/f_ebur128.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Hi. Thanks for the patch. I suspect the issue has been here for a longer
> time, if only on one branch of the conditional build:
> 
> #if CONFIG_SWRESAMPLE
> …
>                 const double sample = fabs(swr_samples[i * nb_channels]);
> #endif
> 
> versus
> 
>             const double *restrict samples_ch = &samples[ch];
> …
>                 const double sample = fabs(samples_ch[nb_channels * i]);
> 
> Anyway, the fix looks good to me.
> 
> Niklas, it has become mostly your code, what do you make of it?

[...]

--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 163 bytes --]

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [FFmpeg-devel] Re: [PATCH] libavfilter: Fix incorrect ebur128 peak calculation.
  2025-11-11  2:33   ` Michael Niedermayer via ffmpeg-devel
@ 2025-11-17 14:47     ` Tobias Rapp via ffmpeg-devel
  2025-11-17 15:01       ` Niklas Haas via ffmpeg-devel
  0 siblings, 1 reply; 7+ messages in thread
From: Tobias Rapp via ffmpeg-devel @ 2025-11-17 14:47 UTC (permalink / raw)
  To: FFmpeg development discussions and patches
  Cc: Michael Niedermayer, Niklas Haas, Tobias Rapp

On 11/11/2025 03:33, Michael Niedermayer via ffmpeg-devel wrote:

> Hi
>
> adding niklas to the CC so its not missed
> but i agree the patch LGTM
>
> On Tue, Nov 04, 2025 at 08:52:36PM +0100, Nicolas George via ffmpeg-devel wrote:
>> Carl Hetherington via ffmpeg-devel (HE12025-11-03):
>>> Since 3b26b782eeded9b9ab7fac013cd1a83a30d68206 it would only look at the
>>> first channel.
>>>
>>> Signed-off-by: cth@carlh.net
>>> ---
>>>   libavfilter/f_ebur128.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> Hi. Thanks for the patch. I suspect the issue has been here for a longer
>> time, if only on one branch of the conditional build:
>>
>> #if CONFIG_SWRESAMPLE
>> …
>>                  const double sample = fabs(swr_samples[i * nb_channels]);
>> #endif
>>
>> versus
>>
>>              const double *restrict samples_ch = &samples[ch];
>> …
>>                  const double sample = fabs(samples_ch[nb_channels * i]);
>>
>> Anyway, the fix looks good to me.
>>
>> Niklas, it has become mostly your code, what do you make of it?

Now that n8.0.1 will be tagged soonish it would be great if this fix 
could be included.

Regards, Tobias

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [FFmpeg-devel] Re: [PATCH] libavfilter: Fix incorrect ebur128 peak calculation.
  2025-11-17 14:47     ` Tobias Rapp via ffmpeg-devel
@ 2025-11-17 15:01       ` Niklas Haas via ffmpeg-devel
  2025-11-18  8:04         ` Tobias Rapp via ffmpeg-devel
  0 siblings, 1 reply; 7+ messages in thread
From: Niklas Haas via ffmpeg-devel @ 2025-11-17 15:01 UTC (permalink / raw)
  To: FFmpeg development discussions and patches
  Cc: Michael Niedermayer, Tobias Rapp, Niklas Haas


On Monday, November 17th, 2025 at 3:48 PM, Tobias Rapp via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
> 
> 
> On 11/11/2025 03:33, Michael Niedermayer via ffmpeg-devel wrote:
> 
> > Hi
> > 
> > adding niklas to the CC so its not missed
> > but i agree the patch LGTM
> > 
> > On Tue, Nov 04, 2025 at 08:52:36PM +0100, Nicolas George via ffmpeg-devel wrote:
> > 
> > > Carl Hetherington via ffmpeg-devel (HE12025-11-03):
> > > 
> > > > Since 3b26b782eeded9b9ab7fac013cd1a83a30d68206 it would only look at the
> > > > first channel.
> > > > 
> > > > Signed-off-by: cth@carlh.net
> > > > ---
> > > > libavfilter/f_ebur128.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > Hi. Thanks for the patch. I suspect the issue has been here for a longer
> > > > time, if only on one branch of the conditional build:
> > > 
> > > #if CONFIG_SWRESAMPLE
> > > …
> > > const double sample = fabs(swr_samples[i * nb_channels]);
> > > #endif
> > > 
> > > versus
> > > 
> > > const double *restrict samples_ch = &samples[ch];
> > > …
> > > const double sample = fabs(samples_ch[nb_channels * i]);
> > > 
> > > Anyway, the fix looks good to me.
> > > 
> > > Niklas, it has become mostly your code, what do you make of it?
> 
> 
> Now that n8.0.1 will be tagged soonish it would be great if this fix
> could be included.
> 
> Regards, Tobias

Hi Tobias,

Sorry for the delay in responding. I agree that this seems to be a bug that has been around for quite a while, and that your fix seems correct.

I also don't suspect the performance will be impacted as a result, unless the compiler is really very clever to eliminate the common subcalculation by reassociating the FFMAX calls.

Thanks,
Niklas

P.s. In the future, it's probably faster if you submit patches directly on https://code.ffmpeg.org/

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [FFmpeg-devel] Re: [PATCH] libavfilter: Fix incorrect ebur128 peak calculation.
  2025-11-17 15:01       ` Niklas Haas via ffmpeg-devel
@ 2025-11-18  8:04         ` Tobias Rapp via ffmpeg-devel
  0 siblings, 0 replies; 7+ messages in thread
From: Tobias Rapp via ffmpeg-devel @ 2025-11-18  8:04 UTC (permalink / raw)
  To: FFmpeg development discussions and patches
  Cc: Michael Niedermayer, Carl Hetherington, Niklas Haas, Tobias Rapp

On 17/11/2025 16:01, Niklas Haas wrote:

> On Monday, November 17th, 2025 at 3:48 PM, Tobias Rapp via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
>>
>> On 11/11/2025 03:33, Michael Niedermayer via ffmpeg-devel wrote:
>>
>>> Hi
>>>
>>> adding niklas to the CC so its not missed
>>> but i agree the patch LGTM
>>>
>>> On Tue, Nov 04, 2025 at 08:52:36PM +0100, Nicolas George via ffmpeg-devel wrote:
>>>
>>>> Carl Hetherington via ffmpeg-devel (HE12025-11-03):
>>>>
>>>>> Since 3b26b782eeded9b9ab7fac013cd1a83a30d68206 it would only look at the
>>>>> first channel.
>>>>>
>>>>> Signed-off-by: cth@carlh.net
>>>>> ---
>>>>> libavfilter/f_ebur128.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> Hi. Thanks for the patch. I suspect the issue has been here for a longer
>>>>> time, if only on one branch of the conditional build:
>>>> #if CONFIG_SWRESAMPLE
>>>> …
>>>> const double sample = fabs(swr_samples[i * nb_channels]);
>>>> #endif
>>>>
>>>> versus
>>>>
>>>> const double *restrict samples_ch = &samples[ch];
>>>> …
>>>> const double sample = fabs(samples_ch[nb_channels * i]);
>>>>
>>>> Anyway, the fix looks good to me.
>>>>
>>>> Niklas, it has become mostly your code, what do you make of it?
>>
>> Now that n8.0.1 will be tagged soonish it would be great if this fix
>> could be included.
>>
>> Regards, Tobias
> Hi Tobias,
>
> Sorry for the delay in responding. I agree that this seems to be a bug that has been around for quite a while, and that your fix seems correct.
>
> I also don't suspect the performance will be impacted as a result, unless the compiler is really very clever to eliminate the common subcalculation by reassociating the FFMAX calls.
>
> Thanks,
> Niklas
>
> P.s. In the future, it's probably faster if you submit patches directly on https://code.ffmpeg.org/

I'm not the author of the patch, just a random person that did a ping :-)

Have added you as a reviewed-by to the commit message and pushed the 
patch to master and release/8.0. Thanks Carl for the patch, and thanks 
to Nicolas, Michael and Niklas for taking a look.

Regards, Tobias

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-11-18  8:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-03 20:38 [FFmpeg-devel] [PATCH] libavfilter: Fix incorrect ebur128 peak calculation Carl Hetherington via ffmpeg-devel
2025-11-04 19:52 ` [FFmpeg-devel] " Nicolas George via ffmpeg-devel
2025-11-04 21:34   ` Carl Hetherington via ffmpeg-devel
2025-11-11  2:33   ` Michael Niedermayer via ffmpeg-devel
2025-11-17 14:47     ` Tobias Rapp via ffmpeg-devel
2025-11-17 15:01       ` Niklas Haas via ffmpeg-devel
2025-11-18  8:04         ` Tobias Rapp via ffmpeg-devel

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git