| Message ID | 1591624871-18131-1-git-send-email-lance.lmwang@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel] avfilter/f_metadata: use AVBPrint API to remove buf size limitation | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
lance.lmwang@gmail.com (12020-06-08): > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > libavfilter/f_metadata.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/libavfilter/f_metadata.c b/libavfilter/f_metadata.c > index 598257b15b..808782d31a 100644 > --- a/libavfilter/f_metadata.c > +++ b/libavfilter/f_metadata.c > @@ -31,6 +31,7 @@ > #include "libavutil/internal.h" > #include "libavutil/opt.h" > #include "libavutil/timestamp.h" > +#include "libavutil/bprint.h" > #include "libavformat/avio.h" > #include "avfilter.h" > #include "audio.h" > @@ -195,9 +196,11 @@ static void print_file(AVFilterContext *ctx, const char *msg, ...) > > va_start(argument_list, msg); > if (msg) { > - char buf[128]; > - vsnprintf(buf, sizeof(buf), msg, argument_list); > - avio_write(s->avio_context, buf, av_strnlen(buf, sizeof(buf))); > + AVBPrint buf; > + av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED); > + av_vbprintf(&buf, msg, argument_list); > + avio_write(s->avio_context, buf.str, buf.len); Error check missing. > + av_bprint_finalize(&buf, NULL); > } > va_end(argument_list); > } Regards,
On Mon, Jun 08, 2020 at 04:02:22PM +0200, Nicolas George wrote: > lance.lmwang@gmail.com (12020-06-08): > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > libavfilter/f_metadata.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/libavfilter/f_metadata.c b/libavfilter/f_metadata.c > > index 598257b15b..808782d31a 100644 > > --- a/libavfilter/f_metadata.c > > +++ b/libavfilter/f_metadata.c > > @@ -31,6 +31,7 @@ > > #include "libavutil/internal.h" > > #include "libavutil/opt.h" > > #include "libavutil/timestamp.h" > > +#include "libavutil/bprint.h" > > #include "libavformat/avio.h" > > #include "avfilter.h" > > #include "audio.h" > > @@ -195,9 +196,11 @@ static void print_file(AVFilterContext *ctx, const char *msg, ...) > > > > va_start(argument_list, msg); > > if (msg) { > > - char buf[128]; > > - vsnprintf(buf, sizeof(buf), msg, argument_list); > > - avio_write(s->avio_context, buf, av_strnlen(buf, sizeof(buf))); > > + AVBPrint buf; > > + av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED); > > + av_vbprintf(&buf, msg, argument_list); > > > + avio_write(s->avio_context, buf.str, buf.len); > > Error check missing. For the print_file is void, so I haven't considered to add error check, can we just print one error message if av_bprint_is_complete checking failed? > > > + av_bprint_finalize(&buf, NULL); > > } > > va_end(argument_list); > > } > > Regards, > > -- > Nicolas George
lance.lmwang@gmail.com (12020-06-08): > For the print_file is void, so I haven't considered to add error > check, can we just print one error message if av_bprint_is_complete > checking failed? Assume the log goes to /dev/null. If there is an error, especially if there is a risk of data loss or data corruption, an error return is necessary. If it requires changing the type of a function, so be it. But you can do better than using BPrint: the existing code writes into a buffer then writes the buffer to avio, even though we have avio_printf(). Regards,
diff --git a/libavfilter/f_metadata.c b/libavfilter/f_metadata.c index 598257b15b..808782d31a 100644 --- a/libavfilter/f_metadata.c +++ b/libavfilter/f_metadata.c @@ -31,6 +31,7 @@ #include "libavutil/internal.h" #include "libavutil/opt.h" #include "libavutil/timestamp.h" +#include "libavutil/bprint.h" #include "libavformat/avio.h" #include "avfilter.h" #include "audio.h" @@ -195,9 +196,11 @@ static void print_file(AVFilterContext *ctx, const char *msg, ...) va_start(argument_list, msg); if (msg) { - char buf[128]; - vsnprintf(buf, sizeof(buf), msg, argument_list); - avio_write(s->avio_context, buf, av_strnlen(buf, sizeof(buf))); + AVBPrint buf; + av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED); + av_vbprintf(&buf, msg, argument_list); + avio_write(s->avio_context, buf.str, buf.len); + av_bprint_finalize(&buf, NULL); } va_end(argument_list); }