diff mbox series

[FFmpeg-devel,v2] avcodec/bsf: switch to av_get_token to parse bsf list string

Message ID 20210703165458.1749-1-ffmpeg@gyani.pro
State Accepted
Commit 301d275301d72387732ccdc526babaf984ddafe5
Headers show
Series [FFmpeg-devel,v2] avcodec/bsf: switch to av_get_token to parse bsf list string | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Gyan Doshi July 3, 2021, 4:54 p.m. UTC 
The recently added setts bsf makes use of the eval API whose
expressions can contain commas. The existing parsing in
av_bsf_list_parse_str() uses av_strtok to naively split
the string at commas, thus preventing the use of setts filter
with expressions containing commas.

av_get_token can work with escaped commas, allowing full use of setts.
---
 libavcodec/bsf.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

Comments

James Almer July 3, 2021, 5:35 p.m. UTC | #1 
On 7/3/2021 1:54 PM, Gyan Doshi wrote:
> The recently added setts bsf makes use of the eval API whose
> expressions can contain commas. The existing parsing in
> av_bsf_list_parse_str() uses av_strtok to naively split
> the string at commas, thus preventing the use of setts filter
> with expressions containing commas.
> 
> av_get_token can work with escaped commas, allowing full use of setts.
> ---
>   libavcodec/bsf.c | 15 ++++-----------
>   1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c
> index 9d67ea5395..0305244f8d 100644
> --- a/libavcodec/bsf.c
> +++ b/libavcodec/bsf.c
> @@ -520,7 +520,6 @@ static int bsf_parse_single(char *str, AVBSFList *bsf_lst)
>   int av_bsf_list_parse_str(const char *str, AVBSFContext **bsf_lst)
>   {
>       AVBSFList *lst;
> -    char *bsf_str, *buf, *dup, *saveptr;
>       int ret;
>   
>       if (!str)
> @@ -530,24 +529,18 @@ int av_bsf_list_parse_str(const char *str, AVBSFContext **bsf_lst)
>       if (!lst)
>           return AVERROR(ENOMEM);
>   
> -    if (!(dup = buf = av_strdup(str))) {
> -        ret = AVERROR(ENOMEM);
> -        goto end;
> -    }
> -
> -    while (bsf_str = av_strtok(buf, ",", &saveptr)) {
> +    do {
> +        char *bsf_str = av_get_token(&str, ",");
>           ret = bsf_parse_single(bsf_str, lst);
> +        av_free(bsf_str);
>           if (ret < 0)
>               goto end;
> -
> -        buf = NULL;
> -    }
> +    } while (*str && *++str);
>   
>       ret = av_bsf_list_finalize(&lst, bsf_lst);
>   end:
>       if (ret < 0)
>           av_bsf_list_free(&lst);
> -    av_free(dup);
>       return ret;
>   }

Should be ok.
Gyan Doshi July 4, 2021, 4:26 a.m. UTC | #2 
On 2021-07-03 23:05, James Almer wrote:
> On 7/3/2021 1:54 PM, Gyan Doshi wrote:
>> The recently added setts bsf makes use of the eval API whose
>> expressions can contain commas. The existing parsing in
>> av_bsf_list_parse_str() uses av_strtok to naively split
>> the string at commas, thus preventing the use of setts filter
>> with expressions containing commas.
>>
>> av_get_token can work with escaped commas, allowing full use of setts.
>> ---
>>   libavcodec/bsf.c | 15 ++++-----------
>>   1 file changed, 4 insertions(+), 11 deletions(-)
>>
>> diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c
>> index 9d67ea5395..0305244f8d 100644
>> --- a/libavcodec/bsf.c
>> +++ b/libavcodec/bsf.c
>> @@ -520,7 +520,6 @@ static int bsf_parse_single(char *str, AVBSFList 
>> *bsf_lst)
>>   int av_bsf_list_parse_str(const char *str, AVBSFContext **bsf_lst)
>>   {
>>       AVBSFList *lst;
>> -    char *bsf_str, *buf, *dup, *saveptr;
>>       int ret;
>>         if (!str)
>> @@ -530,24 +529,18 @@ int av_bsf_list_parse_str(const char *str, 
>> AVBSFContext **bsf_lst)
>>       if (!lst)
>>           return AVERROR(ENOMEM);
>>   -    if (!(dup = buf = av_strdup(str))) {
>> -        ret = AVERROR(ENOMEM);
>> -        goto end;
>> -    }
>> -
>> -    while (bsf_str = av_strtok(buf, ",", &saveptr)) {
>> +    do {
>> +        char *bsf_str = av_get_token(&str, ",");
>>           ret = bsf_parse_single(bsf_str, lst);
>> +        av_free(bsf_str);
>>           if (ret < 0)
>>               goto end;
>> -
>> -        buf = NULL;
>> -    }
>> +    } while (*str && *++str);
>>         ret = av_bsf_list_finalize(&lst, bsf_lst);
>>   end:
>>       if (ret < 0)
>>           av_bsf_list_free(&lst);
>> -    av_free(dup);
>>       return ret;
>>   }
>
> Should be ok.

Thanks. Pushed as 301d275301d72387732ccdc526babaf984ddafe5

Gyan
diff mbox series

Patch

diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c
index 9d67ea5395..0305244f8d 100644
--- a/libavcodec/bsf.c
+++ b/libavcodec/bsf.c
@@ -520,7 +520,6 @@  static int bsf_parse_single(char *str, AVBSFList *bsf_lst)
 int av_bsf_list_parse_str(const char *str, AVBSFContext **bsf_lst)
 {
     AVBSFList *lst;
-    char *bsf_str, *buf, *dup, *saveptr;
     int ret;
 
     if (!str)
@@ -530,24 +529,18 @@  int av_bsf_list_parse_str(const char *str, AVBSFContext **bsf_lst)
     if (!lst)
         return AVERROR(ENOMEM);
 
-    if (!(dup = buf = av_strdup(str))) {
-        ret = AVERROR(ENOMEM);
-        goto end;
-    }
-
-    while (bsf_str = av_strtok(buf, ",", &saveptr)) {
+    do {
+        char *bsf_str = av_get_token(&str, ",");
         ret = bsf_parse_single(bsf_str, lst);
+        av_free(bsf_str);
         if (ret < 0)
             goto end;
-
-        buf = NULL;
-    }
+    } while (*str && *++str);
 
     ret = av_bsf_list_finalize(&lst, bsf_lst);
 end:
     if (ret < 0)
         av_bsf_list_free(&lst);
-    av_free(dup);
     return ret;
 }