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

diff mbox

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/utils.c    |  2 ++
 libavformat/version.h  |  2 +-
 4 files changed, 21 insertions(+), 1 deletion(-)

--
2.0.1

On Tue,  5 Aug 2014 11:13:04 -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/utils.c    |  2 ++
>  libavformat/version.h  |  2 +-
>  4 files changed, 21 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..f69765e 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. 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 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/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;
> +

Why? IMO it would be better to have the user reset them. This reduces
the chance that events are accidentally missed.

>      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, \
> --
> 2.0.1
> 
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel

On Tue, Aug 5, 2014 at 12:05 PM, wm4 <nfxjfg@googlemail.com> wrote:
> On Tue,  5 Aug 2014 11:13:04 -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/utils.c    |  2 ++
>>  libavformat/version.h  |  2 +-
>>  4 files changed, 21 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..f69765e 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. 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 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/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;
>> +
>
> Why? IMO it would be better to have the user reset them. This reduces
> the chance that events are accidentally missed.

To me, as a user, it seems more awkward that I would have to reset a
flag in a struct from a library than the library resetting it after a
call. Plus, since the user should be checking the flags after every
call to av_read_packet, it's unlikely they'll miss the event.

On Tue, 5 Aug 2014 12:29:09 -0400
Andrew Stone <andrew@clovar.com> wrote:

> On Tue, Aug 5, 2014 at 12:05 PM, wm4 <nfxjfg@googlemail.com> wrote:
> > On Tue,  5 Aug 2014 11:13:04 -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/utils.c    |  2 ++
> >>  libavformat/version.h  |  2 +-
> >>  4 files changed, 21 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..f69765e 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. 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 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/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;
> >> +
> >
> > Why? IMO it would be better to have the user reset them. This reduces
> > the chance that events are accidentally missed.
> 
> To me, as a user, it seems more awkward that I would have to reset a
> flag in a struct from a library than the library resetting it after a
> call. Plus, since the user should be checking the flags after every
> call to av_read_packet, it's unlikely they'll miss the event.

What if other calls are going to set event flags? Seeking perhaps? To
me as user, it actually seems natural that I have to reset the flags,
rather than wondering what exactly will reset them. If I have to reset
the flags, I can check it in one place, rather than having to litter my
code with calls to check for events after every lavf API call.

Besides, event flags might be used for things that are not even related
to reading packets in the future.

On Tue, Aug 5, 2014 at 12:38 PM, wm4 <nfxjfg@googlemail.com> wrote:
> On Tue, 5 Aug 2014 12:29:09 -0400
> Andrew Stone <andrew@clovar.com> wrote:
>
>> On Tue, Aug 5, 2014 at 12:05 PM, wm4 <nfxjfg@googlemail.com> wrote:
>> > On Tue,  5 Aug 2014 11:13:04 -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/utils.c    |  2 ++
>> >>  libavformat/version.h  |  2 +-
>> >>  4 files changed, 21 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..f69765e 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. 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 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/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;
>> >> +
>> >
>> > Why? IMO it would be better to have the user reset them. This reduces
>> > the chance that events are accidentally missed.
>>
>> To me, as a user, it seems more awkward that I would have to reset a
>> flag in a struct from a library than the library resetting it after a
>> call. Plus, since the user should be checking the flags after every
>> call to av_read_packet, it's unlikely they'll miss the event.
>
> What if other calls are going to set event flags? Seeking perhaps? To
> me as user, it actually seems natural that I have to reset the flags,
> rather than wondering what exactly will reset them. If I have to reset
> the flags, I can check it in one place, rather than having to litter my
> code with calls to check for events after every lavf API call.
>
> Besides, event flags might be used for things that are not even related
> to reading packets in the future.

Fair point. I hadn't considered that. I'll go ahead and change it.