| Message ID | 20190805213454.9548-3-cus@passwd.hu |
|---|---|
| State | Superseded |
| Headers | show |
Marton Balint (12019-08-05): > We do this by switching to AVBPrint. > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > doc/APIchanges | 3 +++ > libavformat/avio.h | 5 ++++- > libavformat/aviobuf.c | 15 ++++++++++----- > libavformat/version.h | 2 +- > 4 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 0b19fb067d..fe36c34b90 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > API changes, most recent first: > > +2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h > + 4K limit removed from avio_printf. > + > 2019-08-xx - xxxxxxxxxx - lavf 58.31.100 - avio.h > Add avio_print_n_strings and avio_print. > > diff --git a/libavformat/avio.h b/libavformat/avio.h > index ca08907917..f2051da18d 100644 > --- a/libavformat/avio.h > +++ b/libavformat/avio.h > @@ -571,7 +571,10 @@ int64_t avio_size(AVIOContext *s); > */ > int avio_feof(AVIOContext *s); > > -/** @warning Writes up to 4 KiB per call */ > +/** > + * Writes a formatted string to the context. > + * @return number of bytes written, < 0 on error. > + */ > int avio_printf(AVIOContext *s, const char *fmt, ...) av_printf_format(2, 3); > > /** > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c > index c37b056b64..007fba6410 100644 > --- a/libavformat/aviobuf.c > +++ b/libavformat/aviobuf.c > @@ -1215,14 +1215,19 @@ int avio_closep(AVIOContext **s) > int avio_printf(AVIOContext *s, const char *fmt, ...) > { > va_list ap; > - char buf[4096]; /* update doc entry in avio.h if changed */ > - int ret; > + AVBPrint bp; > > + av_bprint_init(&bp, 0, INT_MAX); INT_MAX? Should be -1 or AV_BPRINT_SIZE_UNLIMITED. > va_start(ap, fmt); > - ret = vsnprintf(buf, sizeof(buf), fmt, ap); > + av_vbprintf(&bp, fmt, ap); > va_end(ap); > - avio_write(s, buf, strlen(buf)); > - return ret; > + if (!av_bprint_is_complete(&bp)) { > + av_bprint_finalize(&bp, NULL); > + return AVERROR(ENOMEM); > + } > + avio_write(s, bp.str, bp.len); > + av_bprint_finalize(&bp, NULL); > + return bp.len; > } > > int avio_print_n_strings(AVIOContext *s, int nb_strings, ...) > diff --git a/libavformat/version.h b/libavformat/version.h > index feceaedc08..9814db8633 100644 > --- a/libavformat/version.h > +++ b/libavformat/version.h > @@ -33,7 +33,7 @@ > // Also please add any ticket numbers that you believe might be affected here > #define LIBAVFORMAT_VERSION_MAJOR 58 > #define LIBAVFORMAT_VERSION_MINOR 31 > -#define LIBAVFORMAT_VERSION_MICRO 100 > +#define LIBAVFORMAT_VERSION_MICRO 101 > > #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ > LIBAVFORMAT_VERSION_MINOR, \ Regards,
On Tue, 6 Aug 2019, Nicolas George wrote: > Marton Balint (12019-08-05): >> We do this by switching to AVBPrint. >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> doc/APIchanges | 3 +++ >> libavformat/avio.h | 5 ++++- >> libavformat/aviobuf.c | 15 ++++++++++----- >> libavformat/version.h | 2 +- >> 4 files changed, 18 insertions(+), 7 deletions(-) >> >> diff --git a/doc/APIchanges b/doc/APIchanges >> index 0b19fb067d..fe36c34b90 100644 >> --- a/doc/APIchanges >> +++ b/doc/APIchanges >> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 >> >> API changes, most recent first: >> >> +2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h >> + 4K limit removed from avio_printf. >> + >> 2019-08-xx - xxxxxxxxxx - lavf 58.31.100 - avio.h >> Add avio_print_n_strings and avio_print. >> >> diff --git a/libavformat/avio.h b/libavformat/avio.h >> index ca08907917..f2051da18d 100644 >> --- a/libavformat/avio.h >> +++ b/libavformat/avio.h >> @@ -571,7 +571,10 @@ int64_t avio_size(AVIOContext *s); >> */ >> int avio_feof(AVIOContext *s); >> >> -/** @warning Writes up to 4 KiB per call */ >> +/** >> + * Writes a formatted string to the context. >> + * @return number of bytes written, < 0 on error. >> + */ >> int avio_printf(AVIOContext *s, const char *fmt, ...) av_printf_format(2, 3); >> >> /** >> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c >> index c37b056b64..007fba6410 100644 >> --- a/libavformat/aviobuf.c >> +++ b/libavformat/aviobuf.c >> @@ -1215,14 +1215,19 @@ int avio_closep(AVIOContext **s) >> int avio_printf(AVIOContext *s, const char *fmt, ...) >> { >> va_list ap; >> - char buf[4096]; /* update doc entry in avio.h if changed */ >> - int ret; >> + AVBPrint bp; >> > >> + av_bprint_init(&bp, 0, INT_MAX); > > INT_MAX? Should be -1 or AV_BPRINT_SIZE_UNLIMITED. The function returns an int so it can't signal writing longer strings that that. Regards, Marton
Marton Balint (12019-08-07): > The function returns an int so it can't signal writing longer strings that > that. Yet, replacing an arbitrary limit by another arbitrary limit, however high, seems very wrong. Regards,
On Thu, 8 Aug 2019, Nicolas George wrote: > Marton Balint (12019-08-07): >> The function returns an int so it can't signal writing longer strings that >> that. > > Yet, replacing an arbitrary limit by another arbitrary limit, however > high, seems very wrong. Current 4k limit is definitely causing errors, the users are not aware of it, because it is not intuitive. I don't see why increasing the limit to 2 GB which is something that we will hardly ever hit is worse than current status which causes actual issues. Are you against increasing the limit? Do you want to increase the limit to UINT_MAX (the AVBprint implementation limit) instead of INT_MAX? Or do you have some other suggestion? Thanks, Marton
On Thu, Aug 8, 2019 at 10:43 AM Nicolas George <george@nsup.org> wrote: > Marton Balint (12019-08-07): > > The function returns an int so it can't signal writing longer strings > that > > that. > > Yet, replacing an arbitrary limit by another arbitrary limit, however > high, seems very wrong. > > But function returns int and not int64_t. > 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".
Marton Balint (12019-08-08): > Current 4k limit is definitely causing errors, the users are not aware of > it, because it is not intuitive. > > I don't see why increasing the limit to 2 GB which is something that we will > hardly ever hit is worse than current status which causes actual issues. > > Are you against increasing the limit? > > Do you want to increase the limit to UINT_MAX (the AVBprint implementation > limit) instead of INT_MAX? > > Or do you have some other suggestion? Another possibility would be to use AV_BPRINT_SIZE_UNLIMITED (thus extending when AVBPrint is extended) but return FFMIN(actual_size, INT_MAX). But in the end, I just said that it felt wrong to have an arbitrary limit just because of a return value type. I do not oppose any solution. Regards,
On Sat, 10 Aug 2019, Nicolas George wrote: > Marton Balint (12019-08-08): >> Current 4k limit is definitely causing errors, the users are not aware of >> it, because it is not intuitive. >> >> I don't see why increasing the limit to 2 GB which is something that we will >> hardly ever hit is worse than current status which causes actual issues. >> >> Are you against increasing the limit? >> >> Do you want to increase the limit to UINT_MAX (the AVBprint implementation >> limit) instead of INT_MAX? >> >> Or do you have some other suggestion? > > Another possibility would be to use AV_BPRINT_SIZE_UNLIMITED (thus > extending when AVBPrint is extended) but return FFMIN(actual_size, > INT_MAX). > > But in the end, I just said that it felt wrong to have an arbitrary > limit just because of a return value type. I do not oppose any solution. Ok, I think I just keep it as is then. Anther thing that came to my mind is that the great benefit of avio_* functions is that the user don't have to check the return value with each call. But in order for the user to still be able to detect ENOMEM condition we could set the io context error flag to AVERROR(ENOMEM) as well, this way it won't get silently ignored. Regards Marton
diff --git a/doc/APIchanges b/doc/APIchanges index 0b19fb067d..fe36c34b90 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2017-10-21 API changes, most recent first: +2019-08-xx - xxxxxxxxxx - lavf 58.31.101 - avio.h + 4K limit removed from avio_printf. + 2019-08-xx - xxxxxxxxxx - lavf 58.31.100 - avio.h Add avio_print_n_strings and avio_print. diff --git a/libavformat/avio.h b/libavformat/avio.h index ca08907917..f2051da18d 100644 --- a/libavformat/avio.h +++ b/libavformat/avio.h @@ -571,7 +571,10 @@ int64_t avio_size(AVIOContext *s); */ int avio_feof(AVIOContext *s); -/** @warning Writes up to 4 KiB per call */ +/** + * Writes a formatted string to the context. + * @return number of bytes written, < 0 on error. + */ int avio_printf(AVIOContext *s, const char *fmt, ...) av_printf_format(2, 3); /** diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index c37b056b64..007fba6410 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -1215,14 +1215,19 @@ int avio_closep(AVIOContext **s) int avio_printf(AVIOContext *s, const char *fmt, ...) { va_list ap; - char buf[4096]; /* update doc entry in avio.h if changed */ - int ret; + AVBPrint bp; + av_bprint_init(&bp, 0, INT_MAX); va_start(ap, fmt); - ret = vsnprintf(buf, sizeof(buf), fmt, ap); + av_vbprintf(&bp, fmt, ap); va_end(ap); - avio_write(s, buf, strlen(buf)); - return ret; + if (!av_bprint_is_complete(&bp)) { + av_bprint_finalize(&bp, NULL); + return AVERROR(ENOMEM); + } + avio_write(s, bp.str, bp.len); + av_bprint_finalize(&bp, NULL); + return bp.len; } int avio_print_n_strings(AVIOContext *s, int nb_strings, ...) diff --git a/libavformat/version.h b/libavformat/version.h index feceaedc08..9814db8633 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -33,7 +33,7 @@ // Also please add any ticket numbers that you believe might be affected here #define LIBAVFORMAT_VERSION_MAJOR 58 #define LIBAVFORMAT_VERSION_MINOR 31 -#define LIBAVFORMAT_VERSION_MICRO 100 +#define LIBAVFORMAT_VERSION_MICRO 101 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ LIBAVFORMAT_VERSION_MINOR, \
We do this by switching to AVBPrint. Signed-off-by: Marton Balint <cus@passwd.hu> --- doc/APIchanges | 3 +++ libavformat/avio.h | 5 ++++- libavformat/aviobuf.c | 15 ++++++++++----- libavformat/version.h | 2 +- 4 files changed, 18 insertions(+), 7 deletions(-)