[01/10] Add AVFormatContext.event_flags for signaling to the user when events have happened in the stream.

Message ID 1407257927-21952-1-git-send-email-andrew@clovar.com
State New
Headers show

Commit Message

Andrew Stone Aug. 5, 2014, 4:58 p.m.
The only flags, for now, indicate if metadata was updated and are set after each call to
av_read_frame(). This comes with the caveat that, on stream start, it might not be set properly
as packets might be buffered in AVFormatContext.packet_buffer before being given to the user
in av_read_frame().
---
 doc/APIchanges         |  4 ++++
 libavformat/avformat.h | 14 ++++++++++++++
 libavformat/version.h  |  2 +-
 3 files changed, 19 insertions(+), 1 deletion(-)

Comments

Anton Khirnov Aug. 6, 2014, 6:18 a.m. | #1
On Tue,  5 Aug 2014 12:58:47 -0400, Andrew Stone <andrew@clovar.com> wrote:
> The only flags, for now, indicate if metadata was updated and are set after each call to
> av_read_frame(). This comes with the caveat that, on stream start, it might not be set properly
> as packets might be buffered in AVFormatContext.packet_buffer before being given to the user
> in av_read_frame().
> ---
>  doc/APIchanges         |  4 ++++
>  libavformat/avformat.h | 14 ++++++++++++++
>  libavformat/version.h  |  2 +-
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 261993b..a677e42 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,10 @@ libavutil:     2013-12-xx
>  
>  API changes, most recent first:
>  
> +2014-xx-xx - xxxxxxx - lavf 55.21.0 - avformat.h
> +  Add AVFormatContext.event_flags for signaling to the user when events
> +  have happened in the stream.
> +
>  2014-07-xx - xxxxxxx - lavu 53.19.0 - avstring.h
>    Make name matching function from lavf public as av_match_name().
>  
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index ab2081a..6b25d6a 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -296,6 +296,10 @@ struct AVFormatContext;
>   *    -  sorting  -- a modified version of a tag that should be used for
>   *       sorting will have '-sort' appended. E.g. artist="The Beatles",
>   *       artist-sort="Beatles, The".
> + * - Some protocols and demuxers support metadata updates. After a successful
> + *   call to av_read_packet(), AVFormatContext.event_flags will be updated to
> + *   indicate if metadata was updated. Currently, only file (AVFormatContext)
> + *   and stream (AVStream) metadata change.
>   *
>   * -  Demuxers attempt to export metadata in a generic format, however tags
>   *    with no generic equivalents are left as they are stored in the container.
> @@ -1177,6 +1181,16 @@ typedef struct AVFormatContext {
>       * @see AVCodecContext.strict_std_compliance
>       */
>      int strict_std_compliance;
> +
> +    /**
> +     * Flags for the user to detect events happening on the stream. Flags must
> +     * be cleared by the user once the event has been handled.
> +     * A combination of AVFMT_EVENT_FLAG_*.
> +     */
> +    int event_flags;
> +#define AVFMT_EVENT_FLAG_METADATA_UPDATED        0x0001 ///< The call resulted in updated metadata for the whole file.
> +#define AVFMT_EVENT_FLAG_STREAM_METADATA_UPDATED 0x0002 ///< The call resulted in updated metadata for a stream.

My original suggestion in the last thread was to have a separate set of flags
for each stream (+global). That way you can see which stream exactly got
updated.
Or is there some specific reason to put all the flags here?
Andrew Stone Aug. 6, 2014, 12:58 p.m. | #2
On Wed, Aug 6, 2014 at 2:18 AM, Anton Khirnov <anton@khirnov.net> wrote:
>
> On Tue,  5 Aug 2014 12:58:47 -0400, Andrew Stone <andrew@clovar.com> wrote:
>> The only flags, for now, indicate if metadata was updated and are set after each call to
>> av_read_frame(). This comes with the caveat that, on stream start, it might not be set properly
>> as packets might be buffered in AVFormatContext.packet_buffer before being given to the user
>> in av_read_frame().
>> ---
>>  doc/APIchanges         |  4 ++++
>>  libavformat/avformat.h | 14 ++++++++++++++
>>  libavformat/version.h  |  2 +-
>>  3 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index 261993b..a677e42 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -13,6 +13,10 @@ libavutil:     2013-12-xx
>>
>>  API changes, most recent first:
>>
>> +2014-xx-xx - xxxxxxx - lavf 55.21.0 - avformat.h
>> +  Add AVFormatContext.event_flags for signaling to the user when events
>> +  have happened in the stream.
>> +
>>  2014-07-xx - xxxxxxx - lavu 53.19.0 - avstring.h
>>    Make name matching function from lavf public as av_match_name().
>>
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index ab2081a..6b25d6a 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -296,6 +296,10 @@ struct AVFormatContext;
>>   *    -  sorting  -- a modified version of a tag that should be used for
>>   *       sorting will have '-sort' appended. E.g. artist="The Beatles",
>>   *       artist-sort="Beatles, The".
>> + * - Some protocols and demuxers support metadata updates. After a successful
>> + *   call to av_read_packet(), AVFormatContext.event_flags will be updated to
>> + *   indicate if metadata was updated. Currently, only file (AVFormatContext)
>> + *   and stream (AVStream) metadata change.
>>   *
>>   * -  Demuxers attempt to export metadata in a generic format, however tags
>>   *    with no generic equivalents are left as they are stored in the container.
>> @@ -1177,6 +1181,16 @@ typedef struct AVFormatContext {
>>       * @see AVCodecContext.strict_std_compliance
>>       */
>>      int strict_std_compliance;
>> +
>> +    /**
>> +     * Flags for the user to detect events happening on the stream. Flags must
>> +     * be cleared by the user once the event has been handled.
>> +     * A combination of AVFMT_EVENT_FLAG_*.
>> +     */
>> +    int event_flags;
>> +#define AVFMT_EVENT_FLAG_METADATA_UPDATED        0x0001 ///< The call resulted in updated metadata for the whole file.
>> +#define AVFMT_EVENT_FLAG_STREAM_METADATA_UPDATED 0x0002 ///< The call resulted in updated metadata for a stream.
>
> My original suggestion in the last thread was to have a separate set of flags
> for each stream (+global). That way you can see which stream exactly got
> updated.
> Or is there some specific reason to put all the flags here?

I just found the simplicity of everything in one spot to be easier to work with.
wm4 Aug. 6, 2014, 4:46 p.m. | #3
On Wed, 06 Aug 2014 08:18:22 +0200
Anton Khirnov <anton@khirnov.net> wrote:

> 
> On Tue,  5 Aug 2014 12:58:47 -0400, Andrew Stone <andrew@clovar.com> wrote:
> > The only flags, for now, indicate if metadata was updated and are set after each call to
> > av_read_frame(). This comes with the caveat that, on stream start, it might not be set properly
> > as packets might be buffered in AVFormatContext.packet_buffer before being given to the user
> > in av_read_frame().
> > ---
> >  doc/APIchanges         |  4 ++++
> >  libavformat/avformat.h | 14 ++++++++++++++
> >  libavformat/version.h  |  2 +-
> >  3 files changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 261993b..a677e42 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -13,6 +13,10 @@ libavutil:     2013-12-xx
> >  
> >  API changes, most recent first:
> >  
> > +2014-xx-xx - xxxxxxx - lavf 55.21.0 - avformat.h
> > +  Add AVFormatContext.event_flags for signaling to the user when events
> > +  have happened in the stream.
> > +
> >  2014-07-xx - xxxxxxx - lavu 53.19.0 - avstring.h
> >    Make name matching function from lavf public as av_match_name().
> >  
> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > index ab2081a..6b25d6a 100644
> > --- a/libavformat/avformat.h
> > +++ b/libavformat/avformat.h
> > @@ -296,6 +296,10 @@ struct AVFormatContext;
> >   *    -  sorting  -- a modified version of a tag that should be used for
> >   *       sorting will have '-sort' appended. E.g. artist="The Beatles",
> >   *       artist-sort="Beatles, The".
> > + * - Some protocols and demuxers support metadata updates. After a successful
> > + *   call to av_read_packet(), AVFormatContext.event_flags will be updated to
> > + *   indicate if metadata was updated. Currently, only file (AVFormatContext)
> > + *   and stream (AVStream) metadata change.
> >   *
> >   * -  Demuxers attempt to export metadata in a generic format, however tags
> >   *    with no generic equivalents are left as they are stored in the container.
> > @@ -1177,6 +1181,16 @@ typedef struct AVFormatContext {
> >       * @see AVCodecContext.strict_std_compliance
> >       */
> >      int strict_std_compliance;
> > +
> > +    /**
> > +     * Flags for the user to detect events happening on the stream. Flags must
> > +     * be cleared by the user once the event has been handled.
> > +     * A combination of AVFMT_EVENT_FLAG_*.
> > +     */
> > +    int event_flags;
> > +#define AVFMT_EVENT_FLAG_METADATA_UPDATED        0x0001 ///< The call resulted in updated metadata for the whole file.
> > +#define AVFMT_EVENT_FLAG_STREAM_METADATA_UPDATED 0x0002 ///< The call resulted in updated metadata for a stream.
> 
> My original suggestion in the last thread was to have a separate set of flags
> for each stream (+global). That way you can see which stream exactly got
> updated.
> Or is there some specific reason to put all the flags here?
> 

You have a point. IMO the event flags should allow the user to know
what exactly changed, without having to compare old and new metadata
manually (because that would be a pain). If the event flags aren't
per-AVStream, it's harder to tell which AVStream changed metadata.

An _additional_ change flag in the AVFormatContext itself could still
be useful, so you don't have to iterate over all streams every time
just to check flags. But then maybe it should be something like
AVFMT_EVENT_FLAG_STREAM_EVENTS_UPDATED?
Andrew Stone Aug. 6, 2014, 7:57 p.m. | #4
On Wed, Aug 6, 2014 at 12:46 PM, wm4 <nfxjfg@googlemail.com> wrote:
> On Wed, 06 Aug 2014 08:18:22 +0200
> Anton Khirnov <anton@khirnov.net> wrote:
>
>>
>> On Tue,  5 Aug 2014 12:58:47 -0400, Andrew Stone <andrew@clovar.com> wrote:
>> > The only flags, for now, indicate if metadata was updated and are set after each call to
>> > av_read_frame(). This comes with the caveat that, on stream start, it might not be set properly
>> > as packets might be buffered in AVFormatContext.packet_buffer before being given to the user
>> > in av_read_frame().
>> > ---
>> >  doc/APIchanges         |  4 ++++
>> >  libavformat/avformat.h | 14 ++++++++++++++
>> >  libavformat/version.h  |  2 +-
>> >  3 files changed, 19 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/doc/APIchanges b/doc/APIchanges
>> > index 261993b..a677e42 100644
>> > --- a/doc/APIchanges
>> > +++ b/doc/APIchanges
>> > @@ -13,6 +13,10 @@ libavutil:     2013-12-xx
>> >
>> >  API changes, most recent first:
>> >
>> > +2014-xx-xx - xxxxxxx - lavf 55.21.0 - avformat.h
>> > +  Add AVFormatContext.event_flags for signaling to the user when events
>> > +  have happened in the stream.
>> > +
>> >  2014-07-xx - xxxxxxx - lavu 53.19.0 - avstring.h
>> >    Make name matching function from lavf public as av_match_name().
>> >
>> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> > index ab2081a..6b25d6a 100644
>> > --- a/libavformat/avformat.h
>> > +++ b/libavformat/avformat.h
>> > @@ -296,6 +296,10 @@ struct AVFormatContext;
>> >   *    -  sorting  -- a modified version of a tag that should be used for
>> >   *       sorting will have '-sort' appended. E.g. artist="The Beatles",
>> >   *       artist-sort="Beatles, The".
>> > + * - Some protocols and demuxers support metadata updates. After a successful
>> > + *   call to av_read_packet(), AVFormatContext.event_flags will be updated to
>> > + *   indicate if metadata was updated. Currently, only file (AVFormatContext)
>> > + *   and stream (AVStream) metadata change.
>> >   *
>> >   * -  Demuxers attempt to export metadata in a generic format, however tags
>> >   *    with no generic equivalents are left as they are stored in the container.
>> > @@ -1177,6 +1181,16 @@ typedef struct AVFormatContext {
>> >       * @see AVCodecContext.strict_std_compliance
>> >       */
>> >      int strict_std_compliance;
>> > +
>> > +    /**
>> > +     * Flags for the user to detect events happening on the stream. Flags must
>> > +     * be cleared by the user once the event has been handled.
>> > +     * A combination of AVFMT_EVENT_FLAG_*.
>> > +     */
>> > +    int event_flags;
>> > +#define AVFMT_EVENT_FLAG_METADATA_UPDATED        0x0001 ///< The call resulted in updated metadata for the whole file.
>> > +#define AVFMT_EVENT_FLAG_STREAM_METADATA_UPDATED 0x0002 ///< The call resulted in updated metadata for a stream.
>>
>> My original suggestion in the last thread was to have a separate set of flags
>> for each stream (+global). That way you can see which stream exactly got
>> updated.
>> Or is there some specific reason to put all the flags here?
>>
>
> You have a point. IMO the event flags should allow the user to know
> what exactly changed, without having to compare old and new metadata
> manually (because that would be a pain). If the event flags aren't
> per-AVStream, it's harder to tell which AVStream changed metadata.
>
> An _additional_ change flag in the AVFormatContext itself could still
> be useful, so you don't have to iterate over all streams every time
> just to check flags. But then maybe it should be something like
> AVFMT_EVENT_FLAG_STREAM_EVENTS_UPDATED?

A flag to indicate that a flag has been set? That seems like a bit too
much. I would honestly rather iterate the streams.
Anton Khirnov Aug. 6, 2014, 8:38 p.m. | #5
On Wed, 6 Aug 2014 15:57:55 -0400, Andrew Stone <andrew@clovar.com> wrote:
> On Wed, Aug 6, 2014 at 12:46 PM, wm4 <nfxjfg@googlemail.com> wrote:
> > On Wed, 06 Aug 2014 08:18:22 +0200
> > Anton Khirnov <anton@khirnov.net> wrote:
> >
> >>
> >> On Tue,  5 Aug 2014 12:58:47 -0400, Andrew Stone <andrew@clovar.com> wrote:
> >> > The only flags, for now, indicate if metadata was updated and are set after each call to
> >> > av_read_frame(). This comes with the caveat that, on stream start, it might not be set properly
> >> > as packets might be buffered in AVFormatContext.packet_buffer before being given to the user
> >> > in av_read_frame().
> >> > ---
> >> >  doc/APIchanges         |  4 ++++
> >> >  libavformat/avformat.h | 14 ++++++++++++++
> >> >  libavformat/version.h  |  2 +-
> >> >  3 files changed, 19 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/doc/APIchanges b/doc/APIchanges
> >> > index 261993b..a677e42 100644
> >> > --- a/doc/APIchanges
> >> > +++ b/doc/APIchanges
> >> > @@ -13,6 +13,10 @@ libavutil:     2013-12-xx
> >> >
> >> >  API changes, most recent first:
> >> >
> >> > +2014-xx-xx - xxxxxxx - lavf 55.21.0 - avformat.h
> >> > +  Add AVFormatContext.event_flags for signaling to the user when events
> >> > +  have happened in the stream.
> >> > +
> >> >  2014-07-xx - xxxxxxx - lavu 53.19.0 - avstring.h
> >> >    Make name matching function from lavf public as av_match_name().
> >> >
> >> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >> > index ab2081a..6b25d6a 100644
> >> > --- a/libavformat/avformat.h
> >> > +++ b/libavformat/avformat.h
> >> > @@ -296,6 +296,10 @@ struct AVFormatContext;
> >> >   *    -  sorting  -- a modified version of a tag that should be used for
> >> >   *       sorting will have '-sort' appended. E.g. artist="The Beatles",
> >> >   *       artist-sort="Beatles, The".
> >> > + * - Some protocols and demuxers support metadata updates. After a successful
> >> > + *   call to av_read_packet(), AVFormatContext.event_flags will be updated to
> >> > + *   indicate if metadata was updated. Currently, only file (AVFormatContext)
> >> > + *   and stream (AVStream) metadata change.
> >> >   *
> >> >   * -  Demuxers attempt to export metadata in a generic format, however tags
> >> >   *    with no generic equivalents are left as they are stored in the container.
> >> > @@ -1177,6 +1181,16 @@ typedef struct AVFormatContext {
> >> >       * @see AVCodecContext.strict_std_compliance
> >> >       */
> >> >      int strict_std_compliance;
> >> > +
> >> > +    /**
> >> > +     * Flags for the user to detect events happening on the stream. Flags must
> >> > +     * be cleared by the user once the event has been handled.
> >> > +     * A combination of AVFMT_EVENT_FLAG_*.
> >> > +     */
> >> > +    int event_flags;
> >> > +#define AVFMT_EVENT_FLAG_METADATA_UPDATED        0x0001 ///< The call resulted in updated metadata for the whole file.
> >> > +#define AVFMT_EVENT_FLAG_STREAM_METADATA_UPDATED 0x0002 ///< The call resulted in updated metadata for a stream.
> >>
> >> My original suggestion in the last thread was to have a separate set of flags
> >> for each stream (+global). That way you can see which stream exactly got
> >> updated.
> >> Or is there some specific reason to put all the flags here?
> >>
> >
> > You have a point. IMO the event flags should allow the user to know
> > what exactly changed, without having to compare old and new metadata
> > manually (because that would be a pain). If the event flags aren't
> > per-AVStream, it's harder to tell which AVStream changed metadata.
> >
> > An _additional_ change flag in the AVFormatContext itself could still
> > be useful, so you don't have to iterate over all streams every time
> > just to check flags. But then maybe it should be something like
> > AVFMT_EVENT_FLAG_STREAM_EVENTS_UPDATED?
> 
> A flag to indicate that a flag has been set? That seems like a bit too
> much. I would honestly rather iterate the streams.

+1
meta-meta-flags are really rather overkill.

As for a single event flag vs per-stream event flags, i slightly prefer the
latter, but can live with either. So the patch is fine with me as is.
Andrew Stone Aug. 8, 2014, 12:24 a.m. | #6
>> A flag to indicate that a flag has been set? That seems like a bit too
>> much. I would honestly rather iterate the streams.
>
> +1
> meta-meta-flags are really rather overkill.
>
> As for a single event flag vs per-stream event flags, i slightly prefer the
> latter, but can live with either. So the patch is fine with me as is.

I prefer them all in one place. wm4, how strongly do you feel about
flag-per-stream?
wm4 Aug. 8, 2014, 10:25 a.m. | #7
On Thu, 7 Aug 2014 20:24:20 -0400
Andrew Stone <andrew@clovar.com> wrote:

> >> A flag to indicate that a flag has been set? That seems like a bit too
> >> much. I would honestly rather iterate the streams.
> >
> > +1
> > meta-meta-flags are really rather overkill.
> >
> > As for a single event flag vs per-stream event flags, i slightly prefer the
> > latter, but can live with either. So the patch is fine with me as is.
> 
> I prefer them all in one place. wm4, how strongly do you feel about
> flag-per-stream?

Unfortunately, I'd still prefer per-stream flags. They are IMHO
somewhat important to conveniently tell _if_ the metadata of a specific
stream changed. (If that were not an issue, we wouldn't need these
flags at all, would we?)

So I guess, give the streams separate flags, and don't add a meta-meta
flag, and require the user to iterate over all streams on each packet
read.

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index 261993b..a677e42 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,10 @@  libavutil:     2013-12-xx
 
 API changes, most recent first:
 
+2014-xx-xx - xxxxxxx - lavf 55.21.0 - avformat.h
+  Add AVFormatContext.event_flags for signaling to the user when events
+  have happened in the stream.
+
 2014-07-xx - xxxxxxx - lavu 53.19.0 - avstring.h
   Make name matching function from lavf public as av_match_name().
 
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index ab2081a..6b25d6a 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -296,6 +296,10 @@  struct AVFormatContext;
  *    -  sorting  -- a modified version of a tag that should be used for
  *       sorting will have '-sort' appended. E.g. artist="The Beatles",
  *       artist-sort="Beatles, The".
+ * - Some protocols and demuxers support metadata updates. After a successful
+ *   call to av_read_packet(), AVFormatContext.event_flags will be updated to
+ *   indicate if metadata was updated. Currently, only file (AVFormatContext)
+ *   and stream (AVStream) metadata change.
  *
  * -  Demuxers attempt to export metadata in a generic format, however tags
  *    with no generic equivalents are left as they are stored in the container.
@@ -1177,6 +1181,16 @@  typedef struct AVFormatContext {
      * @see AVCodecContext.strict_std_compliance
      */
     int strict_std_compliance;
+
+    /**
+     * Flags for the user to detect events happening on the stream. Flags must
+     * be cleared by the user once the event has been handled.
+     * A combination of AVFMT_EVENT_FLAG_*.
+     */
+    int event_flags;
+#define AVFMT_EVENT_FLAG_METADATA_UPDATED        0x0001 ///< The call resulted in updated metadata for the whole file.
+#define AVFMT_EVENT_FLAG_STREAM_METADATA_UPDATED 0x0002 ///< The call resulted in updated metadata for a stream.
+
     /*****************************************************************
      * All fields below this line are not part of the public API. They
      * may not be used outside of libavformat and can be changed and
diff --git a/libavformat/version.h b/libavformat/version.h
index 6e9ce67..fbf5a0c 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -30,7 +30,7 @@ 
 #include "libavutil/version.h"
 
 #define LIBAVFORMAT_VERSION_MAJOR 55
-#define LIBAVFORMAT_VERSION_MINOR 20
+#define LIBAVFORMAT_VERSION_MINOR 21
 #define LIBAVFORMAT_VERSION_MICRO  0
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \