| Message ID | 20211130004950.30697-4-cus@passwd.hu |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avformat/file: use proper return value in file_close | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
Marton Balint: > Otherwise there is no way to detect any error during avio_close(). > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > doc/APIchanges | 3 +++ > libavformat/avformat.h | 6 +++++- > libavformat/internal.h | 6 +++++- > libavformat/options.c | 4 ++-- > libavformat/utils.c | 6 ++++-- > libavformat/version.h | 4 ++-- > 6 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index bc9f4e38da..090263aedf 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -14,6 +14,9 @@ libavutil: 2021-04-27 > > API changes, most recent first: > > +2021-12-xx - xxxxxxxxxx - lavf 59.10.100 - avformat.h > + AVFormatContext io_open now returns an int. > + > 2021-11-22 - xxxxxxxxxx - lavu 57.9.100 - pixfmt.h > Add AV_PIX_FMT_P210, AV_PIX_FMT_P410, AV_PIX_FMT_P216, and AV_PIX_FMT_P416. > > diff --git a/libavformat/avformat.h b/libavformat/avformat.h > index 75699f3a32..eec1f6b20c 100644 > --- a/libavformat/avformat.h > +++ b/libavformat/avformat.h > @@ -1756,8 +1756,12 @@ typedef struct AVFormatContext { > > /** > * A callback for closing the streams opened with AVFormatContext.io_open(). > + * > + * @param s the format context > + * @param pb IO context to be closed and freed > + * @return 0 on success, a negative AVERROR code on failure > */ > - void (*io_close)(struct AVFormatContext *s, AVIOContext *pb); > + int (*io_close)(struct AVFormatContext *s, AVIOContext *pb); > > /** > * ',' separated list of disallowed protocols. > diff --git a/libavformat/internal.h b/libavformat/internal.h > index 20e93d9267..4f28885f3c 100644 > --- a/libavformat/internal.h > +++ b/libavformat/internal.h > @@ -905,8 +905,12 @@ int ff_format_output_open(AVFormatContext *s, const char *url, AVDictionary **op > /* > * A wrapper around AVFormatContext.io_close that should be used > * instead of calling the pointer directly. > + * > + * @param s AVFormatContext > + * @param *pb the AVIOContext to be closed and freed. Can be NULL. > + * @return >=0 on success, negative AVERROR in case of failure > */ > -void ff_format_io_close(AVFormatContext *s, AVIOContext **pb); > +int ff_format_io_close(AVFormatContext *s, AVIOContext **pb); > > /** > * Utility function to check if the file uses http or https protocol > diff --git a/libavformat/options.c b/libavformat/options.c > index 72c9bdcefe..15f0c9e90c 100644 > --- a/libavformat/options.c > +++ b/libavformat/options.c > @@ -146,9 +146,9 @@ static int io_open_default(AVFormatContext *s, AVIOContext **pb, > return ffio_open_whitelist(pb, url, flags, &s->interrupt_callback, options, s->protocol_whitelist, s->protocol_blacklist); > } > > -static void io_close_default(AVFormatContext *s, AVIOContext *pb) > +static int io_close_default(AVFormatContext *s, AVIOContext *pb) > { > - avio_close(pb); > + return avio_close(pb); > } > > AVFormatContext *avformat_alloc_context(void) > diff --git a/libavformat/utils.c b/libavformat/utils.c > index 7840e8717c..db5ef46dc2 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -1829,11 +1829,13 @@ int ff_format_output_open(AVFormatContext *s, const char *url, AVDictionary **op > return 0; > } > > -void ff_format_io_close(AVFormatContext *s, AVIOContext **pb) > +int ff_format_io_close(AVFormatContext *s, AVIOContext **pb) > { > + int ret = 0; > if (*pb) > - s->io_close(s, *pb); > + ret = s->io_close(s, *pb); > *pb = NULL; > + return ret; > } > > int ff_is_http_proto(const char *filename) { > diff --git a/libavformat/version.h b/libavformat/version.h > index 0705ee4112..1623835a78 100644 > --- a/libavformat/version.h > +++ b/libavformat/version.h > @@ -32,8 +32,8 @@ > // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium) > // Also please add any ticket numbers that you believe might be affected here > #define LIBAVFORMAT_VERSION_MAJOR 59 > -#define LIBAVFORMAT_VERSION_MINOR 9 > -#define LIBAVFORMAT_VERSION_MICRO 102 > +#define LIBAVFORMAT_VERSION_MINOR 10 > +#define LIBAVFORMAT_VERSION_MICRO 100 > > #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ > LIBAVFORMAT_VERSION_MINOR, \ > This is an API break. - Andreas
On Tue, 30 Nov 2021, Andreas Rheinhardt wrote: > Marton Balint: >> Otherwise there is no way to detect any error during avio_close(). >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> doc/APIchanges | 3 +++ >> libavformat/avformat.h | 6 +++++- >> libavformat/internal.h | 6 +++++- >> libavformat/options.c | 4 ++-- >> libavformat/utils.c | 6 ++++-- >> libavformat/version.h | 4 ++-- >> 6 files changed, 21 insertions(+), 8 deletions(-) >> >> diff --git a/doc/APIchanges b/doc/APIchanges >> index bc9f4e38da..090263aedf 100644 >> --- a/doc/APIchanges >> +++ b/doc/APIchanges >> @@ -14,6 +14,9 @@ libavutil: 2021-04-27 >> >> API changes, most recent first: >> >> +2021-12-xx - xxxxxxxxxx - lavf 59.10.100 - avformat.h >> + AVFormatContext io_open now returns an int. >> + >> 2021-11-22 - xxxxxxxxxx - lavu 57.9.100 - pixfmt.h >> Add AV_PIX_FMT_P210, AV_PIX_FMT_P410, AV_PIX_FMT_P216, and AV_PIX_FMT_P416. >> >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h >> index 75699f3a32..eec1f6b20c 100644 >> --- a/libavformat/avformat.h >> +++ b/libavformat/avformat.h >> @@ -1756,8 +1756,12 @@ typedef struct AVFormatContext { >> >> /** >> * A callback for closing the streams opened with AVFormatContext.io_open(). >> + * >> + * @param s the format context >> + * @param pb IO context to be closed and freed >> + * @return 0 on success, a negative AVERROR code on failure >> */ >> - void (*io_close)(struct AVFormatContext *s, AVIOContext *pb); >> + int (*io_close)(struct AVFormatContext *s, AVIOContext *pb); >> >> /** >> * ',' separated list of disallowed protocols. > > This is an API break. To be honest I wanted to avoid the io_close2() dance. This change has limited impact and we still did not have a release since the bump. I can rework if somebody feels that introducing io_close2() is the better way. Please let me know. Thanks, Marton
Quoting Marton Balint (2021-11-30 10:07:21) > > > On Tue, 30 Nov 2021, Andreas Rheinhardt wrote: > > > Marton Balint: > >> Otherwise there is no way to detect any error during avio_close(). > >> > >> Signed-off-by: Marton Balint <cus@passwd.hu> > >> --- > >> doc/APIchanges | 3 +++ > >> libavformat/avformat.h | 6 +++++- > >> libavformat/internal.h | 6 +++++- > >> libavformat/options.c | 4 ++-- > >> libavformat/utils.c | 6 ++++-- > >> libavformat/version.h | 4 ++-- > >> 6 files changed, 21 insertions(+), 8 deletions(-) > >> > >> diff --git a/doc/APIchanges b/doc/APIchanges > >> index bc9f4e38da..090263aedf 100644 > >> --- a/doc/APIchanges > >> +++ b/doc/APIchanges > >> @@ -14,6 +14,9 @@ libavutil: 2021-04-27 > >> > >> API changes, most recent first: > >> > >> +2021-12-xx - xxxxxxxxxx - lavf 59.10.100 - avformat.h > >> + AVFormatContext io_open now returns an int. > >> + > >> 2021-11-22 - xxxxxxxxxx - lavu 57.9.100 - pixfmt.h > >> Add AV_PIX_FMT_P210, AV_PIX_FMT_P410, AV_PIX_FMT_P216, and AV_PIX_FMT_P416. > >> > >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h > >> index 75699f3a32..eec1f6b20c 100644 > >> --- a/libavformat/avformat.h > >> +++ b/libavformat/avformat.h > >> @@ -1756,8 +1756,12 @@ typedef struct AVFormatContext { > >> > >> /** > >> * A callback for closing the streams opened with AVFormatContext.io_open(). > >> + * > >> + * @param s the format context > >> + * @param pb IO context to be closed and freed > >> + * @return 0 on success, a negative AVERROR code on failure > >> */ > >> - void (*io_close)(struct AVFormatContext *s, AVIOContext *pb); > >> + int (*io_close)(struct AVFormatContext *s, AVIOContext *pb); > >> > >> /** > >> * ',' separated list of disallowed protocols. > > > > This is an API break. > > To be honest I wanted to avoid the io_close2() dance. This change has > limited impact and we still did not have a release since the bump. > > I can rework if somebody feels that introducing io_close2() is the better > way. Please let me know. I do. I also feel very uncomfortable with arguments along the lines of "breaking things outside of releases is ok". We've always considered git master to be stable (except for a _short_ period after a major bump).
On Sat, Dec 4, 2021 at 9:00 PM Marton Balint <cus@passwd.hu> wrote: > > Otherwise there is no way to detect any error during avio_close(). > Returning errors is nice and all, but whats the expected reaction to an error here, since we're already trying to close? Can't exactly close and go home (since thats what you just tried) like you would on an error to open. - Hendrik
On Sat, 4 Dec 2021, Hendrik Leppkes wrote: > On Sat, Dec 4, 2021 at 9:00 PM Marton Balint <cus@passwd.hu> wrote: >> >> Otherwise there is no way to detect any error during avio_close(). >> > > Returning errors is nice and all, but whats the expected reaction to > an error here, since we're already trying to close? Can't exactly > close and go home (since thats what you just tried) like you would on > an error to open. It is probably not very useful for the read case, it is paramount however for the write case. To be sure you successfully wrote your data you need to check the return status. Regards, Marton
diff --git a/doc/APIchanges b/doc/APIchanges index bc9f4e38da..090263aedf 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -14,6 +14,9 @@ libavutil: 2021-04-27 API changes, most recent first: +2021-12-xx - xxxxxxxxxx - lavf 59.10.100 - avformat.h + AVFormatContext io_open now returns an int. + 2021-11-22 - xxxxxxxxxx - lavu 57.9.100 - pixfmt.h Add AV_PIX_FMT_P210, AV_PIX_FMT_P410, AV_PIX_FMT_P216, and AV_PIX_FMT_P416. diff --git a/libavformat/avformat.h b/libavformat/avformat.h index 75699f3a32..eec1f6b20c 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1756,8 +1756,12 @@ typedef struct AVFormatContext { /** * A callback for closing the streams opened with AVFormatContext.io_open(). + * + * @param s the format context + * @param pb IO context to be closed and freed + * @return 0 on success, a negative AVERROR code on failure */ - void (*io_close)(struct AVFormatContext *s, AVIOContext *pb); + int (*io_close)(struct AVFormatContext *s, AVIOContext *pb); /** * ',' separated list of disallowed protocols. diff --git a/libavformat/internal.h b/libavformat/internal.h index 20e93d9267..4f28885f3c 100644 --- a/libavformat/internal.h +++ b/libavformat/internal.h @@ -905,8 +905,12 @@ int ff_format_output_open(AVFormatContext *s, const char *url, AVDictionary **op /* * A wrapper around AVFormatContext.io_close that should be used * instead of calling the pointer directly. + * + * @param s AVFormatContext + * @param *pb the AVIOContext to be closed and freed. Can be NULL. + * @return >=0 on success, negative AVERROR in case of failure */ -void ff_format_io_close(AVFormatContext *s, AVIOContext **pb); +int ff_format_io_close(AVFormatContext *s, AVIOContext **pb); /** * Utility function to check if the file uses http or https protocol diff --git a/libavformat/options.c b/libavformat/options.c index 72c9bdcefe..15f0c9e90c 100644 --- a/libavformat/options.c +++ b/libavformat/options.c @@ -146,9 +146,9 @@ static int io_open_default(AVFormatContext *s, AVIOContext **pb, return ffio_open_whitelist(pb, url, flags, &s->interrupt_callback, options, s->protocol_whitelist, s->protocol_blacklist); } -static void io_close_default(AVFormatContext *s, AVIOContext *pb) +static int io_close_default(AVFormatContext *s, AVIOContext *pb) { - avio_close(pb); + return avio_close(pb); } AVFormatContext *avformat_alloc_context(void) diff --git a/libavformat/utils.c b/libavformat/utils.c index 7840e8717c..db5ef46dc2 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -1829,11 +1829,13 @@ int ff_format_output_open(AVFormatContext *s, const char *url, AVDictionary **op return 0; } -void ff_format_io_close(AVFormatContext *s, AVIOContext **pb) +int ff_format_io_close(AVFormatContext *s, AVIOContext **pb) { + int ret = 0; if (*pb) - s->io_close(s, *pb); + ret = s->io_close(s, *pb); *pb = NULL; + return ret; } int ff_is_http_proto(const char *filename) { diff --git a/libavformat/version.h b/libavformat/version.h index 0705ee4112..1623835a78 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -32,8 +32,8 @@ // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium) // Also please add any ticket numbers that you believe might be affected here #define LIBAVFORMAT_VERSION_MAJOR 59 -#define LIBAVFORMAT_VERSION_MINOR 9 -#define LIBAVFORMAT_VERSION_MICRO 102 +#define LIBAVFORMAT_VERSION_MINOR 10 +#define LIBAVFORMAT_VERSION_MICRO 100 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ LIBAVFORMAT_VERSION_MINOR, \
Otherwise there is no way to detect any error during avio_close(). Signed-off-by: Marton Balint <cus@passwd.hu> --- doc/APIchanges | 3 +++ libavformat/avformat.h | 6 +++++- libavformat/internal.h | 6 +++++- libavformat/options.c | 4 ++-- libavformat/utils.c | 6 ++++-- libavformat/version.h | 4 ++-- 6 files changed, 21 insertions(+), 8 deletions(-)