diff mbox series

[FFmpeg-devel,v1,4/4] avformat/hlsenc: use av_asprintf()

Message ID 20200326135700.11167-4-lance.lmwang@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel,v1,1/4] avformat/hlsenc: remove the first slash of the relative path line in the master m3u8 file
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Limin Wang March 26, 2020, 1:57 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/hlsenc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Steven Liu March 29, 2020, 8:32 a.m. UTC | #1
> 2020年3月26日 下午9:57,lance.lmwang@gmail.com 写道:
> 
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavformat/hlsenc.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index d7b9c0e20a..694dab42dd 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -2950,13 +2950,11 @@ static int hls_init(AVFormatContext *s)
>                 if (ret < 0)
>                     goto fail;
>             } else {
> -                vs->vtt_m3u8_name = av_malloc(vtt_basename_size);
> +                vs->vtt_m3u8_name = av_asprintf("%s_vtt.m3u8", vs->vtt_basename);
>                 if (!vs->vtt_m3u8_name) {
>                     ret = AVERROR(ENOMEM);
>                     goto fail;
>                 }
> -                strcpy(vs->vtt_m3u8_name, vs->vtt_basename);
> -                av_strlcat(vs->vtt_m3u8_name, "_vtt.m3u8", vtt_basename_size);
>             }
>             av_strlcat(vs->vtt_basename, vtt_pattern, vtt_basename_size);
>         }
> -- 
> 2.21.0
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".


LGTM


Thanks

Steven Liu
Steven Liu April 8, 2020, 2:23 p.m. UTC | #2
> 2020年3月26日 下午9:57,lance.lmwang@gmail.com 写道:
> 
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavformat/hlsenc.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index d7b9c0e20a..694dab42dd 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -2950,13 +2950,11 @@ static int hls_init(AVFormatContext *s)
>                 if (ret < 0)
>                     goto fail;
>             } else {
> -                vs->vtt_m3u8_name = av_malloc(vtt_basename_size);
> +                vs->vtt_m3u8_name = av_asprintf("%s_vtt.m3u8", vs->vtt_basename);
As mkver suggestion, perhaps use bprint is better, is it?
>                 if (!vs->vtt_m3u8_name) {
>                     ret = AVERROR(ENOMEM);
>                     goto fail;
>                 }
> -                strcpy(vs->vtt_m3u8_name, vs->vtt_basename);
> -                av_strlcat(vs->vtt_m3u8_name, "_vtt.m3u8", vtt_basename_size);
>             }
>             av_strlcat(vs->vtt_basename, vtt_pattern, vtt_basename_size);
>         }
> -- 
> 2.21.0
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Thanks

Steven Liu
Nicolas George April 8, 2020, 2:32 p.m. UTC | #3
Steven Liu (12020-04-08):
> As mkver suggestion, perhaps use bprint is better, is it?

It does not seem like a situation where BPrint is beneficial.

Other filename handling functions in this HLS code would, though.

Regards,
Steven Liu April 8, 2020, 2:39 p.m. UTC | #4
> 2020年4月8日 下午10:32,Nicolas George <george@nsup.org> 写道:
> 
> Steven Liu (12020-04-08):
>> As mkver suggestion, perhaps use bprint is better, is it?
> 
> It does not seem like a situation where BPrint is beneficial.

I cannot get the mean of BPrint where should or when should use it, maybe you can introduce it,
I always think BPrint is used to instead of av_[a]sprintf, 
because I saw mkver submit 3 patches to modify from snprintf to BPrint

> 
> Other filename handling functions in this HLS code would, though.
> 
> Regards,
> 
> -- 
>  Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Thanks

Steven Liu
Andreas Rheinhardt April 8, 2020, 2:39 p.m. UTC | #5
Steven Liu:
> 
> 
>> 2020年3月26日 下午9:57,lance.lmwang@gmail.com 写道:
>>
>> From: Limin Wang <lance.lmwang@gmail.com>
>>
>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> ---
>> libavformat/hlsenc.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index d7b9c0e20a..694dab42dd 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -2950,13 +2950,11 @@ static int hls_init(AVFormatContext *s)
>>                 if (ret < 0)
>>                     goto fail;
>>             } else {
>> -                vs->vtt_m3u8_name = av_malloc(vtt_basename_size);
>> +                vs->vtt_m3u8_name = av_asprintf("%s_vtt.m3u8", vs->vtt_basename);
> As mkver suggestion, perhaps use bprint is better, is it?

The situation here is completely different: The lifetime of the strings
for which I used an AVBPrint was just a part of a function call, ergo it
can be replaced with a buffer on the stack (if the string is small
enough). But the lifetime of vs->vtt_m3u8_name extends beyond
hls_init(), ergo it can't be put on the stack (as is implicitly done
when using an AVBPrint). It needs to be allocated on the heap.

