One pass volume normalization (ebur128)
I am once again proposing a patch for one pass volume normalization base on ebur128, as I see this still did not make it into FFMpeg 2.0. My patch is heaveily based on Clément Bsch's proposal in http://permalink.gmane.org/gmane.comp.video.ffmpeg.devel/159978 We have been using this patch now for more than 4 months and 1800+ videos of approximately 1 hour have been transcoded with it. Part of our FFMpeg commandline reads as -filter_complex \ "[0:v]setpts=PTS-STARTPTS[v0];[0:a]asetpts=PTS-STARTPTS,ebur128=metadata=1,volume=metadata=lavfi.r128.I,ebur128[a0]" \ -map [v0] -map [a0] It uses the already present ebur128 meta injection to adjust the volume on the fly. What would be the objection to move this into the FFMpeg core, so I do not have to patch my FFMpeg every time I compile a new one? I applied the patch below to FFMpeg Release/v.2.0. Jan diff --git a/libavfilter/af_volume.c b/libavfilter/af_volume.c index a2ac1e2..6372bb2 100644 --- a/libavfilter/af_volume.c +++ b/libavfilter/af_volume.c @@ -51,18 +51,24 @@ static const AVOption volume_options[] = { { "fixed", "select 8-bit fixed-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FIXED }, INT_MIN, INT_MAX, A|F, "precision" }, { "float", "select 32-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FLOAT }, INT_MIN, INT_MAX, A|F, "precision" }, { "double", "select 64-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_DOUBLE }, INT_MIN, INT_MAX, A|F, "precision" }, + { "metadata", "set the metadata key for loudness normalization", OFFSET(metadata), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = A|F }, { NULL }, }; AVFILTER_DEFINE_CLASS(volume); +static void set_fixed_volume(VolumeContext *vol, double volume) +{ + vol->volume_i = (int)(volume * 256 + 0.5); + vol->volume = vol->volume_i / 256.0; +} + static av_cold int init(AVFilterContext *ctx) { VolumeContext *vol = ctx->priv; if (vol->precision == PRECISION_FIXED) { - vol->volume_i = (int)(vol->volume * 256 + 0.5); - vol->volume = vol->volume_i / 256.0; + set_fixed_volume(vol, vol->volume); av_log(ctx, AV_LOG_VERBOSE, "volume:(%d/256)(%f)(%1.2fdB) precision:fixed\n", vol->volume_i, vol->volume, 20.0*log(vol->volume)/M_LN10); } else { @@ -171,13 +177,13 @@ static av_cold void volume_init(VolumeContext *vol) switch (av_get_packed_sample_fmt(vol->sample_fmt)) { case AV_SAMPLE_FMT_U8: - if (vol->volume_i < 0x1000000) + if (vol->volume_i < 0x1000000 && !vol->metadata) vol->scale_samples = scale_samples_u8_small; else vol->scale_samples = scale_samples_u8; break; case AV_SAMPLE_FMT_S16: - if (vol->volume_i < 0x10000) + if (vol->volume_i < 0x10000 && !vol->metadata) vol->scale_samples = scale_samples_s16_small; else vol->scale_samples = scale_samples_s16; @@ -216,11 +222,30 @@ static int config_output(AVFilterLink *outlink) static int filter_frame(AVFilterLink *inlink, AVFrame *buf) { - VolumeContext *vol = inlink->dst->priv; - AVFilterLink *outlink = inlink->dst->outputs[0]; + AVFilterContext *ctx = inlink->dst; + VolumeContext *vol = ctx->priv; + AVFilterLink *outlink = ctx->outputs[0]; int nb_samples = buf->nb_samples; AVFrame *out_buf; + if (vol->metadata) { + double loudness, new_volume, timestamp, mx; + AVDictionaryEntry *e; + mx = 20; + timestamp = (float)(1.0 * buf->pts / outlink->sample_rate); + mx = fmin(mx, timestamp); + e = av_dict_get(buf->metadata, vol->metadata, NULL, 0); + if (e) { + loudness = av_strtod(e->value, NULL); + if (loudness > -69) { + new_volume = fmax(-mx,fmin(mx,(-23 - loudness))); + av_log(NULL, AV_LOG_VERBOSE, "loudness=%f => %f => volume=%f\n", + loudness, new_volume, pow(10, new_volume / 20)); + set_fixed_volume(vol, pow(10, new_volume / 20)); + } + } + } + if (vol->volume == 1.0 || vol->volume_i == 256) return ff_filter_frame(outlink, buf); @@ -269,6 +294,12 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf) return ff_filter_frame(outlink, out_buf); } +static av_cold void uninit(AVFilterContext *ctx) +{ + VolumeContext *vol = ctx->priv; + av_opt_free(vol); +} + static const AVFilterPad avfilter_af_volume_inputs[] = { { .name = "default", @@ -294,6 +325,7 @@ AVFilter avfilter_af_volume = { .priv_size = sizeof(VolumeContext), .priv_class = &volume_class, .init = init, + .uninit = uninit, .inputs = avfilter_af_volume_inputs, .outputs = avfilter_af_volume_outputs, .flags = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC, diff --git a/libavfilter/af_volume.h b/libavfilter/af_volume.h index bd7932e..4deca9c 100644 --- a/libavfilter/af_volume.h +++ b/libavfilter/af_volume.h @@ -48,6 +48,7 @@ typedef struct VolumeContext { void (*scale_samples)(uint8_t *dst, const uint8_t *src, int nb_samples, int volume); int samples_align; + char *metadata; } VolumeContext; void ff_volume_init_x86(VolumeContext *vol); diff --git a/libavfilter/f_ebur128.c b/libavfilter/f_ebur128.c index 88d37e8..f4ce6d9 100644 --- a/libavfilter/f_ebur128.c +++ b/libavfilter/f_ebur128.c @@ -410,7 +410,7 @@ static av_cold int init(AVFilterContext *ctx) if (ebur128->loglevel != AV_LOG_INFO && ebur128->loglevel != AV_LOG_VERBOSE) { - if (ebur128->do_video || ebur128->metadata) + if (ebur128->do_video) ebur128->loglevel = AV_LOG_VERBOSE; else ebur128->loglevel = AV_LOG_INFO; @@ -689,7 +689,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *insamples) SET_META("LRA.high", ebur128->lra_high); } - av_log(ctx, ebur128->loglevel, "t: %-10s " LOG_FMT "\n", + av_log(ctx, ebur128->metadata || !ebur128->do_video ? AV_LOG_VERBOSE : ebur128->loglevel, "t: %-10s " LOG_FMT "\n", av_ts2timestr(pts, &outlink->time_base), loudness_400, loudness_3000, ebur128->integrated_loudness, ebur128->loudness_range); diff --git a/libavfilter/x86/af_volume_init.c b/libavfilter/x86/af_volume_init.c index 81d605f..fab5a03 100644 --- a/libavfilter/x86/af_volume_init.c +++ b/libavfilter/x86/af_volume_init.c @@ -39,7 +39,7 @@ av_cold void ff_volume_init_x86(VolumeContext *vol) enum AVSampleFormat sample_fmt = av_get_packed_sample_fmt(vol->sample_fmt); if (sample_fmt == AV_SAMPLE_FMT_S16) { - if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768) { + if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768 && !vol->metadata) { vol->scale_samples = ff_scale_samples_s16_sse2; vol->samples_align = 8; }
On 7/13/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
I am once again proposing a patch for one pass volume normalization base on ebur128, as I see this still did not make it into FFMpeg 2.0. My patch is heaveily based on Clement Boesch's proposal in http://permalink.gmane.org/gmane.comp.video.ffmpeg.devel/159978
We have been using this patch now for more than 4 months and 1800+ videos of approximately 1 hour have been transcoded with it.
Part of our FFMpeg commandline reads as -filter_complex \
"[0:v]setpts=PTS-STARTPTS[v0];[0:a]asetpts=PTS-STARTPTS,ebur128=metadata=1,volume=metadata=lavfi.r128.I,ebur128[a0]" \ -map [v0] -map [a0]
It uses the already present ebur128 meta injection to adjust the volume on the fly. What would be the objection to move this into the FFMpeg core, so I do not have to patch my FFMpeg every time I compile a new one? I applied the patch below to FFMpeg Release/v.2.0.
Jan
diff --git a/libavfilter/af_volume.c b/libavfilter/af_volume.c index a2ac1e2..6372bb2 100644 --- a/libavfilter/af_volume.c +++ b/libavfilter/af_volume.c @@ -51,18 +51,24 @@ static const AVOption volume_options[] = { { "fixed", "select 8-bit fixed-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FIXED }, INT_MIN, INT_MAX, A|F, "precision" }, { "float", "select 32-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FLOAT }, INT_MIN, INT_MAX, A|F, "precision" }, { "double", "select 64-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_DOUBLE }, INT_MIN, INT_MAX, A|F, "precision" }, + { "metadata", "set the metadata key for loudness normalization", OFFSET(metadata), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = A|F }, { NULL }, };
AVFILTER_DEFINE_CLASS(volume);
+static void set_fixed_volume(VolumeContext *vol, double volume) +{ + vol->volume_i = (int)(volume * 256 + 0.5); + vol->volume = vol->volume_i / 256.0; +} + static av_cold int init(AVFilterContext *ctx) { VolumeContext *vol = ctx->priv;
if (vol->precision == PRECISION_FIXED) { - vol->volume_i = (int)(vol->volume * 256 + 0.5); - vol->volume = vol->volume_i / 256.0; + set_fixed_volume(vol, vol->volume); av_log(ctx, AV_LOG_VERBOSE, "volume:(%d/256)(%f)(%1.2fdB) precision:fixed\n", vol->volume_i, vol->volume, 20.0*log(vol->volume)/M_LN10); } else { @@ -171,13 +177,13 @@ static av_cold void volume_init(VolumeContext *vol)
switch (av_get_packed_sample_fmt(vol->sample_fmt)) { case AV_SAMPLE_FMT_U8: - if (vol->volume_i < 0x1000000) + if (vol->volume_i < 0x1000000 && !vol->metadata) vol->scale_samples = scale_samples_u8_small; else vol->scale_samples = scale_samples_u8; break; case AV_SAMPLE_FMT_S16: - if (vol->volume_i < 0x10000) + if (vol->volume_i < 0x10000 && !vol->metadata) vol->scale_samples = scale_samples_s16_small; else vol->scale_samples = scale_samples_s16; @@ -216,11 +222,30 @@ static int config_output(AVFilterLink *outlink)
static int filter_frame(AVFilterLink *inlink, AVFrame *buf) { - VolumeContext *vol = inlink->dst->priv; - AVFilterLink *outlink = inlink->dst->outputs[0]; + AVFilterContext *ctx = inlink->dst; + VolumeContext *vol = ctx->priv; + AVFilterLink *outlink = ctx->outputs[0]; int nb_samples = buf->nb_samples; AVFrame *out_buf;
+ if (vol->metadata) { + double loudness, new_volume, timestamp, mx; + AVDictionaryEntry *e; + mx = 20; + timestamp = (float)(1.0 * buf->pts / outlink->sample_rate); + mx = fmin(mx, timestamp); + e = av_dict_get(buf->metadata, vol->metadata, NULL, 0); + if (e) { + loudness = av_strtod(e->value, NULL); + if (loudness > -69) { + new_volume = fmax(-mx,fmin(mx,(-23 - loudness))); + av_log(NULL, AV_LOG_VERBOSE, "loudness=%f => %f => volume=%f\n", + loudness, new_volume, pow(10, new_volume / 20)); + set_fixed_volume(vol, pow(10, new_volume / 20)); + } + } + } + if (vol->volume == 1.0 || vol->volume_i == 256) return ff_filter_frame(outlink, buf);
@@ -269,6 +294,12 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf) return ff_filter_frame(outlink, out_buf); }
+static av_cold void uninit(AVFilterContext *ctx) +{ + VolumeContext *vol = ctx->priv; + av_opt_free(vol); +} + static const AVFilterPad avfilter_af_volume_inputs[] = { { .name = "default", @@ -294,6 +325,7 @@ AVFilter avfilter_af_volume = { .priv_size = sizeof(VolumeContext), .priv_class = &volume_class, .init = init, + .uninit = uninit, .inputs = avfilter_af_volume_inputs, .outputs = avfilter_af_volume_outputs, .flags = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC, diff --git a/libavfilter/af_volume.h b/libavfilter/af_volume.h index bd7932e..4deca9c 100644 --- a/libavfilter/af_volume.h +++ b/libavfilter/af_volume.h @@ -48,6 +48,7 @@ typedef struct VolumeContext { void (*scale_samples)(uint8_t *dst, const uint8_t *src, int nb_samples, int volume); int samples_align; + char *metadata; } VolumeContext;
void ff_volume_init_x86(VolumeContext *vol); diff --git a/libavfilter/f_ebur128.c b/libavfilter/f_ebur128.c index 88d37e8..f4ce6d9 100644 --- a/libavfilter/f_ebur128.c +++ b/libavfilter/f_ebur128.c @@ -410,7 +410,7 @@ static av_cold int init(AVFilterContext *ctx)
if (ebur128->loglevel != AV_LOG_INFO && ebur128->loglevel != AV_LOG_VERBOSE) { - if (ebur128->do_video || ebur128->metadata) + if (ebur128->do_video) ebur128->loglevel = AV_LOG_VERBOSE; else ebur128->loglevel = AV_LOG_INFO; @@ -689,7 +689,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *insamples) SET_META("LRA.high", ebur128->lra_high); }
- av_log(ctx, ebur128->loglevel, "t: %-10s " LOG_FMT "\n", + av_log(ctx, ebur128->metadata || !ebur128->do_video ? AV_LOG_VERBOSE : ebur128->loglevel, "t: %-10s " LOG_FMT "\n", av_ts2timestr(pts, &outlink->time_base), loudness_400, loudness_3000, ebur128->integrated_loudness, ebur128->loudness_range); diff --git a/libavfilter/x86/af_volume_init.c b/libavfilter/x86/af_volume_init.c index 81d605f..fab5a03 100644 --- a/libavfilter/x86/af_volume_init.c +++ b/libavfilter/x86/af_volume_init.c @@ -39,7 +39,7 @@ av_cold void ff_volume_init_x86(VolumeContext *vol) enum AVSampleFormat sample_fmt = av_get_packed_sample_fmt(vol->sample_fmt);
if (sample_fmt == AV_SAMPLE_FMT_S16) { - if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768) { + if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768 &&
Why? This is suboptimal.
!vol->metadata) { vol->scale_samples = ff_scale_samples_s16_sse2; vol->samples_align = 8; }
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Paul B Mahol in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 19:28:05 +0000):
On 7/13/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
--- a/libavfilter/x86/af_volume_init.c +++ b/libavfilter/x86/af_volume_init.c @@ -39,7 +39,7 @@ av_cold void ff_volume_init_x86(VolumeContext *vol) enum AVSampleFormat sample_fmt = av_get_packed_sample_fmt(vol->sample_fmt);
if (sample_fmt == AV_SAMPLE_FMT_S16) { - if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768) { + if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768 &&
Why? This is suboptimal.
I really would not know. It was already there in Clement's patch at http://permalink.gmane.org/gmane.comp.video.ffmpeg.devel/159978 Feel free to make it optimal. Jan
On 7/13/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
Paul B Mahol in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 19:28:05 +0000):
On 7/13/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
--- a/libavfilter/x86/af_volume_init.c +++ b/libavfilter/x86/af_volume_init.c @@ -39,7 +39,7 @@ av_cold void ff_volume_init_x86(VolumeContext *vol) enum AVSampleFormat sample_fmt = av_get_packed_sample_fmt(vol->sample_fmt);
if (sample_fmt == AV_SAMPLE_FMT_S16) { - if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768) { + if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768 &&
Why? This is suboptimal.
I really would not know. It was already there in Clement's patch at http://permalink.gmane.org/gmane.comp.video.ffmpeg.devel/159978
Feel free to make it optimal.
But you could remove that hunk and test it.
Paul B Mahol in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 22:42:46 +0000):
On 7/13/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
I really would not know. It was already there in Clement's patch at http://permalink.gmane.org/gmane.comp.video.ffmpeg.devel/159978
Feel free to make it optimal.
But you could remove that hunk and test it.
Clement must have had a valid reason to put it there (February this year). I could of course test it on my videos, but there is no guarantee my test videos fall into the right category. What has changed in the last few months? Jan
On 7/13/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
Paul B Mahol in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 22:42:46 +0000):
On 7/13/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
I really would not know. It was already there in Clement's patch at http://permalink.gmane.org/gmane.comp.video.ffmpeg.devel/159978
Feel free to make it optimal.
But you could remove that hunk and test it.
Clement must have had a valid reason to put it there (February this year). I could of course test it on my videos, but there is no guarantee my test videos fall into the right category. What has changed in the last few months?
I see no valid reasons for such unrelated change in patch. So original objection.
Le quintidi 25 messidor, an CCXXI, Jan Ehrhardt a écrit :
Subject: [FFmpeg-devel] One pass volume normalization (ebur128)
Single-pass volume normalization is not possible, please do not call the feature that way.
I am once again proposing a patch for one pass volume normalization base on ebur128, as I see this still did not make it into FFMpeg 2.0. My patch is heaveily based on Clément Bœsch's proposal in http://permalink.gmane.org/gmane.comp.video.ffmpeg.devel/159978
We have been using this patch now for more than 4 months and 1800+ videos of approximately 1 hour have been transcoded with it.
Part of our FFMpeg commandline reads as -filter_complex \ "[0:v]setpts=PTS-STARTPTS[v0];[0:a]asetpts=PTS-STARTPTS,ebur128=metadata=1,volume=metadata=lavfi.r128.I,ebur128[a0]" \ -map [v0] -map [a0]
r128.I is not a good choice, but there is nothing better yet.
It uses the already present ebur128 meta injection to adjust the volume on the fly. What would be the objection to move this into the FFMpeg core, so I do not have to patch my FFMpeg every time I compile a new one? I applied the patch below to FFMpeg Release/v.2.0.
Jan
diff --git a/libavfilter/af_volume.c b/libavfilter/af_volume.c index a2ac1e2..6372bb2 100644 --- a/libavfilter/af_volume.c +++ b/libavfilter/af_volume.c
Missing documentation update.
@@ -51,18 +51,24 @@ static const AVOption volume_options[] = { { "fixed", "select 8-bit fixed-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FIXED }, INT_MIN, INT_MAX, A|F, "precision" }, { "float", "select 32-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FLOAT }, INT_MIN, INT_MAX, A|F, "precision" }, { "double", "select 64-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_DOUBLE }, INT_MIN, INT_MAX, A|F, "precision" },
+ { "metadata", "set the metadata key for loudness normalization", OFFSET(metadata), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = A|F },
Inconsistent indentation.
{ NULL }, };
AVFILTER_DEFINE_CLASS(volume);
+static void set_fixed_volume(VolumeContext *vol, double volume) +{ + vol->volume_i = (int)(volume * 256 + 0.5); + vol->volume = vol->volume_i / 256.0; +} + static av_cold int init(AVFilterContext *ctx) { VolumeContext *vol = ctx->priv;
if (vol->precision == PRECISION_FIXED) { - vol->volume_i = (int)(vol->volume * 256 + 0.5); - vol->volume = vol->volume_i / 256.0; + set_fixed_volume(vol, vol->volume); av_log(ctx, AV_LOG_VERBOSE, "volume:(%d/256)(%f)(%1.2fdB) precision:fixed\n", vol->volume_i, vol->volume, 20.0*log(vol->volume)/M_LN10); } else { @@ -171,13 +177,13 @@ static av_cold void volume_init(VolumeContext *vol)
switch (av_get_packed_sample_fmt(vol->sample_fmt)) { case AV_SAMPLE_FMT_U8: - if (vol->volume_i < 0x1000000) + if (vol->volume_i < 0x1000000 && !vol->metadata) vol->scale_samples = scale_samples_u8_small; else vol->scale_samples = scale_samples_u8; break; case AV_SAMPLE_FMT_S16: - if (vol->volume_i < 0x10000) + if (vol->volume_i < 0x10000 && !vol->metadata) vol->scale_samples = scale_samples_s16_small; else vol->scale_samples = scale_samples_s16; @@ -216,11 +222,30 @@ static int config_output(AVFilterLink *outlink)
static int filter_frame(AVFilterLink *inlink, AVFrame *buf) { - VolumeContext *vol = inlink->dst->priv; - AVFilterLink *outlink = inlink->dst->outputs[0]; + AVFilterContext *ctx = inlink->dst; + VolumeContext *vol = ctx->priv; + AVFilterLink *outlink = ctx->outputs[0]; int nb_samples = buf->nb_samples; AVFrame *out_buf;
+ if (vol->metadata) { + double loudness, new_volume, timestamp, mx; + AVDictionaryEntry *e; + mx = 20; + timestamp = (float)(1.0 * buf->pts / outlink->sample_rate); + mx = fmin(mx, timestamp); + e = av_dict_get(buf->metadata, vol->metadata, NULL, 0); + if (e) { + loudness = av_strtod(e->value, NULL);
+ if (loudness > -69) { + new_volume = fmax(-mx,fmin(mx,(-23 - loudness))); + av_log(NULL, AV_LOG_VERBOSE, "loudness=%f => %f => volume=%f\n", + loudness, new_volume, pow(10, new_volume / 20)); + set_fixed_volume(vol, pow(10, new_volume / 20)); + }
This paragraph has several problems. First, it is missing spaces around words, that is easy to fix. Second, it has a duplicated mathematical formula, which is pretty much a recipe for inconsistency. That is easy to fix too. Third, it has several hardcoded values, and that is not good design. It seems to me that using an expression, evaluated each time the metadata value changes and with that value available as a variable would be a much nicer design.
+ } + } + if (vol->volume == 1.0 || vol->volume_i == 256) return ff_filter_frame(outlink, buf);
@@ -269,6 +294,12 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf) return ff_filter_frame(outlink, out_buf); }
+static av_cold void uninit(AVFilterContext *ctx) +{ + VolumeContext *vol = ctx->priv; + av_opt_free(vol); +}
AFAIK, this is unneeded since the "evil plan".
+ static const AVFilterPad avfilter_af_volume_inputs[] = { { .name = "default", @@ -294,6 +325,7 @@ AVFilter avfilter_af_volume = { .priv_size = sizeof(VolumeContext), .priv_class = &volume_class, .init = init, + .uninit = uninit, .inputs = avfilter_af_volume_inputs, .outputs = avfilter_af_volume_outputs, .flags = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC, diff --git a/libavfilter/af_volume.h b/libavfilter/af_volume.h index bd7932e..4deca9c 100644 --- a/libavfilter/af_volume.h +++ b/libavfilter/af_volume.h @@ -48,6 +48,7 @@ typedef struct VolumeContext { void (*scale_samples)(uint8_t *dst, const uint8_t *src, int nb_samples, int volume); int samples_align; + char *metadata; } VolumeContext;
void ff_volume_init_x86(VolumeContext *vol);
diff --git a/libavfilter/f_ebur128.c b/libavfilter/f_ebur128.c index 88d37e8..f4ce6d9 100644 --- a/libavfilter/f_ebur128.c +++ b/libavfilter/f_ebur128.c
Unrelated.
@@ -410,7 +410,7 @@ static av_cold int init(AVFilterContext *ctx)
if (ebur128->loglevel != AV_LOG_INFO && ebur128->loglevel != AV_LOG_VERBOSE) {
- if (ebur128->do_video || ebur128->metadata) + if (ebur128->do_video) ebur128->loglevel = AV_LOG_VERBOSE; else ebur128->loglevel = AV_LOG_INFO; @@ -689,7 +689,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *insamples) SET_META("LRA.high", ebur128->lra_high); }
- av_log(ctx, ebur128->loglevel, "t: %-10s " LOG_FMT "\n", + av_log(ctx, ebur128->metadata || !ebur128->do_video ? AV_LOG_VERBOSE : ebur128->loglevel, "t: %-10s " LOG_FMT "\n", av_ts2timestr(pts, &outlink->time_base), loudness_400, loudness_3000, ebur128->integrated_loudness, ebur128->loudness_range); diff --git a/libavfilter/x86/af_volume_init.c b/libavfilter/x86/af_volume_init.c index 81d605f..fab5a03 100644 --- a/libavfilter/x86/af_volume_init.c +++ b/libavfilter/x86/af_volume_init.c @@ -39,7 +39,7 @@ av_cold void ff_volume_init_x86(VolumeContext *vol) enum AVSampleFormat sample_fmt = av_get_packed_sample_fmt(vol->sample_fmt);
if (sample_fmt == AV_SAMPLE_FMT_S16) { - if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768) { + if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768 && !vol->metadata) { vol->scale_samples = ff_scale_samples_s16_sse2; vol->samples_align = 8; }
Regards, -- Nicolas George
On 7/13/13, Nicolas George <nicolas.george@normalesup.org> wrote:
Le quintidi 25 messidor, an CCXXI, Jan Ehrhardt a ecrit :
Subject: [FFmpeg-devel] One pass volume normalization (ebur128)
Single-pass volume normalization is not possible, please do not call the feature that way.
I am once again proposing a patch for one pass volume normalization base on ebur128, as I see this still did not make it into FFMpeg 2.0. My patch is heaveily based on Clement Boesch's proposal in http://permalink.gmane.org/gmane.comp.video.ffmpeg.devel/159978
We have been using this patch now for more than 4 months and 1800+ videos of approximately 1 hour have been transcoded with it.
Part of our FFMpeg commandline reads as -filter_complex \
"[0:v]setpts=PTS-STARTPTS[v0];[0:a]asetpts=PTS-STARTPTS,ebur128=metadata=1,volume=metadata=lavfi.r128.I,ebur128[a0]" \ -map [v0] -map [a0]
r128.I is not a good choice, but there is nothing better yet.
It uses the already present ebur128 meta injection to adjust the volume on the fly. What would be the objection to move this into the FFMpeg core, so I do not have to patch my FFMpeg every time I compile a new one? I applied the patch below to FFMpeg Release/v.2.0.
Jan
diff --git a/libavfilter/af_volume.c b/libavfilter/af_volume.c index a2ac1e2..6372bb2 100644 --- a/libavfilter/af_volume.c +++ b/libavfilter/af_volume.c
Missing documentation update.
@@ -51,18 +51,24 @@ static const AVOption volume_options[] = { { "fixed", "select 8-bit fixed-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FIXED }, INT_MIN, INT_MAX, A|F, "precision" }, { "float", "select 32-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FLOAT }, INT_MIN, INT_MAX, A|F, "precision" }, { "double", "select 64-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_DOUBLE }, INT_MIN, INT_MAX, A|F, "precision" },
+ { "metadata", "set the metadata key for loudness normalization", OFFSET(metadata), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = A|F },
Inconsistent indentation.
Identation is fine.
{ NULL }, };
AVFILTER_DEFINE_CLASS(volume);
+static void set_fixed_volume(VolumeContext *vol, double volume) +{ + vol->volume_i = (int)(volume * 256 + 0.5); + vol->volume = vol->volume_i / 256.0; +} + static av_cold int init(AVFilterContext *ctx) { VolumeContext *vol = ctx->priv;
if (vol->precision == PRECISION_FIXED) { - vol->volume_i = (int)(vol->volume * 256 + 0.5); - vol->volume = vol->volume_i / 256.0; + set_fixed_volume(vol, vol->volume); av_log(ctx, AV_LOG_VERBOSE, "volume:(%d/256)(%f)(%1.2fdB) precision:fixed\n", vol->volume_i, vol->volume, 20.0*log(vol->volume)/M_LN10); } else { @@ -171,13 +177,13 @@ static av_cold void volume_init(VolumeContext *vol)
switch (av_get_packed_sample_fmt(vol->sample_fmt)) { case AV_SAMPLE_FMT_U8: - if (vol->volume_i < 0x1000000) + if (vol->volume_i < 0x1000000 && !vol->metadata) vol->scale_samples = scale_samples_u8_small; else vol->scale_samples = scale_samples_u8; break; case AV_SAMPLE_FMT_S16: - if (vol->volume_i < 0x10000) + if (vol->volume_i < 0x10000 && !vol->metadata) vol->scale_samples = scale_samples_s16_small; else vol->scale_samples = scale_samples_s16; @@ -216,11 +222,30 @@ static int config_output(AVFilterLink *outlink)
static int filter_frame(AVFilterLink *inlink, AVFrame *buf) { - VolumeContext *vol = inlink->dst->priv; - AVFilterLink *outlink = inlink->dst->outputs[0]; + AVFilterContext *ctx = inlink->dst; + VolumeContext *vol = ctx->priv; + AVFilterLink *outlink = ctx->outputs[0]; int nb_samples = buf->nb_samples; AVFrame *out_buf;
+ if (vol->metadata) { + double loudness, new_volume, timestamp, mx; + AVDictionaryEntry *e; + mx = 20; + timestamp = (float)(1.0 * buf->pts / outlink->sample_rate); + mx = fmin(mx, timestamp); + e = av_dict_get(buf->metadata, vol->metadata, NULL, 0); + if (e) { + loudness = av_strtod(e->value, NULL);
+ if (loudness > -69) { + new_volume = fmax(-mx,fmin(mx,(-23 - loudness))); + av_log(NULL, AV_LOG_VERBOSE, "loudness=%f => %f => volume=%f\n", + loudness, new_volume, pow(10, new_volume / 20)); + set_fixed_volume(vol, pow(10, new_volume / 20)); + }
This paragraph has several problems. First, it is missing spaces around words, that is easy to fix.
Second, it has a duplicated mathematical formula, which is pretty much a recipe for inconsistency. That is easy to fix too.
Third, it has several hardcoded values, and that is not good design.
It seems to me that using an expression, evaluated each time the metadata value changes and with that value available as a variable would be a much nicer design.
Four, it is using NULL for log context.
+ } + } + if (vol->volume == 1.0 || vol->volume_i == 256) return ff_filter_frame(outlink, buf);
@@ -269,6 +294,12 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf) return ff_filter_frame(outlink, out_buf); }
+static av_cold void uninit(AVFilterContext *ctx) +{ + VolumeContext *vol = ctx->priv; + av_opt_free(vol); +}
AFAIK, this is unneeded since the "evil plan".
+ static const AVFilterPad avfilter_af_volume_inputs[] = { { .name = "default", @@ -294,6 +325,7 @@ AVFilter avfilter_af_volume = { .priv_size = sizeof(VolumeContext), .priv_class = &volume_class, .init = init, + .uninit = uninit, .inputs = avfilter_af_volume_inputs, .outputs = avfilter_af_volume_outputs, .flags = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC, diff --git a/libavfilter/af_volume.h b/libavfilter/af_volume.h index bd7932e..4deca9c 100644 --- a/libavfilter/af_volume.h +++ b/libavfilter/af_volume.h @@ -48,6 +48,7 @@ typedef struct VolumeContext { void (*scale_samples)(uint8_t *dst, const uint8_t *src, int nb_samples, int volume); int samples_align; + char *metadata; } VolumeContext;
void ff_volume_init_x86(VolumeContext *vol);
diff --git a/libavfilter/f_ebur128.c b/libavfilter/f_ebur128.c index 88d37e8..f4ce6d9 100644 --- a/libavfilter/f_ebur128.c +++ b/libavfilter/f_ebur128.c
Unrelated.
@@ -410,7 +410,7 @@ static av_cold int init(AVFilterContext *ctx)
if (ebur128->loglevel != AV_LOG_INFO && ebur128->loglevel != AV_LOG_VERBOSE) {
- if (ebur128->do_video || ebur128->metadata) + if (ebur128->do_video) ebur128->loglevel = AV_LOG_VERBOSE; else ebur128->loglevel = AV_LOG_INFO; @@ -689,7 +689,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *insamples) SET_META("LRA.high", ebur128->lra_high); }
- av_log(ctx, ebur128->loglevel, "t: %-10s " LOG_FMT "\n", + av_log(ctx, ebur128->metadata || !ebur128->do_video ? AV_LOG_VERBOSE : ebur128->loglevel, "t: %-10s " LOG_FMT "\n", av_ts2timestr(pts, &outlink->time_base), loudness_400, loudness_3000, ebur128->integrated_loudness, ebur128->loudness_range); diff --git a/libavfilter/x86/af_volume_init.c b/libavfilter/x86/af_volume_init.c index 81d605f..fab5a03 100644 --- a/libavfilter/x86/af_volume_init.c +++ b/libavfilter/x86/af_volume_init.c @@ -39,7 +39,7 @@ av_cold void ff_volume_init_x86(VolumeContext *vol) enum AVSampleFormat sample_fmt = av_get_packed_sample_fmt(vol->sample_fmt);
if (sample_fmt == AV_SAMPLE_FMT_S16) { - if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768) { + if (EXTERNAL_SSE2(mm_flags) && vol->volume_i < 32768 && !vol->metadata) { vol->scale_samples = ff_scale_samples_s16_sse2; vol->samples_align = 8; }
Regards,
-- Nicolas George
Nicolas George in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 21:41:52 +0200):
Le quintidi 25 messidor, an CCXXI, Jan Ehrhardt a écrit :
Subject: [FFmpeg-devel] One pass volume normalization (ebur128)
Single-pass volume normalization is not possible, please do not call the feature that way.
Call it what you like. I am using it in a single pass transcode. Just like the -af volnorm filter in MEncoder.
r128.I is not a good choice, but there is nothing better yet.
You can use all the r128 variables, that are inserted in the metadata.
Missing documentation update.
I know.
@@ -51,18 +51,24 @@ static const AVOption volume_options[] = { { "fixed", "select 8-bit fixed-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FIXED }, INT_MIN, INT_MAX, A|F, "precision" }, { "float", "select 32-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FLOAT }, INT_MIN, INT_MAX, A|F, "precision" }, { "double", "select 64-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_DOUBLE }, INT_MIN, INT_MAX, A|F, "precision" },
+ { "metadata", "set the metadata key for loudness normalization", OFFSET(metadata), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = A|F },
Inconsistent indentation.
Not really. If you look at the origional you will see that fixed, float and double are values for the precision.
+ if (vol->metadata) { + double loudness, new_volume, timestamp, mx; + AVDictionaryEntry *e; + mx = 20; + timestamp = (float)(1.0 * buf->pts / outlink->sample_rate); + mx = fmin(mx, timestamp); + e = av_dict_get(buf->metadata, vol->metadata, NULL, 0); + if (e) { + loudness = av_strtod(e->value, NULL); + if (loudness > -69) { + new_volume = fmax(-mx,fmin(mx,(-23 - loudness))); + av_log(NULL, AV_LOG_VERBOSE, "loudness=%f => %f => volume=%f\n", + loudness, new_volume, pow(10, new_volume / 20)); + set_fixed_volume(vol, pow(10, new_volume / 20)); + }
This paragraph has several problems. First, it is missing spaces around words, that is easy to fix.
ACK.
Second, it has a duplicated mathematical formula, which is pretty much a recipe for inconsistency. That is easy to fix too.
ACK.
Third, it has several hardcoded values, and that is not good design.
Two of the three hardcoded values should be hardcoded. The -23 is part of the EBU R128 specs: http://tech.ebu.ch/loudness The 69 was suggested by Clement. If there is no sound at all, the volume level seems to be reported as -71 or somemething like that. -69 means there is sound (with a very low volume). The 20 is indeed an arbitrary choice, to maximize the volume adjustment during the first 20 seconds of a video.
It seems to me that using an expression, evaluated each time the metadata value changes and with that value available as a variable would be a much nicer design.
I agree, but this is a little above my head.
AFAIK, this is unneeded since the "evil plan".
I do not even know what the "evil plan" is...
diff --git a/libavfilter/f_ebur128.c b/libavfilter/f_ebur128.c index 88d37e8..f4ce6d9 100644 --- a/libavfilter/f_ebur128.c +++ b/libavfilter/f_ebur128.c
Unrelated.
Not quite either. f_ebur128.c hardcodes the errorlevel to verbose if the metadata are set. You do not want to see the intermediate metadata if you do a 'one pass' transcoode. If needed you can always set the loglevel to view them. Jan
Le quintidi 25 messidor, an CCXXI, Jan Ehrhardt a écrit :
Not really. If you look at the origional you will see that fixed, float and double are values for the precision.
I was indeed wrong on this point.
Two of the three hardcoded values should be hardcoded. The -23 is part of the EBU R128 specs: http://tech.ebu.ch/loudness
Not all people want to implement EBU R128. And even if they do, with the current infrastructure, using r128.I will not yield a global -23 dB loudness and using the other variables will give audible distortion. Therefore, it is not currently possible to comply with EBU R128 recommendations in single pass.
Not quite either. f_ebur128.c hardcodes the errorlevel to verbose if the metadata are set. You do not want to see the intermediate metadata if you do a 'one pass' transcoode. If needed you can always set the loglevel to view them.
This still do not belong in this patch. Regards, -- Nicolas George
Nicolas George in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 23:07:11 +0200):
Le quintidi 25 messidor, an CCXXI, Jan Ehrhardt a écrit :
Not really. If you look at the origional you will see that fixed, float and double are values for the precision.
I was indeed wrong on this point.
OK.
Two of the three hardcoded values should be hardcoded. The -23 is part of the EBU R128 specs: http://tech.ebu.ch/loudness
Not all people want to implement EBU R128.
-23 LUFS is a good starting point. If they want a higher or lower volume, they could add another volume filter after the volume=metadata=lavfi.r128.I. A dual volume filter is no nice solution, but it is workable.
And even if they do, with the current infrastructure, using r128.I will not yield a global -23 dB loudness and using the other variables will give audible distortion. Therefore, it is not currently possible to comply with EBU R128 recommendations in single pass.
There are some situations where you cannot do a dual pass. For instance, if you are live broadcasting a symphony with a grand finale, there is no hardware or software solution possible that produces a fully EBU R128 compliant loudness. Something like my patch must be close enough. After all, EBU R128 is only a recommendation.
Not quite either. f_ebur128.c hardcodes the errorlevel to verbose if the metadata are set. You do not want to see the intermediate metadata if you do a 'one pass' transcoode. If needed you can always set the loglevel to view them.
This still do not belong in this patch.
I do not mind to make it a separate patch. How do we proceed? I can make some adjustments to my patch, but for some (f.i. the "evil plan") I have a lack of knowledgde. Could you make the adjustments and publish a new patch? It would be really nice if it would be there in, say, version 3.0. Jan
On Jul 13, 2013 3:36 PM, "Jan Ehrhardt" <phpdev@ehrhardt.nl> wrote:
How do we proceed? I can make some adjustments to my patch, but for some (f.i. the "evil plan") I have a lack of knowledgde.
http://article.gmane.org/gmane.comp.video.libav.devel/39137/ [...] Timothy
Timothy Gu in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 16:05:46 -0700):
On Jul 13, 2013 3:36 PM, "Jan Ehrhardt" <phpdev@ehrhardt.nl> wrote:
How do we proceed? I can make some adjustments to my patch, but for some (f.i. the "evil plan") I have a lack of knowledgde.
http://article.gmane.org/gmane.comp.video.libav.devel/39137/
Hmmm. Libav. I suppose it has been merged into FFMpeg. Still, I do not like the idea of removing things if I do not know exactly why they were there in the first place. I know my patch ain't broken, why fix something when I do not know the backgrounds? Jan
On 7/13/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
Timothy Gu in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 16:05:46 -0700):
On Jul 13, 2013 3:36 PM, "Jan Ehrhardt" <phpdev@ehrhardt.nl> wrote:
How do we proceed? I can make some adjustments to my patch, but for some (f.i. the "evil plan") I have a lack of knowledgde.
http://article.gmane.org/gmane.comp.video.libav.devel/39137/
Hmmm. Libav. I suppose it has been merged into FFMpeg.
Still, I do not like the idea of removing things if I do not know exactly why they were there in the first place. I know my patch ain't broken, why fix something when I do not know the backgrounds?
If I said you should remove it, then you most definitely should. Generic code from libavfilter already calls av_opt_free(). See avfilter_free().
Paul B Mahol in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 23:37:17 +0000):
If I said you should remove it, then you most definitely should.
Hmmm. Really nice way to treat outsiders...
Generic code from libavfilter already calls av_opt_free(). See avfilter_free().
Are we still talking about the same thing? At first you were commenting on a suboptimal change in af_volume_init.c and now you seem to be referring to the uinit thing. Should both be removed? Jan
On 7/14/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
Paul B Mahol in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 23:37:17 +0000):
If I said you should remove it, then you most definitely should.
Hmmm. Really nice way to treat outsiders...
Generic code from libavfilter already calls av_opt_free(). See avfilter_free().
Are we still talking about the same thing? At first you were commenting on a suboptimal change in af_volume_init.c and now you seem to be referring to the uinit thing. Should both be removed?
Nice troll. I'm ignoring your patches for rest of eternity.
Paul B Mahol in gmane.comp.video.ffmpeg.devel (Sun, 14 Jul 2013 15:53:16 +0000):
On 7/14/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
Paul B Mahol in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 23:37:17 +0000):
If I said you should remove it, then you most definitely should.
Hmmm. Really nice way to treat outsiders...
Generic code from libavfilter already calls av_opt_free(). See avfilter_free().
Are we still talking about the same thing? At first you were commenting on a suboptimal change in af_volume_init.c and now you seem to be referring to the uinit thing. Should both be removed?
Nice troll. I'm ignoring your patches for rest of eternity.
Think of me as a troll. I am not. But I do not like to be commanded. "If I said you should remove it, then you most definitely should." could have been put a lot more friendly. And the latter was a sincere question. Jan
There are some situations where you cannot do a dual pass. For instance, if you are live broadcasting a symphony with a grand finale, there is no hardware or software solution possible that produces a fully EBU R128 compliant loudness. Something like my patch must be close enough. After all, EBU R128 is only a recommendation.
EBU R128 specifically does not address any realtime paradigm for the simple reason: Say you have film credits, the normaliser thinks it's too quiet. The next advert is then boosted. R128 refers to programme loudness.
Kieran Kunhya in gmane.comp.video.ffmpeg.devel (Sun, 14 Jul 2013 00:07:02 +0100):
There are some situations where you cannot do a dual pass. For instance, if you are live broadcasting a symphony with a grand finale, there is no hardware or software solution possible that produces a fully EBU R128 compliant loudness. Something like my patch must be close enough. After all, EBU R128 is only a recommendation.
EBU R128 specifically does not address any realtime paradigm for the simple reason: Say you have film credits, the normaliser thinks it's too quiet. The next advert is then boosted.
If the next advert is prerecorded, you can apply separate loudness rules on it.
R128 refers to programme loudness.
I believe you. But aren't there loudness recommendations for live steams of popconcerts, the Bayreuther Festspiele or the like? Jan
If the next advert is prerecorded, you can apply separate loudness rules on it.
That's not possible for example when receiving a channel from a satellite feed or even from an SDI feed.
R128 refers to programme loudness.
I believe you. But aren't there loudness recommendations for live steams of popconcerts, the Bayreuther Festspiele or the like?
As far as I know this is the job of the mixing engineer using traditional meters. I really think this patch should not be committed since it has little to do with R128, although a lot of commercial manufacturers call their realtime products R128 compatible. I also don't understand why you specifically need 1-pass when you appear to have a file-based workflow.
Kieran Kunhya in gmane.comp.video.ffmpeg.devel (Sun, 14 Jul 2013 16:20:27 +0100):
R128 refers to programme loudness.
I believe you. But aren't there loudness recommendations for live steams of popconcerts, the Bayreuther Festspiele or the like?
As far as I know this is the job of the mixing engineer using traditional meters.
Strange, since there are methods to come close to the EBU recommendation for prerecorded programs.
I really think this patch should not be committed since it has little to do with R128, although a lot of commercial manufacturers call their realtime products R128 compatible.
OK, that is an argument I did not hear before. And the argument seems plausible. But am I remembering it correctly that you were the one that once on #ffmpeg-devel uttered the wish to do normalization on the fly? Did you change your mind?
I also don't understand why you specifically need 1-pass when you appear to have a file-based workflow.
Simple, speed. My users are professionals that record interviews with their clients (with their consent) and put them securely online to get feedback from their colleagues and coaches. The typical recording is one to two hours, depending on the project. But last week someone from Kansas even uploaded a 5:28h recording (1GB after transcoding to 400*300). The project in Kansas has put 7000 hours of video online since they started in October 2011, so they are doing a lot of transcoding. I try to squeeze out every minute of the transcoding time to keep my users satisfied. FFMpeg 2.0 was a happy surprise because it was about 10% faster than the previous version I had rolled out to the users. For volume normalization I have long been depending on the -af volnorm filter of MEncoder. But MEncoder is not maintained anymore, so we were looking for an alternative for '-af volnorm' within FFMpeg. Clement's patch provided us with that. I am in no way bound to EBU recommendations, so maybe we should rephrase the issue: how can we implement an equivalent to the -af volnorm filter in FFMpeg? See here for the docs on -af volnorm: http://www.mplayerhq.hu/DOCS/man/en/mplayer.1.html#AUDIO%20FILTERS Jan PS. http://x32.elijst.nl/single.mp4 is a good example why I need volume normalization. The video is too short to make full use of the filter. The volume is only increased from -40 LUFS to -35. But if I use the same video 4 times in a row the volume is already raised to -23.8 LUFS. So with videos of 40+ seconds I get the desired result.
But am I remembering it correctly that you were the one that once on #ffmpeg-devel uttered the wish to do normalization on the fly? Did you change your mind?
Yes, having read the spec in more detailed and spoken to people who helped create R128. I don't think it's a good idea to have realtime normalisation and call it R128.
Kieran Kunhya in gmane.comp.video.ffmpeg.devel (Sun, 14 Jul 2013 18:05:28 +0100):
But am I remembering it correctly that you were the one that once on #ffmpeg-devel uttered the wish to do normalization on the fly? Did you change your mind?
Yes, having read the spec in more detailed and spoken to people who helped create R128. I don't think it's a good idea to have realtime normalisation and call it R128.
Of course, we could make a duplicate of f_ebur128.c, remove every reference to R128 and use that for the injection of volume stats into the metadata. But it seems silly to duplicate a feature that is already in FFMpeg. For my kind of normalisation, two things are needed: volume injection in the metadata and use of those metadata for volume adjustment. How do we get rid of the R128 label without duplicating features? Jan
On Sun, Jul 14, 2013 at 7:45 PM, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
Kieran Kunhya in gmane.comp.video.ffmpeg.devel (Sun, 14 Jul 2013 18:05:28 +0100):
But am I remembering it correctly that you were the one that once on #ffmpeg-devel uttered the wish to do normalization on the fly? Did you change your mind?
Yes, having read the spec in more detailed and spoken to people who helped create R128. I don't think it's a good idea to have realtime normalisation and call it R128.
Of course, we could make a duplicate of f_ebur128.c, remove every reference to R128 and use that for the injection of volume stats into the metadata. But it seems silly to duplicate a feature that is already in FFMpeg.
For my kind of normalisation, two things are needed: volume injection in the metadata and use of those metadata for volume adjustment. How do we get rid of the R128 label without duplicating features?
I don't see what's wrong with having an out of spec option as long as it's clear it's not actually R128.
Kieran Kunhya in gmane.comp.video.ffmpeg.devel (Sun, 14 Jul 2013 19:49:56 +0100):
On Sun, Jul 14, 2013 at 7:45 PM, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
For my kind of normalisation, two things are needed: volume injection in the metadata and use of those metadata for volume adjustment. How do we get rid of the R128 label without duplicating features?
I don't see what's wrong with having an out of spec option as long as it's clear it's not actually R128.
OK, I will make a new patch with Nicolas' and Paul's comments (as far as I understand them) and be back. Jan
Le sextidi 26 messidor, an CCXXI, Jan Ehrhardt a écrit :
Simple, speed.
I am in no way bound to EBU recommendations
Then use two passes and volumedetect. If I take the decoding time for audio (Vorbis -q 4) as measurement unit, -af ebur128 costs about 4 while -af volumedetect costs about 0.4 (and -af volume 0.04). Therefore, two decoding for two passes plus volumedetect cost ~2.5 while one pass with ebur128 costs ~5. If you have enough RAM, you can save the PCM while running volumedetect and avoid the second decoding too. Regards, -- Nicolas George
Nicolas George in gmane.comp.video.ffmpeg.devel (Sun, 14 Jul 2013 23:14:59 +0200):
Le sextidi 26 messidor, an CCXXI, Jan Ehrhardt a écrit :
Simple, speed.
I am in no way bound to EBU recommendations
Then use two passes and volumedetect.
If I take the decoding time for audio (Vorbis -q 4) as measurement unit, -af ebur128 costs about 4 while -af volumedetect costs about 0.4 (and -af volume 0.04). Therefore, two decoding for two passes plus volumedetect cost ~2.5 while one pass with ebur128 costs ~5.
OK, you've got some points for using volumedetect. The question is if you still get those differences, taken into account that disk speed might be a limiting factor. Our recordings are on a SD card and should stay there. Any one-pass scheme has the definite advantage that the disk has to be accessed only once for reading the input file (and, no, SD cards do not normally have a disk cache). Besides that, using two passes is not easy, as I will explain below.
If you have enough RAM, you can save the PCM while running volumedetect and avoid the second decoding too.
I do not control the user environment at all. The only thing I know is that the host PC has at least Windows XP. For my users it is a one click experience: they connect a SD card with a Camcorder recording through USB to their computer, start our software which is on the same SD card, review if the right video is there and click on 'Yes, upload this for me'. Then, behind the scenes, MEncoder or FFMpeg starts compressing the recording, saves the resulting video on the SD card (I am not allowed to use the HD of the host PC) and LFTP starts uploading the compressed file over a SFTP-connection to our server. Sometimes hours after the one-click the video is online. As there is no user-interaction at all, I will have to find a way to pass the outcome of the volumedetect to a second FFMpeg-commandline for the second pass. As far as I know now, this can only be done by parsing the screen output of the first pass, extract the volume level and use that for composing the second command line. I would rather not do that, even if it delivers another 5% (or something like that) speed increase. Everything that breaks the process into more steps is bound to lead to errors. Would it be possible to insert the momentary value for volumedetect in the metadata and use that as input in af_volume.c? One pass normalization based on volumedetect should be faster than what we have now. How would we achieve that? Jan
Jan Ehrhardt in gmane.comp.video.ffmpeg.devel (Mon, 15 Jul 2013 06:47:48 +0200):
OK, you've got some points for using volumedetect. The question is if you still get those differences, taken into account that disk speed might be a limiting factor. Our recordings are on a SD card and should stay there. Any one-pass scheme has the definite advantage that the disk has to be accessed only once for reading the input file (and, no, SD cards do not normally have a disk cache).
I did a little test on a 2GB Sony MPEG recording. Transcoding using the R128 input took (on my i5) 178 seconds. Volumedetection on the same file only took 8 seconds and when I applied a volume=-6dB on the source file I was 166 seconds further. Net difference: 8 + 166 = 174 versus 178 is little bit more than a 2% speed gain. [two pass]
I would rather not do that, even if it delivers another 5% (or something like that) speed increase. Everything that breaks the process into more steps is bound to lead to errors.
When I copied the Sony recording to the SD card, I realised it was even a little more complicated. MPEG files can be combined using concat, but for instance MP4 files cannot. We talked about that back in October 2012: http://permalink.gmane.org/gmane.comp.video.ffmpeg.user/41031 I have now expanded that commandline with volume normalization for both the [a0] and [a1] audio tracks. Breaking it up for two pass encoding would in fact mean three pass in that case: (1) voldetect a0, (2) voldetect a1, (3) the transcoding. Way too complicated to justify a 2-3% speed gain.
Would it be possible to insert the momentary value for volumedetect in the metadata and use that as input in af_volume.c? One pass normalization based on volumedetect should be faster than what we have now.
How would we achieve that?
There is potential for about a 6-7% speed gain (166/178) if we can inject the momentary value for 'mean volume' into the metadata and use that for one pass normalization. I would really be interested. And then we would finally have an equivalent for MEncoders volnorm filter. Jan
Le septidi 27 messidor, an CCXXI, Jan Ehrhardt a écrit :
OK, you've got some points for using volumedetect. The question is if you still get those differences, taken into account that disk speed might be a limiting factor.
I have no idea, and "profile, don't speculate".
As there is no user-interaction at all, I will have to find a way to pass the outcome of the volumedetect to a second FFMpeg-commandline for the second pass.
That is not difficult at all.
As far as I know now, this can only be done by parsing the screen output of the first pass
It is true that the results of volumedetect go only in ffmpeg's log, but it is specifically designed to be easily parsable. We still need a clean solution for filters that want to communicate out-of-band information. Also, calling it "screen output" shows a deep misunderstanding of how things work.
Would it be possible to insert the momentary value for volumedetect in the metadata and use that as input in af_volume.c? One pass normalization based on volumedetect should be faster than what we have now.
The reason volumedetect is fast is because it does not do all this stuff that, done properly, is expensive, and done improperly is worse than useless.
I did a little test on a 2GB Sony MPEG recording. Transcoding using the R128 input took (on my i5) 178 seconds. Volumedetection on the same file only took 8 seconds and when I applied a volume=-6dB on the source file I was 166 seconds further. Net difference: 8 + 166 = 174 versus 178 is little bit more than a 2% speed gain.
Since you do not explain what is being measured (I suspect it includes video transcoding), this information is mostly useless.
I have now expanded that commandline with volume normalization for both the [a0] and [a1] audio tracks. Breaking it up for two pass encoding would in fact mean three pass in that case: (1) voldetect a0, (2) voldetect a1, (3) the transcoding.
I believe you are wrong, volumedetect should be ran on the whole concatenated audio. Regards, -- Nicolas George
Nicolas George in gmane.comp.video.ffmpeg.devel (Mon, 15 Jul 2013 10:45:36 +0200):
Le septidi 27 messidor, an CCXXI, Jan Ehrhardt a écrit :
OK, you've got some points for using volumedetect. The question is if you still get those differences, taken into account that disk speed might be a limiting factor.
I have no idea, and "profile, don't speculate".
That is why I did the test om a 2GB Sony Camcorder file. See the explanation below.
It is true that the results of volumedetect go only in ffmpeg's log, but it is specifically designed to be easily parsable. We still need a clean solution for filters that want to communicate out-of-band information.
Also, calling it "screen output" shows a deep misunderstanding of how things work.
No, that is no deep misunderstanding. On a SD card, you really do not want to use FFMpeg's logs, because they slow down things terribly. For instance, volume detection on a concatenated stream of 2 2GB Sony Camcorder files takes 17 seconds with no logging and 27 seconds with logging. Redirecting the screen output (stderr) to a file is 40% faster than using the log.
I did a little test on a 2GB Sony MPEG recording. Transcoding using the R128 input took (on my i5) 178 seconds. Volumedetection on the same file only took 8 seconds and when I applied a volume=-6dB on the source file I was 166 seconds further. Net difference: 8 + 166 = 174 versus 178 is little bit more than a 2% speed gain.
Since you do not explain what is being measured (I suspect it includes video transcoding), this information is mostly useless.
I thought that would be obvious. The Camcorder file of course contains both video and audio and we want them both. A two-pass transcode using volumedetect is 2-3% faster that a single pass transcode using the R128 metadata.
I have now expanded that commandline with volume normalization for both the [a0] and [a1] audio tracks. Breaking it up for two pass encoding would in fact mean three pass in that case: (1) voldetect a0, (2) voldetect a1, (3) the transcoding.
I believe you are wrong, volumedetect should be ran on the whole concatenated audio.
Sometimes you do want volumedetect on concatenated audio, sometimes you do not. In my case, the first recording is often a short intro on what the interview with the client will be about. It is very often taken under different sound conditions than the remainder of the recording(s). I have seen people recording the introduction inside a car before entering a house et cetera. These introductory recordings should be treated separately with respect to audio normalization. In a single pass transcode this means minor changes in the commandline options, in a 2-pass transcode it means breaking up the single commandline into at least three (1) voldetect intro, (2) voldetect remainder, (3) transcoding. Which is far less robust than the single command line. Jan PS. With respect to the slowness of writing to SD cards see this patch: https://github.com/FFmpeg/FFmpeg/commit/f4d9148fe282879b9fcc755767c9c04de9dd... By simply increasing the copy-buffer from 1K to 32K qt-faststart became 400 times as fast.
On Tue, Jul 16, 2013 at 6:35 AM, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
These introductory recordings should be treated separately with respect to audio normalization. In a single pass transcode this means minor changes in the commandline options, in a 2-pass transcode it means breaking up the single commandline into at least three (1) voldetect intro, (2) voldetect remainder, (3) transcoding. Which is far less robust than the single command line.
Why do you think it's less robust?
L'octidi 28 messidor, an CCXXI, Jan Ehrhardt a écrit :
No, that is no deep misunderstanding. On a SD card, you really do not want to use FFMpeg's logs, because they slow down things terribly. For instance, volume detection on a concatenated stream of 2 2GB Sony Camcorder files takes 17 seconds with no logging and 27 seconds with logging. Redirecting the screen output (stderr) to a file is 40% faster than using the log.
It seems clear to me your problem here is of system programming: establishing pipes with auxiliary processes to get their output without writing to a temporary file nor using "screen output" (and slightly more subtle: get two output streams from the same program without blocking).
I thought that would be obvious.
I thought it would be obvious that, to measure the speed of a process, you try to isolate this process as much as possible, not drown within unrelated work.
Sometimes you do want volumedetect on concatenated audio, sometimes you do not. In my case, the first recording is often a short intro on what the interview with the client will be about. It is very often taken under different sound conditions than the remainder of the recording(s). I have seen people recording the introduction inside a car before entering a house et cetera.
That is reasonable, but I do not see that it creates additional difficulty.
PS. With respect to the slowness of writing to SD cards see this patch: https://github.com/FFmpeg/FFmpeg/commit/f4d9148fe282879b9fcc755767c9c04de9dd... By simply increasing the copy-buffer from 1K to 32K qt-faststart became 400 times as fast.
It seems to me this is a sign of a crappy (or badly configured) operating system, but it is completely unrelated to the problem at hand since there is absolutely nothing to write in any permanent file until the final phase. Regards, -- Nicolas George
Nicolas George in gmane.comp.video.ffmpeg.devel (Tue, 16 Jul 2013 15:36:31 +0200):
It seems to me this is a sign of a crappy (or badly configured) operating system, but it is completely unrelated to the problem at hand since there is absolutely nothing to write in any permanent file until the final phase.
It might be a crappy OS (Windows), which cannot be configured better. But it perfectly explains the speed decrease (from 17 to 27 seconds) when the only thing you do is turn FFMpeg's logging on. Every program that does not buffer its output by itself suffers from the same problem. I already mentioned qt-faststart. MP4Box was another one (until the creators solved it on my request): http://sourceforge.net/p/gpac/discussion/287547/thread/9703f17d The same happens with writing a logfile to a SD card bij FFMpeg. Jan
L'octidi 28 messidor, an CCXXI, Jan Ehrhardt a écrit :
It might be a crappy OS (Windows), which cannot be configured better. But it perfectly explains the speed decrease (from 17 to 27 seconds) when the only thing you do is turn FFMpeg's logging on.
No, it does not, since the amount of ffmpeg's log messages is negligible. Please see the rest of my previous message for things that matter. Regards, -- Nicolas George
Nicolas George in gmane.comp.video.ffmpeg.devel (Tue, 16 Jul 2013 17:28:55 +0200):
L'octidi 28 messidor, an CCXXI, Jan Ehrhardt a écrit :
It might be a crappy OS (Windows), which cannot be configured better. But it perfectly explains the speed decrease (from 17 to 27 seconds) when the only thing you do is turn FFMpeg's logging on.
No, it does not, since the amount of ffmpeg's log messages is negligible. Please see the rest of my previous message for things that matter.
Two command lines: ffmpeg.exe \ -i U:\MP_ROOT\100PNV01\SD00004.MP4 \ -i U:\MP_ROOT\100PNV01\SD00005.MP4 \ -filter_complex " \ [0:a]asetpts=PTS-STARTPTS[a0]; \ [1:a]asetpts=PTS-STARTPTS[a1]; \ [a0][a1]concat=n=2:v=0:a=1[a]; \ [a]volumedetect[am]" -map [am] \ -f null -y /dev/null Execution time: 16.765 seconds. ffmpeg.exe \ -i U:\MP_ROOT\100PNV01\SD00004.MP4 \ -i U:\MP_ROOT\100PNV01\SD00005.MP4 \ -filter_complex " \ [0:a]asetpts=PTS-STARTPTS[a0]; \ [1:a]asetpts=PTS-STARTPTS[a1]; \ [a0][a1]concat=n=2:v=0:a=1[a]; \ [a]volumedetect[am]" -map [am] \ -report -f null -y /dev/null Execution time: 27.154 seconds Please tell me where I go wrong. And/or guide me to a quicker volumedetect an a concatenated audio track. Jan
L'octidi 28 messidor, an CCXXI, Jan Ehrhardt a écrit :
Two command lines:
Learn to use pipes. Regards, -- Nicolas George
On Tue, Jul 16, 2013 at 08:16:39PM +0200, Jan Ehrhardt wrote:
Nicolas George in gmane.comp.video.ffmpeg.devel (Tue, 16 Jul 2013 17:28:55 +0200):
L'octidi 28 messidor, an CCXXI, Jan Ehrhardt a écrit :
It might be a crappy OS (Windows), which cannot be configured better. But it perfectly explains the speed decrease (from 17 to 27 seconds) when the only thing you do is turn FFMpeg's logging on.
No, it does not, since the amount of ffmpeg's log messages is negligible. Please see the rest of my previous message for things that matter.
Two command lines:
ffmpeg.exe \ -i U:\MP_ROOT\100PNV01\SD00004.MP4 \ -i U:\MP_ROOT\100PNV01\SD00005.MP4 \ -filter_complex " \ [0:a]asetpts=PTS-STARTPTS[a0]; \ [1:a]asetpts=PTS-STARTPTS[a1]; \ [a0][a1]concat=n=2:v=0:a=1[a]; \ [a]volumedetect[am]" -map [am] \ -f null -y /dev/null
Execution time: 16.765 seconds.
ffmpeg.exe \ -i U:\MP_ROOT\100PNV01\SD00004.MP4 \ -i U:\MP_ROOT\100PNV01\SD00005.MP4 \ -filter_complex " \ [0:a]asetpts=PTS-STARTPTS[a0]; \ [1:a]asetpts=PTS-STARTPTS[a1]; \ [a0][a1]concat=n=2:v=0:a=1[a]; \ [a]volumedetect[am]" -map [am] \ -report -f null -y /dev/null
Execution time: 27.154 seconds
Please tell me where I go wrong. And/or guide me to a quicker volumedetect an a concatenated audio track.
You probably have write caching or something similar off (on Windows, it is off by default for anything it believes you might be able to detach). If you use log/temporary files you really should be using a disk suitable for it. Since pipes are unfortunately a rather unusable pain, a RAM disk would be the best performing alternative.
Reimar Döffinger in gmane.comp.video.ffmpeg.devel (Tue, 16 Jul 2013 21:30:04 +0200):
Execution time: 16.765 seconds.
Execution time: 27.154 seconds
Please tell me where I go wrong. And/or guide me to a quicker volumedetect an a concatenated audio track.
You probably have write caching or something similar off (on Windows, it is off by default for anything it believes you might be able to detach).
Yes, I know caching is off but I cannot control that on the user-PC's. I've already searched for methods to change it programmatically, but could find no way.
If you use log/temporary files you really should be using a disk suitable for it.
I cannot even use the HDD of the PC. In between the SD card and the PC are all kinds of IT-obstacles and privacy-related issues.
Since pipes are unfortunately a rather unusable pain,
Actually, that was not that hard. Adding 2>&1 | find "mean_volume" to the commandline gave me the desired line. However, due to the lack of grep and sed on Windows it is not easy to parse that line further. If I would need to, I'd probably use the PHP on the SD card to parse it further.
a RAM disk would be the best performing alternative.
I have been on that track as well, but for another reason. Space is limited on the SD cards. Sometimes there is not enough room for the output file. My largest output file has been 1GB (5:28h 400*300 MP4) up until now. A 1GB+ ramdisk would be ideal in those cases. If you know a commandline installable RAMdisk with no need for registry access... Jan
L'octidi 28 messidor, an CCXXI, Reimar Döffinger a écrit :
Since pipes are unfortunately a rather unusable pain,
If you are talking about pipes in windows, I will take your word for it, as I do not know anything about windows system programming. If you are talking about pipes in Unix/POSIX-style systems, I strongly disagree. AFAIK, PHP comes from the Unix world, so its pipes should be similar to Unix pipes. OTOH, PHP should not be used for anything beyond webforums for teenagers. Regards, -- Nicolas George
Jan Ehrhardt in gmane.comp.video.ffmpeg.devel (Mon, 15 Jul 2013 06:47:48 +0200):
Would it be possible to insert the momentary value for volumedetect in the metadata and use that as input in af_volume.c? One pass normalization based on volumedetect should be faster than what we have now.
To answer my own question: Yes, that is possible. Since Nicolas problably does not want to tamper with volumedetect, I created a clone called af_volumeinject.c. See below. Just like I now use volume=metadata=lavfi.r128.I this volumeinject allows me to use volume=metadata=lavfi.mean_volume. It is only a tiny bit slower than something like volume=-3dB, but faster than a two pass volumedetect + volume=-xx.xdB. One thing came while testing volumeinject. I am really a novice in loudness and would have expected that volumedetect and ebur128 would report more or less the same value. They do not. They were 3-4 dB apart in my testfiles, with ebur128 reporting a louder value. Jan af_volumeinject.c: /* * Copyright (c) 2012 Nicolas George * * This file is (not yet) part of FFmpeg. * * FFmpeg is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public License * as published by the Free Software Foundation; either * version 2.1 of the License, or (at your option) any later version. * * FFmpeg is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public License * along with FFmpeg; if not, write to the Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ #include "libavutil/channel_layout.h" #include "libavutil/avassert.h" #include "audio.h" #include "avfilter.h" #include "internal.h" typedef struct { /** * Number of samples at each PCM value. * histogram[0x8000 + i] is the number of samples at value i. * The extra element is there for symmetry. */ uint64_t histogram[0x10001]; uint64_t nb_tot_samples; uint64_t power; } VolInjectContext; static int query_formats(AVFilterContext *ctx) { static const enum AVSampleFormat sample_fmts[] = { AV_SAMPLE_FMT_S16, AV_SAMPLE_FMT_S16P, AV_SAMPLE_FMT_NONE }; AVFilterFormats *formats; if (!(formats = ff_make_format_list(sample_fmts))) return AVERROR(ENOMEM); ff_set_common_formats(ctx, formats); return 0; } #define MAX_DB 91 static inline double logdb(uint64_t v) { double d = v / (double)(0x8000 * 0x8000); if (!v) return MAX_DB; return log(d) * -4.3429448190325182765112891891660508229; /* -10/log(10) */ } static int filter_frame(AVFilterLink *inlink, AVFrame *samples) { AVFilterContext *ctx = inlink->dst; VolInjectContext *vd = ctx->priv; int64_t layout = samples->channel_layout; int nb_samples = samples->nb_samples; int nb_channels = av_get_channel_layout_nb_channels(layout); int nb_planes = nb_channels; int plane, i; int16_t *pcm; uint64_t mean_power = 0; double mean_volume; char metabuf[128]; if (!av_sample_fmt_is_planar(samples->format)) { nb_samples *= nb_channels; nb_planes = 1; } for (plane = 0; plane < nb_planes; plane++) { pcm = (int16_t *)samples->extended_data[plane]; for (i = 0; i < nb_samples; i++) { vd->histogram[pcm[i] + 0x8000]++; vd->nb_tot_samples++; vd->power += pcm[i] * pcm[i]; } } av_log(ctx, AV_LOG_VERBOSE, "tot_samples: %"PRId64"\n", vd->nb_tot_samples); if (vd->nb_tot_samples) { mean_power = (vd->power + vd->nb_tot_samples / 2) / vd->nb_tot_samples; mean_volume = -logdb(mean_power); #define SET_META(name, var) do { \ snprintf(metabuf, sizeof(metabuf), "%.3f", var); \ av_dict_set(&samples->metadata, "lavfi." name, metabuf, 0); \ } while (0) SET_META("mean_volume", mean_volume); av_log(ctx, AV_LOG_VERBOSE, "mean_volume: %.1f dB\n", mean_volume); } return ff_filter_frame(inlink->dst->outputs[0], samples); } static void print_stats(AVFilterContext *ctx) { VolInjectContext *vd = ctx->priv; int i, max_volume, shift; uint64_t nb_samples = 0, power = 0, nb_samples_shift = 0, sum = 0; uint64_t histdb[MAX_DB + 1] = { 0 }; for (i = 0; i < 0x10000; i++) nb_samples += vd->histogram[i]; av_log(ctx, AV_LOG_INFO, "n_samples: %"PRId64"\n", nb_samples); if (!nb_samples) return; /* If nb_samples > 1<<34, there is a risk of overflow in the multiplication or the sum: shift all histogram values to avoid that. The total number of samples must be recomputed to avoid rounding errors. */ shift = av_log2(nb_samples >> 33); for (i = 0; i < 0x10000; i++) { nb_samples_shift += vd->histogram[i] >> shift; power += (i - 0x8000) * (i - 0x8000) * (vd->histogram[i] >> shift); } if (!nb_samples_shift) return; power = (power + nb_samples_shift / 2) / nb_samples_shift; av_assert0(power <= 0x8000 * 0x8000); av_log(ctx, AV_LOG_INFO, "mean_volume: %.1f dB\n", -logdb(power)); max_volume = 0x8000; while (max_volume > 0 && !vd->histogram[0x8000 + max_volume] && !vd->histogram[0x8000 - max_volume]) max_volume--; av_log(ctx, AV_LOG_INFO, "max_volume: %.1f dB\n", -logdb(max_volume * max_volume)); for (i = 0; i < 0x10000; i++) histdb[(int)logdb((i - 0x8000) * (i - 0x8000))] += vd->histogram[i]; for (i = 0; i <= MAX_DB && !histdb[i]; i++); for (; i <= MAX_DB && sum < nb_samples / 1000; i++) { av_log(ctx, AV_LOG_INFO, "histogram_%ddb: %"PRId64"\n", i, histdb[i]); sum += histdb[i]; } } static av_cold void uninit(AVFilterContext *ctx) { print_stats(ctx); } static const AVFilterPad volumeinject_inputs[] = { { .name = "default", .type = AVMEDIA_TYPE_AUDIO, .get_audio_buffer = ff_null_get_audio_buffer, .filter_frame = filter_frame, }, { NULL } }; static const AVFilterPad volumeinject_outputs[] = { { .name = "default", .type = AVMEDIA_TYPE_AUDIO, }, { NULL } }; AVFilter avfilter_af_volumeinject = { .name = "volumeinject", .description = NULL_IF_CONFIG_SMALL("inject audio volume."), .priv_size = sizeof(VolInjectContext), .query_formats = query_formats, .uninit = uninit, .inputs = volumeinject_inputs, .outputs = volumeinject_outputs, };
On 7/17/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
Jan Ehrhardt in gmane.comp.video.ffmpeg.devel (Mon, 15 Jul 2013 06:47:48 +0200):
Would it be possible to insert the momentary value for volumedetect in the metadata and use that as input in af_volume.c? One pass normalization based on volumedetect should be faster than what we have now.
To answer my own question: Yes, that is possible. Since Nicolas problably does not want to tamper with volumedetect, I created a clone called af_volumeinject.c. See below.
Just like I now use volume=metadata=lavfi.r128.I this volumeinject allows me to use volume=metadata=lavfi.mean_volume. It is only a tiny bit slower than something like volume=-3dB, but faster than a two pass volumedetect + volume=-xx.xdB.
One thing came while testing volumeinject. I am really a novice in loudness and would have expected that volumedetect and ebur128 would report more or less the same value. They do not. They were 3-4 dB apart in my testfiles, with ebur128 reporting a louder value.
Because they are doing different things ...
Le nonidi 29 messidor, an CCXXI, Jan Ehrhardt a écrit :
To answer my own question: Yes, that is possible. Since Nicolas problably does not want to tamper with volumedetect, I created a clone called af_volumeinject.c. See below.
I consider this very bad design. I already explained why I think this is bad design from the result point of view, but you are of course free to keep it in your software. From ffmpeg point of view, I consider this bad design for the following reasons: * Two filters with almost identical features and no good reason to separate them (there is a good reason to have ebur128 and volumedetect: correctness vs. speed). * Exposing intermediate values that have no relevancy whatsoever. The final results of volumedetect are already quite dubious as volume measurements, but at least they have a clear mathematical meaning. The intermediate values are just random excerpts taken at instants decided by whatever framing caused by previous filters. * Adding a feature to suit a very personal and specific need. IMHO, the correct design for solving this issue would require some or all the following points: * Dynamic expression evaluation for the volume filter. IIRC, Stefano had a patch that was pretty good; in fact, I thought it was already applied since a long time ago. The expression should be able to reference metatdata (at least one item). * A filter to smooth a metadata value over time, so that r128.M can be turned into something suitable for volume normalization. * A switch to volumedetect to inject as metadata the momentary RMS of the signal over a configurable frame, to use in place of r128.M and trade correctness for speed. * A switch to volumedetect to inject as metadata the final results (on a dummy final frame maybe?). * A clean way for filters to report information to the outside world. Hopefully, usable for structured information (porting ffprobe's printers), but just flat would already be nice (maybe extending the ffmetadata to dump per-frame metadata?). * A scriptable filter capable of waiting the final frame on one of its inputs and using the metadata in it to send a command to a volume filter. Some of these are fairly easy, other are quite hard, and some pose problems of design decisions rather than implementation. Anyone should feel free to submit patches implementing any of these points. Regards, -- Nicolas George
Nicolas George in gmane.comp.video.ffmpeg.devel (Wed, 17 Jul 2013 17:11:15 +0200):
I consider this very bad design. I already explained why I think this is bad design from the result point of view, but you are of course free to keep it in your software.
From ffmpeg point of view, I consider this bad design for the following reasons:
* Two filters with almost identical features and no good reason to separate them (there is a good reason to have ebur128 and volumedetect: correctness vs. speed).
The reason of the duplicate is clear: I was expecting a reaction by you like this.
* Exposing intermediate values that have no relevancy whatsoever. The final results of volumedetect are already quite dubious as volume measurements, but at least they have a clear mathematical meaning.
Strange. First you steer me in the direction of volumedetect and now it seems the other way around.
* Adding a feature to suit a very personal and specific need.
Here we differ. I do not think having the equivalent of MEncoder's '-af volnorm' is a very personal and specific need. For live broadcasts you need a way to normalize the loudness and the only way to do that is looking back in time at the previous frames.
IMHO, the correct design for solving this issue would require some or all the following points:
* Dynamic expression evaluation for the volume filter. IIRC, Stefano had a patch that was pretty good; in fact, I thought it was already applied since a long time ago. The expression should be able to reference metadata (at least one item).
As far as I know, this was never implemented. Neither were any of Clement's proposals. That is exactly the reason why I brought up the subject once again.
* A filter to smooth a metadata value over time, so that r128.M can be turned into something suitable for volume normalization.
* A switch to volumedetect to inject as metadata the momentary RMS of the signal over a configurable frame, to use in place of r128.M and trade correctness for speed.
I would welcome those features, but keep in mind that neither r128.I nor r128.M can be used at all at the moment.
* A switch to volumedetect to inject as metadata the final results (on a dummy final frame maybe?).
As a switch is is OK, of course. But it will not help for live streams.
Some of these are fairly easy, other are quite hard, and some pose problems of design decisions rather than implementation. Anyone should feel free to submit patches implementing any of these points.
My suggestion was to start with implementing a basic filter like I proposed when I started this discussion. It was met with not only the expected technical comments, but also with arguments against the idea of one-pass or on-the-fly normalization at all. See my current working version below. I have taken away some of the technical issues. Do with it whatever you like. My idea is that is is a good starting point for future patches like you suggested. Jan diff --git a/libavfilter/af_volume.c b/libavfilter/af_volume.c index a2ac1e2..87491ea 100644 --- a/libavfilter/af_volume.c +++ b/libavfilter/af_volume.c @@ -51,18 +51,26 @@ static const AVOption volume_options[] = { { "fixed", "select 8-bit fixed-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FIXED }, INT_MIN, INT_MAX, A|F, "precision" }, { "float", "select 32-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FLOAT }, INT_MIN, INT_MAX, A|F, "precision" }, { "double", "select 64-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_DOUBLE }, INT_MIN, INT_MAX, A|F, "precision" }, + { "metadata", "set the metadata key for loudness normalization", OFFSET(metadata), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = A|F }, + { "normvol", "set volume normalization level", + OFFSET(normvol), AV_OPT_TYPE_DOUBLE, { .dbl = -23.0 }, INT_MIN, INT_MAX, A|F }, { NULL }, }; AVFILTER_DEFINE_CLASS(volume); +static void set_fixed_volume(VolumeContext *vol, double volume) +{ + vol->volume_i = (int)(volume * 256 + 0.5); + vol->volume = vol->volume_i / 256.0; +} + static av_cold int init(AVFilterContext *ctx) { VolumeContext *vol = ctx->priv; if (vol->precision == PRECISION_FIXED) { - vol->volume_i = (int)(vol->volume * 256 + 0.5); - vol->volume = vol->volume_i / 256.0; + set_fixed_volume(vol, vol->volume); av_log(ctx, AV_LOG_VERBOSE, "volume:(%d/256)(%f)(%1.2fdB) precision:fixed\n", vol->volume_i, vol->volume, 20.0*log(vol->volume)/M_LN10); } else { @@ -216,11 +224,31 @@ static int config_output(AVFilterLink *outlink) static int filter_frame(AVFilterLink *inlink, AVFrame *buf) { - VolumeContext *vol = inlink->dst->priv; - AVFilterLink *outlink = inlink->dst->outputs[0]; + AVFilterContext *ctx = inlink->dst; + VolumeContext *vol = ctx->priv; + AVFilterLink *outlink = ctx->outputs[0]; int nb_samples = buf->nb_samples; AVFrame *out_buf; + if (vol->metadata) { + double loudness, new_volume, pow_volume, timestamp, mx; + AVDictionaryEntry *e; + mx = 20; + timestamp = (float)(1.0 * buf->pts / outlink->sample_rate); + mx = fmin(mx, timestamp); + e = av_dict_get(buf->metadata, vol->metadata, NULL, 0); + if (e) { + loudness = av_strtod(e->value, NULL); + if (loudness > -69) { + new_volume = fmax(-mx, fmin(mx, (vol->normvol - loudness))); + pow_volume = pow(10, new_volume / 20); + av_log(ctx, AV_LOG_VERBOSE, "loudness=%f => %f => volume=%f\n", + loudness, new_volume, pow_volume); + set_fixed_volume(vol, pow_volume); + } + } + } + if (vol->volume == 1.0 || vol->volume_i == 256) return ff_filter_frame(outlink, buf); diff --git a/libavfilter/af_volume.h b/libavfilter/af_volume.h index bd7932e..d79d040 100644 --- a/libavfilter/af_volume.h +++ b/libavfilter/af_volume.h @@ -48,6 +48,8 @@ typedef struct VolumeContext { void (*scale_samples)(uint8_t *dst, const uint8_t *src, int nb_samples, int volume); int samples_align; + char *metadata; + double normvol; } VolumeContext; void ff_volume_init_x86(VolumeContext *vol);
Le nonidi 29 messidor, an CCXXI, Jan Ehrhardt a écrit :
Strange. First you steer me in the direction of volumedetect and now it seems the other way around.
I will explain one last time: using the intermediate value of r128.I or its equivalent from volumedetect for volume normalization is WRONG. I do not want to help doing something wrong, nor will I support adding a feature in ffmpeg whose main use is to do something wrong. I also believe that it would have been better if Clément did not expose the intermediate value of r128.I, only its final value. The RIGHT way of doing normalization is to use either a smoothed immediate value or the final value with two passes. The first pass would cost almost nothing if correctly implemented.
Here we differ. I do not think having the equivalent of MEncoder's '-af volnorm' is a very personal and specific need. For live broadcasts you need a way to normalize the loudness and the only way to do that is looking back in time at the previous frames.
I just looked at volnorm's algorithm, it is far from good.
diff --git a/libavfilter/af_volume.c b/libavfilter/af_volume.c index a2ac1e2..87491ea 100644 --- a/libavfilter/af_volume.c +++ b/libavfilter/af_volume.c
I think any patch that will make it harder to implement dynamic expression evaluation is a bad idea, but I am not maintainer of av_volume. Regards, -- Nicolas George
On 7/18/13, Nicolas George <nicolas.george@normalesup.org> wrote:
Le nonidi 29 messidor, an CCXXI, Jan Ehrhardt a écrit :
Strange. First you steer me in the direction of volumedetect and now it seems the other way around.
I will explain one last time: using the intermediate value of r128.I or its equivalent from volumedetect for volume normalization is WRONG. I do not want to help doing something wrong, nor will I support adding a feature in ffmpeg whose main use is to do something wrong. I also believe that it would have been better if Clément did not expose the intermediate value of r128.I, only its final value.
The RIGHT way of doing normalization is to use either a smoothed immediate value or the final value with two passes. The first pass would cost almost nothing if correctly implemented.
So am I correct that there is no smoothed intermediate value available as of today? One liners are just sexy if it were possible to do volume normalization on the fly :) -roger-
(Sorry for the thread necromancy.) Le septidi 17 prairial, an CCXXII, Roger Pack a écrit :
So am I correct that there is no smoothed intermediate value available as of today?
Indeed. Patch welcome.
One liners are just sexy if it were possible to do volume normalization on the fly :)
Volume normalization on the fly is possible with a long one-liner: ffmpeg ... -af volume="$(ffmpeg ... -af ebur128 | parse)" ... If the program is coherent, a constant volume adjustment is better, and it can only be achieved with two passes. Regards, -- Nicolas George
On 7/13/13, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
Nicolas George in gmane.comp.video.ffmpeg.devel (Sat, 13 Jul 2013 21:41:52 +0200):
Le quintidi 25 messidor, an CCXXI, Jan Ehrhardt a ecrit :
Subject: [FFmpeg-devel] One pass volume normalization (ebur128)
Single-pass volume normalization is not possible, please do not call the feature that way.
Call it what you like. I am using it in a single pass transcode. Just like the -af volnorm filter in MEncoder.
r128.I is not a good choice, but there is nothing better yet.
You can use all the r128 variables, that are inserted in the metadata.
Missing documentation update.
I know.
@@ -51,18 +51,24 @@ static const AVOption volume_options[] = { { "fixed", "select 8-bit fixed-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FIXED }, INT_MIN, INT_MAX, A|F, "precision" }, { "float", "select 32-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_FLOAT }, INT_MIN, INT_MAX, A|F, "precision" }, { "double", "select 64-bit floating-point", 0, AV_OPT_TYPE_CONST, { .i64 = PRECISION_DOUBLE }, INT_MIN, INT_MAX, A|F, "precision" },
+ { "metadata", "set the metadata key for loudness normalization", OFFSET(metadata), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = A|F },
Inconsistent indentation.
Not really. If you look at the origional you will see that fixed, float and double are values for the precision.
+ if (vol->metadata) { + double loudness, new_volume, timestamp, mx; + AVDictionaryEntry *e; + mx = 20; + timestamp = (float)(1.0 * buf->pts / outlink->sample_rate); + mx = fmin(mx, timestamp); + e = av_dict_get(buf->metadata, vol->metadata, NULL, 0); + if (e) { + loudness = av_strtod(e->value, NULL); + if (loudness > -69) { + new_volume = fmax(-mx,fmin(mx,(-23 - loudness))); + av_log(NULL, AV_LOG_VERBOSE, "loudness=%f => %f => volume=%f\n", + loudness, new_volume, pow(10, new_volume / 20)); + set_fixed_volume(vol, pow(10, new_volume / 20)); + }
This paragraph has several problems. First, it is missing spaces around words, that is easy to fix.
ACK.
Second, it has a duplicated mathematical formula, which is pretty much a recipe for inconsistency. That is easy to fix too.
ACK.
Third, it has several hardcoded values, and that is not good design.
Two of the three hardcoded values should be hardcoded. The -23 is part of the EBU R128 specs: http://tech.ebu.ch/loudness
The 69 was suggested by Clement. If there is no sound at all, the volume level seems to be reported as -71 or somemething like that. -69 means there is sound (with a very low volume).
The 20 is indeed an arbitrary choice, to maximize the volume adjustment during the first 20 seconds of a video.
It seems to me that using an expression, evaluated each time the metadata value changes and with that value available as a variable would be a much nicer design.
I agree, but this is a little above my head.
AFAIK, this is unneeded since the "evil plan".
I do not even know what the "evil plan" is...
It means, that code is not needed any more. Just remove whole function.
diff --git a/libavfilter/f_ebur128.c b/libavfilter/f_ebur128.c index 88d37e8..f4ce6d9 100644 --- a/libavfilter/f_ebur128.c +++ b/libavfilter/f_ebur128.c
Unrelated.
Not quite either. f_ebur128.c hardcodes the errorlevel to verbose if the metadata are set. You do not want to see the intermediate metadata if you do a 'one pass' transcoode. If needed you can always set the loglevel to view them.
Jan
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
participants (9)
-
Claudio Freire -
Jan Ehrhardt -
Kieran Kunhya -
Nicolas George -
Nicolas George -
Paul B Mahol -
Reimar Döffinger -
Roger Pack -
Timothy Gu