diff mbox series

[FFmpeg-devel,v2,05/14] avfilter/af_aresample: Use preinit instead of init_dict

Message ID AM7PR03MB6660E0329775F3E3BF88C7798FD99@AM7PR03MB6660.eurprd03.prod.outlook.com
State Accepted
Commit 0c34cf899dc67febb57f5da4ec50640985e4f670
Headers show
Series [FFmpeg-devel,v2,01/14] avfilter/vsrc_testsrc: Deduplicate AVClasses | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Andreas Rheinhardt Sept. 13, 2021, 11:23 p.m. UTC 
By using preinit, the SwrContext already exists directly after
allocating the filter, so that the filter's AVClass's child_next
becomes usable for setting options with the AV_OPT_SEARCH_CHILDREN
search flag. This means that it is no longer necessary to use
the init_dict callback for this filter.

Furthermore, the earlier code did not abide by the documentation
of the init_dict callback at all: Instead of only returning the
options that have not been recognized it always returned all options
on any av_opt_set() error and errored out in this case; yet if
the error was just caused by an unrecognized option, it should not
error out at all and instead return said option.

This behaviour has been inherited by avfilter_init_dict(),
contradicting its documentation. This is also fixed by this commit.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavfilter/af_aresample.c | 27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

Comments

Nicolas George Sept. 16, 2021, 12:36 p.m. UTC | #1 
Andreas Rheinhardt (12021-09-14):
> By using preinit, the SwrContext already exists directly after
> allocating the filter, so that the filter's AVClass's child_next
> becomes usable for setting options with the AV_OPT_SEARCH_CHILDREN
> search flag. This means that it is no longer necessary to use
> the init_dict callback for this filter.
> 
> Furthermore, the earlier code did not abide by the documentation
> of the init_dict callback at all: Instead of only returning the
> options that have not been recognized it always returned all options
> on any av_opt_set() error and errored out in this case; yet if
> the error was just caused by an unrecognized option, it should not
> error out at all and instead return said option.
> 
> This behaviour has been inherited by avfilter_init_dict(),
> contradicting its documentation. This is also fixed by this commit.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavfilter/af_aresample.c | 27 +++++++--------------------
>  1 file changed, 7 insertions(+), 20 deletions(-)

This looks like a good change. Thanks.

Regards,
diff mbox series

Patch

diff --git a/libavfilter/af_aresample.c b/libavfilter/af_aresample.c
index e46369b98d..8e0eaee17b 100644
--- a/libavfilter/af_aresample.c
+++ b/libavfilter/af_aresample.c
@@ -43,31 +43,16 @@  typedef struct AResampleContext {
     int more_data;
 } AResampleContext;
 
-static av_cold int init_dict(AVFilterContext *ctx, AVDictionary **opts)
+static av_cold int preinit(AVFilterContext *ctx)
 {
     AResampleContext *aresample = ctx->priv;
-    int ret = 0;
 
     aresample->next_pts = AV_NOPTS_VALUE;
     aresample->swr = swr_alloc();
-    if (!aresample->swr) {
-        ret = AVERROR(ENOMEM);
-        goto end;
-    }
-
-    if (opts) {
-        AVDictionaryEntry *e = NULL;
+    if (!aresample->swr)
+        return AVERROR(ENOMEM);
 
-        while ((e = av_dict_get(*opts, "", e, AV_DICT_IGNORE_SUFFIX))) {
-            if ((ret = av_opt_set(aresample->swr, e->key, e->value, 0)) < 0)
-                goto end;
-        }
-        av_dict_free(opts);
-    }
-    if (aresample->sample_rate_arg > 0)
-        av_opt_set_int(aresample->swr, "osr", aresample->sample_rate_arg, 0);
-end:
-    return ret;
+    return 0;
 }
 
 static av_cold void uninit(AVFilterContext *ctx)
@@ -90,6 +75,8 @@  static int query_formats(AVFilterContext *ctx)
     AVFilterChannelLayouts *in_layouts, *out_layouts;
     int ret;
 
+    if (aresample->sample_rate_arg > 0)
+        av_opt_set_int(aresample->swr, "osr", aresample->sample_rate_arg, 0);
     av_opt_get_sample_fmt(aresample->swr, "osf", 0, &out_format);
     av_opt_get_int(aresample->swr, "osr", 0, &out_rate);
     av_opt_get_int(aresample->swr, "ocl", 0, &out_layout);
@@ -343,7 +330,7 @@  static const AVFilterPad aresample_outputs[] = {
 const AVFilter ff_af_aresample = {
     .name          = "aresample",
     .description   = NULL_IF_CONFIG_SMALL("Resample audio data."),
-    .init_dict     = init_dict,
+    .preinit       = preinit,
     .uninit        = uninit,
     .query_formats = query_formats,
     .priv_size     = sizeof(AResampleContext),