Le sextidi 6 nivôse, an CCXXV, Marton Balint a écrit : > Current code returned the number of channels as channel layout in that case, > and if nret is not set then unknown layouts are typically not supported.
Good catch.
Let me see if I got this correctly:
av_get_channel_layout("2c") = stereo
av_get_channel_layout("2C") = 0
av_get_channel_layout("13c") = 0
av_get_channel_layout("13C") = 0
av_get_extended_channel_layout("2c") = stereo / 2
av_get_extended_channel_layout("2C") = 0 / 2
av_get_extended_channel_layout("13c") = EINVAL
av_get_extended_channel_layout("13C") = 0 / 13
ff_parse_channel_layout():
before after
nret == NULL 2c stereo stereo
nret == NULL 2C EINVAL EINVAL
nret == NULL 13c (int64)13 = FL+FC+LF EINVAL
nret == NULL 13C EINVAL EINVAL
nret != NULL 2c stereo / 2 stereo / 2
nret != NULL 2C EINVAL 0 / 2
nret != NULL 13c 0 / 13 EINVAL
nret != NULL 13C EINVAL 0 / 13
Do we agree?
> Also use the common parsing code. This breaks a very specific case, using
> af_pan with an unknown channel layout such as '13c', from now on, only '13C'
> will work.
I think you could easily add a temporary workaround and a warning.
(I suggest, from now on, we flag temporary workarounds and warnings with
a standardized comment: "/* [TEMPORARY 2016-12 -> 2017-12]*/"; that way,
we can find them using git grep.)
> Signed-off-by: Marton Balint <c...@passwd.hu>
> ---
> libavfilter/formats.c | 20 ++++++--------------
> libavfilter/tests/formats.c | 3 +++
> tests/ref/fate/filter-formats | 5 ++++-
> 3 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> index 840780d..8be606f 100644
> --- a/libavfilter/formats.c
> +++ b/libavfilter/formats.c
> @@ -662,24 +662,16 @@ int ff_parse_sample_rate(int *ret, const char *arg,
> void *log_ctx)
> int ff_parse_channel_layout(int64_t *ret, int *nret, const char *arg,
> void *log_ctx)
> {
> - char *tail;
> int64_t chlayout;
> + int nb_channels;
>
> - chlayout = av_get_channel_layout(arg);
> - if (chlayout == 0) {
> - chlayout = strtol(arg, &tail, 10);
> - if (!(*tail == '\0' || *tail == 'c' && *(tail + 1) == '\0') ||
> chlayout <= 0 || chlayout > 63) {
This is losing the >63 test since av_get_extended_channel_layout() tests
for >64. lavfi can not deal with channel layouts with channel #63 set,
because the bit serves to mark channel counts disguised as layouts;
channel #63 is not defined in lavu anyway. Unknown channel layouts
should work with even more than 64 channels, but that would require
testing.
> - av_log(log_ctx, AV_LOG_ERROR, "Invalid channel layout '%s'\n",
> arg);
> - return AVERROR(EINVAL);
> - }
> - if (nret) {
> - *nret = chlayout;
> - *ret = 0;
> - return 0;
> - }
> + if (av_get_extended_channel_layout(arg, &chlayout, &nb_channels) < 0 ||
> (!chlayout && !nret)) {
> + av_log(log_ctx, AV_LOG_ERROR, "Invalid channel layout '%s'\n", arg);
Could you split the test to distinguish the warning?
av_get_extended_channel_layout < 0 -> "Invalid channel layout"
(!chlayout && !nret) -> "channel count without layout unsupported"
(it also makes it easier to add the temporary workaround)
> + return AVERROR(EINVAL);
> }
> *ret = chlayout;
> if (nret)
> - *nret = av_get_channel_layout_nb_channels(chlayout);
> + *nret = nb_channels;
> +
> return 0;
> }
> diff --git a/libavfilter/tests/formats.c b/libavfilter/tests/formats.c
> index 0e8ba4a..5450742 100644
> --- a/libavfilter/tests/formats.c
> +++ b/libavfilter/tests/formats.c
> @@ -39,6 +39,9 @@ int main(void)
> "-1c",
> "60c",
> "65c",
> + "2C",
> + "60C",
> + "65C",
> "5.1",
> "stereo",
> "1+1+1+1",
> diff --git a/tests/ref/fate/filter-formats b/tests/ref/fate/filter-formats
> index 4c303d8..17ff5b2 100644
> --- a/tests/ref/fate/filter-formats
> +++ b/tests/ref/fate/filter-formats
> @@ -75,8 +75,11 @@ quad(side)
> 0 = ff_parse_channel_layout(0000000000000004, 1, 1c);
> 0 = ff_parse_channel_layout(0000000000000003, 2, 2c);
> -1 = ff_parse_channel_layout(FFFFFFFFFFFFFFFF, -1, -1c);
> -0 = ff_parse_channel_layout(0000000000000000, 60, 60c);
> +-1 = ff_parse_channel_layout(FFFFFFFFFFFFFFFF, -1, 60c);
> -1 = ff_parse_channel_layout(FFFFFFFFFFFFFFFF, -1, 65c);
> +0 = ff_parse_channel_layout(0000000000000000, 2, 2C);
> +0 = ff_parse_channel_layout(0000000000000000, 60, 60C);
> +-1 = ff_parse_channel_layout(FFFFFFFFFFFFFFFF, -1, 65C);
> 0 = ff_parse_channel_layout(000000000000003F, 6, 5.1);
> 0 = ff_parse_channel_layout(0000000000000003, 2, stereo);
> 0 = ff_parse_channel_layout(0000000000000001, 1, 1+1+1+1);
Regards,
--
Nicolas George
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
