| Message ID | CAPYw7P5CV0zX-w6v0Y+ok9OgXRbfJ4iih_YNtP6jQAN2b4E4Rw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avfilter/src_movie: support unknown channel layouts | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/configure_loongarch64 | warning | Failed to apply patch |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Paul B Mahol: > - av_log(log_ctx, AV_LOG_ERROR, > + char *cl_name; > + > + av_log(log_ctx, AV_LOG_WARNING, > "Channel layout is not set in stream %d, and could not " > "be guessed from the number of channels (%d)\n", > st_index, dec_par->ch_layout.nb_channels); > - return AVERROR(EINVAL); > + cl_name = av_asprintf("%dC", dec_par->ch_layout.nb_channels); > + av_channel_layout_from_string(&chl, cl_name); > + free(cl_name); 1. Wrong deallocator. 2. The allocation is completely unnecessary: One can just use snprintf with a big enough (yet still small) buffer. 3. But even that is unnecessary: Just set chl = (AVChannelLayout){ .order = AV_CHANNEL_ORDER_UNSPEC, .nb_channels = dec_par->ch_layout.nb_channels }. - Andreas
On 10/31/2022 9:15 AM, Andreas Rheinhardt wrote: > Paul B Mahol: >> - av_log(log_ctx, AV_LOG_ERROR, >> + char *cl_name; >> + >> + av_log(log_ctx, AV_LOG_WARNING, >> "Channel layout is not set in stream %d, and could not " >> "be guessed from the number of channels (%d)\n", >> st_index, dec_par->ch_layout.nb_channels); >> - return AVERROR(EINVAL); >> + cl_name = av_asprintf("%dC", dec_par->ch_layout.nb_channels); >> + av_channel_layout_from_string(&chl, cl_name); >> + free(cl_name); > > 1. Wrong deallocator. > 2. The allocation is completely unnecessary: One can just use snprintf > with a big enough (yet still small) buffer. > 3. But even that is unnecessary: Just set chl = (AVChannelLayout){ > .order = AV_CHANNEL_ORDER_UNSPEC, .nb_channels = > dec_par->ch_layout.nb_channels }. He doesn't even need to do that because it's already set. av_channel_layout_default() will give set the output layout as an UNSPEC one with nb_channels amount of channels if it can't find a named native layout for it. There is however a problem with this patch as is, and it's the next printed warning now that he removed the return. After this change it will mention a layout was guessed when one wasn't. So this patch should simply change the "return AVERROR(EINVAL)" into another "return av_channel_layout_copy(&dec_par->ch_layout, &chl);"
On 10/31/22, Paul B Mahol <onemda@gmail.com> wrote: > Patch attached. > Fixed patch attached.
On 10/31/22, Paul B Mahol <onemda@gmail.com> wrote: > On 10/31/22, Paul B Mahol <onemda@gmail.com> wrote: >> Patch attached. >> > > Fixed patch attached. > Will apply immediately.
From 975a677906256f5f7a6da876a1eede21c5cb2a8e Mon Sep 17 00:00:00 2001 From: Paul B Mahol <onemda@gmail.com> Date: Mon, 31 Oct 2022 12:55:17 +0100 Subject: [PATCH] avfilter/src_movie: support unknown channel layouts Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavfilter/src_movie.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libavfilter/src_movie.c b/libavfilter/src_movie.c index 711854c23c..2e30e54ad2 100644 --- a/libavfilter/src_movie.c +++ b/libavfilter/src_movie.c @@ -196,11 +196,15 @@ static int guess_channel_layout(MovieStream *st, int st_index, void *log_ctx) av_channel_layout_default(&chl, dec_par->ch_layout.nb_channels); if (!KNOWN(&chl)) { - av_log(log_ctx, AV_LOG_ERROR, + char *cl_name; + + av_log(log_ctx, AV_LOG_WARNING, "Channel layout is not set in stream %d, and could not " "be guessed from the number of channels (%d)\n", st_index, dec_par->ch_layout.nb_channels); - return AVERROR(EINVAL); + cl_name = av_asprintf("%dC", dec_par->ch_layout.nb_channels); + av_channel_layout_from_string(&chl, cl_name); + free(cl_name); } av_channel_layout_describe(&chl, buf, sizeof(buf)); -- 2.37.2