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

Message ID 1406851002-8054-2-git-send-email-andrew@clovar.com
State New
Headers show

Commit Message

Andrew Stone July 31, 2014, 11:56 p.m.
The only flag, for now, is AVFMT_EVENT_FLAG_METADATA_UPDATED, which is updated 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 | 9 +++++++++
 libavformat/utils.c    | 2 ++
 libavformat/version.h  | 2 +-
 4 files changed, 16 insertions(+), 1 deletion(-)

Comments

wm4 Aug. 1, 2014, 11:14 p.m. | #1
On Thu, 31 Jul 2014 19:56:33 -0400
Andrew Stone <andrew@clovar.com> wrote:

> The only flag, for now, is AVFMT_EVENT_FLAG_METADATA_UPDATED, which is updated 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 | 9 +++++++++
>  libavformat/utils.c    | 2 ++
>  libavformat/version.h  | 2 +-
>  4 files changed, 16 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..356677f 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1177,6 +1177,15 @@ typedef struct AVFormatContext {
>       * @see AVCodecContext.strict_std_compliance
>       */
>      int strict_std_compliance;
> +
> +    /**
> +     * Flags for the user to detect events happening on the stream. These
> +     * are cleared and set anew after each call to av_read_frame().
> +     * A combination of AVFMT_EVENT_FLAG_*.
> +     */
> +    int event_flags;
> +#define AVFMT_EVENT_FLAG_METADATA_UPDATED 0x0001 ///< The call resulted in updated metadata.
> +

There are many "metadata" fields in libavformat:
- AVFormatContext.metadata
- AVStream.metadata
- AVProgram.metadata
- AVChapter.metadata

Keep in mind that the library user has to check manually what exactly
updated. The doxygen for the event flag thus should be very clear on
what fields might have been changed.

It seems for our purposes, only the AVFormatContext and AVStream fields
can be updated, so that should be documented.

Also, there's a doxygen section about metadata further up in
avformat.h. Maybe you should add some words about metadata update there.

In general, I think it's an OK API, although I'm slightly disgruntled
about how this API is different from FFmpeg's.

>      /*****************************************************************
>       * 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/utils.c b/libavformat/utils.c
> index 698fcfe..6d191f1 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -982,6 +982,8 @@ int av_read_frame(AVFormatContext *s, AVPacket *pkt)
>      const int genpts = s->flags & AVFMT_FLAG_GENPTS;
>      int eof = 0;
>  
> +    s->event_flags = 0;
> +
>      if (!genpts)
>          return s->packet_buffer
>                 ? read_from_packet_buffer(&s->packet_buffer,
> 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, \
Andrew Stone Aug. 2, 2014, 4:27 a.m. | #2
On Fri, Aug 1, 2014 at 7:14 PM, wm4 <nfxjfg@googlemail.com> wrote:
> On Thu, 31 Jul 2014 19:56:33 -0400
> Andrew Stone <andrew@clovar.com> wrote:
>
>> The only flag, for now, is AVFMT_EVENT_FLAG_METADATA_UPDATED, which is updated 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 | 9 +++++++++
>>  libavformat/utils.c    | 2 ++
>>  libavformat/version.h  | 2 +-
>>  4 files changed, 16 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..356677f 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -1177,6 +1177,15 @@ typedef struct AVFormatContext {
>>       * @see AVCodecContext.strict_std_compliance
>>       */
>>      int strict_std_compliance;
>> +
>> +    /**
>> +     * Flags for the user to detect events happening on the stream. These
>> +     * are cleared and set anew after each call to av_read_frame().
>> +     * A combination of AVFMT_EVENT_FLAG_*.
>> +     */
>> +    int event_flags;
>> +#define AVFMT_EVENT_FLAG_METADATA_UPDATED 0x0001 ///< The call resulted in updated metadata.
>> +
>
> There are many "metadata" fields in libavformat:
> - AVFormatContext.metadata
> - AVStream.metadata
> - AVProgram.metadata
> - AVChapter.metadata
>
> Keep in mind that the library user has to check manually what exactly
> updated. The doxygen for the event flag thus should be very clear on
> what fields might have been changed.
>
> It seems for our purposes, only the AVFormatContext and AVStream fields
> can be updated, so that should be documented.

Actually, it might be even better to have a few flags to indicate
which set of metadata changed.

>
> Also, there's a doxygen section about metadata further up in
> avformat.h. Maybe you should add some words about metadata update there.

Will do.

>
> In general, I think it's an OK API, although I'm slightly disgruntled
> about how this API is different from FFmpeg's.

FFmpeg's includes the entire dictionary of values whenever there is a
change, without specifying where the change is. Using multiple flags
has the same net effect with much less code, and, IMHO, is easier to
work with.

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..356677f 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1177,6 +1177,15 @@  typedef struct AVFormatContext {
      * @see AVCodecContext.strict_std_compliance
      */
     int strict_std_compliance;
+
+    /**
+     * Flags for the user to detect events happening on the stream. These
+     * are cleared and set anew after each call to av_read_frame().
+     * A combination of AVFMT_EVENT_FLAG_*.
+     */
+    int event_flags;
+#define AVFMT_EVENT_FLAG_METADATA_UPDATED 0x0001 ///< The call resulted in updated metadata.
+
     /*****************************************************************
      * 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/utils.c b/libavformat/utils.c
index 698fcfe..6d191f1 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -982,6 +982,8 @@  int av_read_frame(AVFormatContext *s, AVPacket *pkt)
     const int genpts = s->flags & AVFMT_FLAG_GENPTS;
     int eof = 0;
 
+    s->event_flags = 0;
+
     if (!genpts)
         return s->packet_buffer
                ? read_from_packet_buffer(&s->packet_buffer,
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, \