- Andreas
Nicolas George April 8, 2020, 2:48 p.m. UTC | #6
Steven Liu (12020-04-08):
> I cannot get the mean of BPrint where should or when should use it, maybe you can introduce it,
> I always think BPrint is used to instead of av_[a]sprintf, 
> because I saw mkver submit 3 patches to modify from snprintf to BPrint

BPrint brings three benefits:

- It keeps track of the string length. It is therefore useful when you
  have several snprintf(). If you have only one, this benefit is
  negated.

- It switches to dynamic allocation if the buffer is too small. If your
  string does not have a bounded size and keeping it entirely is
  important, then replacing a buffer with an arbitrary high limit with
  BPrint is useful.

- Before switching to dynamic allocation, it uses a local buffer in the
  structure (probably on stack). If the string does not need to outlive
  its creation very far, it works. If we need to keep the string, a
  dynamic allocation is necessary anyway.

For this case: (1) there is only one snprintf(), (2) we don't use a
fixed-size buffer and (3) we'll keep the string.

Regards,
Steven Liu April 8, 2020, 3:42 p.m. UTC | #7
> 2020年4月8日 下午10:48,Nicolas George <george@nsup.org> 写道:
> 
> Steven Liu (12020-04-08):
>> I cannot get the mean of BPrint where should or when should use it, maybe you can introduce it,
>> I always think BPrint is used to instead of av_[a]sprintf, 
>> because I saw mkver submit 3 patches to modify from snprintf to BPrint
> 
> BPrint brings three benefits:
> 
> - It keeps track of the string length. It is therefore useful when you
>  have several snprintf(). If you have only one, this benefit is
>  negated.
> 
> - It switches to dynamic allocation if the buffer is too small. If your
>  string does not have a bounded size and keeping it entirely is
>  important, then replacing a buffer with an arbitrary high limit with
>  BPrint is useful.
> 
> - Before switching to dynamic allocation, it uses a local buffer in the
>  structure (probably on stack). If the string does not need to outlive
>  its creation very far, it works. If we need to keep the string, a
>  dynamic allocation is necessary anyway.
> 
> For this case: (1) there is only one snprintf(), (2) we don't use a
> fixed-size buffer and (3) we'll keep the string.

very clear, I understand it now, Thanks Nicolas.
> 
> Regards,
> 
> -- 
>  Nicolas George

Thanks

Steven Liu
Steven Liu April 8, 2020, 3:51 p.m. UTC | #8
> 2020年3月29日 下午4:32,Steven Liu <lq@chinaffmpeg.org> 写道:
> 
> 
>> 2020年3月26日 下午9:57,lance.lmwang@gmail.com 写道:
>> 
>> From: Limin Wang <lance.lmwang@gmail.com>
>> 
>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> ---
>> libavformat/hlsenc.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index d7b9c0e20a..694dab42dd 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -2950,13 +2950,11 @@ static int hls_init(AVFormatContext *s)
>>                if (ret < 0)
>>                    goto fail;
>>            } else {
>> -                vs->vtt_m3u8_name = av_malloc(vtt_basename_size);
>> +                vs->vtt_m3u8_name = av_asprintf("%s_vtt.m3u8", vs->vtt_basename);
>>                if (!vs->vtt_m3u8_name) {
>>                    ret = AVERROR(ENOMEM);
>>                    goto fail;
>>                }
>> -                strcpy(vs->vtt_m3u8_name, vs->vtt_basename);
>> -                av_strlcat(vs->vtt_m3u8_name, "_vtt.m3u8", vtt_basename_size);
>>            }
>>            av_strlcat(vs->vtt_basename, vtt_pattern, vtt_basename_size);
>>        }
>> -- 
>> 2.21.0
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> 
> LGTM
Will apply
> 
> 
> Thanks
> 
> Steven Liu
> 

Thanks

Steven Liu
diff mbox series

Patch

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index d7b9c0e20a..694dab42dd 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -2950,13 +2950,11 @@  static int hls_init(AVFormatContext *s)
                 if (ret < 0)
                     goto fail;
             } else {
-                vs->vtt_m3u8_name = av_malloc(vtt_basename_size);
+                vs->vtt_m3u8_name = av_asprintf("%s_vtt.m3u8", vs->vtt_basename);
                 if (!vs->vtt_m3u8_name) {
                     ret = AVERROR(ENOMEM);
                     goto fail;
                 }
-                strcpy(vs->vtt_m3u8_name, vs->vtt_basename);
-                av_strlcat(vs->vtt_m3u8_name, "_vtt.m3u8", vtt_basename_size);
             }
             av_strlcat(vs->vtt_basename, vtt_pattern, vtt_basename_size);
         }