| 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 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
> 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
> 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
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,
> 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
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
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,
> 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
> 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 --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); }