diff mbox

[FFmpeg-devel,PATCHv2,3/3] avfilter/formats: do not allow unknown layouts in ff_parse_channel_layout if nret is not set

Message ID 20170101235736.27213-1-cus@passwd.hu
State Accepted
Commit 977fd8841921f0109bee220e1f99dd662c166ae1
Headers show

Commit Message

Marton Balint Jan. 1, 2017, 11:57 p.m. UTC
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.

Also use the common parsing code. Use a temporary workaround to parse an
unknown channel layout such as '13c', after a 1 year grace period only '13C'
will work.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/formats.c         | 26 +++++++++++++++-----------
 libavfilter/tests/formats.c   |  3 +++
 tests/ref/fate/filter-formats |  3 +++
 3 files changed, 21 insertions(+), 11 deletions(-)

Comments

Marton Balint Jan. 10, 2017, 10:43 p.m. UTC | #1
On Mon, 2 Jan 2017, Marton Balint wrote:

> 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.
>
> Also use the common parsing code. Use a temporary workaround to parse an
> unknown channel layout such as '13c', after a 1 year grace period only '13C'
> will work.
>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
> libavfilter/formats.c         | 26 +++++++++++++++-----------
> libavfilter/tests/formats.c   |  3 +++
> tests/ref/fate/filter-formats |  3 +++
> 3 files changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> index 840780d..d4de862 100644
> --- a/libavfilter/formats.c
> +++ b/libavfilter/formats.c
> @@ -664,22 +664,26 @@ int ff_parse_channel_layout(int64_t *ret, int *nret, const char *arg,
> {
>     char *tail;
>     int64_t chlayout;
> -
> -    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) {
> +    int nb_channels;
> +
> +    if (av_get_extended_channel_layout(arg, &chlayout, &nb_channels) < 0) {
> +        /* [TEMPORARY 2016-12 -> 2017-12]*/
> +        nb_channels = strtol(arg, &tail, 10);
> +        if (!errno && *tail == 'c' && *(tail + 1) == '\0' && nb_channels > 0 && nb_channels < 64) {
> +            chlayout = 0;
> +            av_log(log_ctx, AV_LOG_WARNING, "Deprecated channel count specification '%s'. This will stop working in releases made in 2018 and after.\n", arg);
> +        } else {
>             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 (!chlayout && !nret) {
> +        av_log(log_ctx, AV_LOG_ERROR, "Unknown channel layout '%s' is not supported.\n", arg);
> +        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..ea85eed 100644
> --- a/tests/ref/fate/filter-formats
> +++ b/tests/ref/fate/filter-formats
> @@ -77,6 +77,9 @@ quad(side)
> -1 = ff_parse_channel_layout(FFFFFFFFFFFFFFFF, -1, -1c);
> 0 = ff_parse_channel_layout(0000000000000000, 60, 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);

Ping...

Thanks,
Marton
Marton Balint Jan. 22, 2017, 3:15 p.m. UTC | #2
On Tue, 10 Jan 2017, Marton Balint wrote:

>
> On Mon, 2 Jan 2017, Marton Balint wrote:
>
>> 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.
>>
>> Also use the common parsing code. Use a temporary workaround to parse an
>> unknown channel layout such as '13c', after a 1 year grace period only 
> '13C'
>> will work.
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>> libavfilter/formats.c         | 26 +++++++++++++++-----------
>> libavfilter/tests/formats.c   |  3 +++
>> tests/ref/fate/filter-formats |  3 +++
>> 3 files changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
>> index 840780d..d4de862 100644
>> --- a/libavfilter/formats.c
>> +++ b/libavfilter/formats.c
>> @@ -664,22 +664,26 @@ int ff_parse_channel_layout(int64_t *ret, int *nret, 
> const char *arg,
>> {
>>     char *tail;
>>     int64_t chlayout;
>> -
>> -    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) {
>> +    int nb_channels;
>> +
>> +    if (av_get_extended_channel_layout(arg, &chlayout, &nb_channels) < 0) 
> {
>> +        /* [TEMPORARY 2016-12 -> 2017-12]*/
>> +        nb_channels = strtol(arg, &tail, 10);
>> +        if (!errno && *tail == 'c' && *(tail + 1) == '\0' && nb_channels > 
> 0 && nb_channels < 64) {
>> +            chlayout = 0;
>> +            av_log(log_ctx, AV_LOG_WARNING, "Deprecated channel count 
> specification '%s'. This will stop working in releases made in 2018 and 
> after.\n", arg);
>> +        } else {
>>             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 (!chlayout && !nret) {
>> +        av_log(log_ctx, AV_LOG_ERROR, "Unknown channel layout '%s' is not 
> supported.\n", arg);
>> +        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..ea85eed 100644
>> --- a/tests/ref/fate/filter-formats
>> +++ b/tests/ref/fate/filter-formats
>> @@ -77,6 +77,9 @@ quad(side)
>> -1 = ff_parse_channel_layout(FFFFFFFFFFFFFFFF, -1, -1c);
>> 0 = ff_parse_channel_layout(0000000000000000, 60, 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);
>
> Ping...

I will apply the series in a day or two if I get no further comments.

Regards,
Marton
Marton Balint Jan. 24, 2017, 10:57 p.m. UTC | #3
On Sun, 22 Jan 2017, Marton Balint wrote:

>
> On Tue, 10 Jan 2017, Marton Balint wrote:
>
>>
>> On Mon, 2 Jan 2017, Marton Balint wrote:
>>
>>> 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.
>>>
>>> Also use the common parsing code. Use a temporary workaround to parse an
>>> unknown channel layout such as '13c', after a 1 year grace period only 
>> '13C'
>>> will work.
>>>
>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>> ---
>>> libavfilter/formats.c         | 26 +++++++++++++++-----------
>>> libavfilter/tests/formats.c   |  3 +++
>>> tests/ref/fate/filter-formats |  3 +++
>>> 3 files changed, 21 insertions(+), 11 deletions(-)
>>>

[...]

>>> 0 = ff_parse_channel_layout(0000000000000001,  1, 1+1+1+1);
>>
>> Ping...
>
> I will apply the series in a day or two if I get no further comments.
>

Series applied.

Regards,
Marton
diff mbox

Patch

diff --git a/libavfilter/formats.c b/libavfilter/formats.c
index 840780d..d4de862 100644
--- a/libavfilter/formats.c
+++ b/libavfilter/formats.c
@@ -664,22 +664,26 @@  int ff_parse_channel_layout(int64_t *ret, int *nret, const char *arg,
 {
     char *tail;
     int64_t chlayout;
-
-    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) {
+    int nb_channels;
+
+    if (av_get_extended_channel_layout(arg, &chlayout, &nb_channels) < 0) {
+        /* [TEMPORARY 2016-12 -> 2017-12]*/
+        nb_channels = strtol(arg, &tail, 10);
+        if (!errno && *tail == 'c' && *(tail + 1) == '\0' && nb_channels > 0 && nb_channels < 64) {
+            chlayout = 0;
+            av_log(log_ctx, AV_LOG_WARNING, "Deprecated channel count specification '%s'. This will stop working in releases made in 2018 and after.\n", arg);
+        } else {
             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 (!chlayout && !nret) {
+        av_log(log_ctx, AV_LOG_ERROR, "Unknown channel layout '%s' is not supported.\n", arg);
+        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..ea85eed 100644
--- a/tests/ref/fate/filter-formats
+++ b/tests/ref/fate/filter-formats
@@ -77,6 +77,9 @@  quad(side)
 -1 = ff_parse_channel_layout(FFFFFFFFFFFFFFFF, -1, -1c);
 0 = ff_parse_channel_layout(0000000000000000, 60, 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);