Re: [libav-devel] [PATCH 1/2] avio: add a destructor for AVIOContext

[wm4]

On Fri, 13 Jan 2017 13:29:55 +0100
Anton Khirnov  wrote:

> Quoting wm4 (2017-01-13 12:09:23)
> > On Fri, 13 Jan 2017 12:04:36 +0100
> > Anton Khirnov  wrote:
> >   
> > > Before this commit, AVIOContext is to be freed with a plain av_free(),
> > > which prevents us from adding any deeper structure to it.
> > > ---
> > >  doc/APIchanges|  3 +++
> > >  libavformat/avio.h|  8 
> > >  libavformat/aviobuf.c | 17 ++---
> > >  libavformat/version.h |  4 ++--
> > >  4 files changed, 27 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/doc/APIchanges b/doc/APIchanges
> > > index c8c2a21..1cab9f4 100644
> > > --- a/doc/APIchanges
> > > +++ b/doc/APIchanges
> > > @@ -13,6 +13,9 @@ libavutil: 2015-08-28
> > >  
> > >  API changes, most recent first:
> > >  
> > > +2016-xx-xx - xxx - lavf 57.11.0 - avio.h
> > > +  Add avio_context_free(). From now on it must be used for freeing 
> > > AVIOContext.
> > > +  
> > 
> > As an API user I'm saying "lol no". This implicitly breaks every API
> > user without warning, and you'd only know once it actually breaks (and
> > you've spent a few hours of debugging). Unless you read APIchanges,
> > which not many are likely to do.  
> 
> I'm not breaking anything just yet, and not planning to in the immediate
> future. The (preliminary) transition plan I have in mind involved
> deprecating avio_alloc_context() and replacing it with
> avio_context_alloc(), with a properly big-endian-namespaced name and
> most likely a different signature. That will give you all the
> deprecation warnings you want. The exact shape of that new API is to be
> determined, but it seemed to me that it's best to add the destructor as
> soon as possible, since the API for that is very straightforward.

Uh, ok, I guess that works.

> (also, if you're a libav API user and don't read APIchanges at least
> once per major bump then you deserve everything you get =p)
> 

Even if I do I can miss something like that easily.

In general I don't really think you can expect users to read that,
unless you present it on a golden tablet on every major release.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/2] avio: add a destructor for AVIOContext

[Luca Barbato]

On 13/01/2017 12:04, Anton Khirnov wrote:
> Before this commit, AVIOContext is to be freed with a plain av_free(),
> which prevents us from adding any deeper structure to it.
> ---
>  doc/APIchanges|  3 +++
>  libavformat/avio.h|  8 
>  libavformat/aviobuf.c | 17 ++---
>  libavformat/version.h |  4 ++--
>  4 files changed, 27 insertions(+), 5 deletions(-)
>

Ok.

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/2] avio: add a destructor for AVIOContext

[Anton Khirnov]

Quoting Hendrik Leppkes (2017-01-13 12:16:16)
> On Fri, Jan 13, 2017 at 10:04 PM, Anton Khirnov  wrote:
> > Before this commit, AVIOContext is to be freed with a plain av_free(),
> > which prevents us from adding any deeper structure to it.
> 
> avio_close(p) already closes and destructs a AVIOContext, what exactly
> is this doing that it can't do?
> 

avio_close() only works on AVIOContexts created with avio_open() (i.e.
using our internal protocol layer), not custom ones.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/2] avio: add a destructor for AVIOContext

[Anton Khirnov]

Quoting wm4 (2017-01-13 12:09:23)
> On Fri, 13 Jan 2017 12:04:36 +0100
> Anton Khirnov  wrote:
> 
> > Before this commit, AVIOContext is to be freed with a plain av_free(),
> > which prevents us from adding any deeper structure to it.
> > ---
> >  doc/APIchanges|  3 +++
> >  libavformat/avio.h|  8 
> >  libavformat/aviobuf.c | 17 ++---
> >  libavformat/version.h |  4 ++--
> >  4 files changed, 27 insertions(+), 5 deletions(-)
> > 
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index c8c2a21..1cab9f4 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -13,6 +13,9 @@ libavutil: 2015-08-28
> >  
> >  API changes, most recent first:
> >  
> > +2016-xx-xx - xxx - lavf 57.11.0 - avio.h
> > +  Add avio_context_free(). From now on it must be used for freeing 
> > AVIOContext.
> > +
> 
> As an API user I'm saying "lol no". This implicitly breaks every API
> user without warning, and you'd only know once it actually breaks (and
> you've spent a few hours of debugging). Unless you read APIchanges,
> which not many are likely to do.

I'm not breaking anything just yet, and not planning to in the immediate
future. The (preliminary) transition plan I have in mind involved
deprecating avio_alloc_context() and replacing it with
avio_context_alloc(), with a properly big-endian-namespaced name and
most likely a different signature. That will give you all the
deprecation warnings you want. The exact shape of that new API is to be
determined, but it seemed to me that it's best to add the destructor as
soon as possible, since the API for that is very straightforward.

(also, if you're a libav API user and don't read APIchanges at least
once per major bump then you deserve everything you get =p)

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/2] avio: add a destructor for AVIOContext

[Hendrik Leppkes]

On Fri, Jan 13, 2017 at 10:04 PM, Anton Khirnov  wrote:
> Before this commit, AVIOContext is to be freed with a plain av_free(),
> which prevents us from adding any deeper structure to it.

avio_close(p) already closes and destructs a AVIOContext, what exactly
is this doing that it can't do?

- Hendrik
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/2] avio: add a destructor for AVIOContext

[wm4]

On Fri, 13 Jan 2017 12:04:36 +0100
Anton Khirnov  wrote:

> Before this commit, AVIOContext is to be freed with a plain av_free(),
> which prevents us from adding any deeper structure to it.
> ---
>  doc/APIchanges|  3 +++
>  libavformat/avio.h|  8 
>  libavformat/aviobuf.c | 17 ++---
>  libavformat/version.h |  4 ++--
>  4 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index c8c2a21..1cab9f4 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,9 @@ libavutil: 2015-08-28
>  
>  API changes, most recent first:
>  
> +2016-xx-xx - xxx - lavf 57.11.0 - avio.h
> +  Add avio_context_free(). From now on it must be used for freeing 
> AVIOContext.
> +

As an API user I'm saying "lol no". This implicitly breaks every API
user without warning, and you'd only know once it actually breaks (and
you've spent a few hours of debugging). Unless you read APIchanges,
which not many are likely to do.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH 1/2] avio: add a destructor for AVIOContext

[Anton Khirnov]

Before this commit, AVIOContext is to be freed with a plain av_free(),
which prevents us from adding any deeper structure to it.
---
 doc/APIchanges|  3 +++
 libavformat/avio.h|  8 
 libavformat/aviobuf.c | 17 ++---
 libavformat/version.h |  4 ++--
 4 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index c8c2a21..1cab9f4 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,9 @@ libavutil: 2015-08-28
 
 API changes, most recent first:
 
+2016-xx-xx - xxx - lavf 57.11.0 - avio.h
+  Add avio_context_free(). From now on it must be used for freeing AVIOContext.
+
 2016-xx-xx - xxx - lavc 57.31.0 - avcodec.h
   Add AVCodecContext.apply_cropping to control whether cropping
   is handled by libavcodec or the caller.
diff --git a/libavformat/avio.h b/libavformat/avio.h
index 7bf7985..e65135e 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -219,6 +219,14 @@ AVIOContext *avio_alloc_context(
   int (*write_packet)(void *opaque, uint8_t *buf, int 
buf_size),
   int64_t (*seek)(void *opaque, int64_t offset, int whence));
 
+/**
+ * Free the supplied IO context and everything associated with it.
+ *
+ * @param s Double pointer to the IO context. This function will write NULL
+ * into s.
+ */
+void avio_context_free(AVIOContext **s);
+
 void avio_w8(AVIOContext *s, int b);
 void avio_write(AVIOContext *s, const unsigned char *buf, int size);
 void avio_wl64(AVIOContext *s, uint64_t val);
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 6d83a96..31476d3 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -165,6 +165,11 @@ AVIOContext *avio_alloc_context(
 return s;
 }
 
+void avio_context_free(AVIOContext **ps)
+{
+av_freep(ps);
+}
+
 static void flush_buffer(AVIOContext *s)
 {
 if (s->buf_ptr > s->buffer) {
@@ -1007,7 +1012,9 @@ int avio_close(AVIOContext *s)
 av_freep(>protocols);
 av_freep(>opaque);
 av_freep(>buffer);
-av_free(s);
+
+avio_context_free();
+
 return ffurl_close(h);
 }
 
@@ -1186,7 +1193,9 @@ int avio_close_dyn_buf(AVIOContext *s, uint8_t **pbuffer)
 *pbuffer = d->buffer;
 size = d->size;
 av_free(d);
-av_free(s);
+
+avio_context_free();
+
 return size - padding;
 }
 
@@ -1229,6 +1238,8 @@ int ffio_close_null_buf(AVIOContext *s)
 
 size = d->size;
 av_free(d);
-av_free(s);
+
+avio_context_free();
+
 return size;
 }
diff --git a/libavformat/version.h b/libavformat/version.h
index 3fa2c44..92f3407 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -30,8 +30,8 @@
 #include "libavutil/version.h"
 
 #define LIBAVFORMAT_VERSION_MAJOR 57
-#define LIBAVFORMAT_VERSION_MINOR 10
-#define LIBAVFORMAT_VERSION_MICRO  2
+#define LIBAVFORMAT_VERSION_MINOR 11
+#define LIBAVFORMAT_VERSION_MICRO  0
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
LIBAVFORMAT_VERSION_MINOR, \
-- 
2.0.0

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel