diff mbox

[FFmpeg-devel] libavformat/hls: Support metadata updates from subdemuxers

Message ID 20180202205945.71872-1-rshaffer@tunein.com
State Accepted
Commit 651d5f963921bbd547373380e1581df9bbc83199
Headers show

Commit Message

rshaffer@tunein.com Feb. 2, 2018, 8:59 p.m. UTC
From: Richard Shaffer <rshaffer@tunein.com>

If a subdemuxer has the updated metadata event flag set, the metadata is copied
to the corresponding stream. The flag is cleared on the subdemuxer and the
appropriate event flag is set on the stream.
---
This is iteration #2.

In this version, we don't try to copy metadata from subdemuxer streams, only
from the AVFormatContext. The case where metadata is updated on a sub-demuxer
stream would have probably never happened.

The above change made the code that was previously in the
update_metadata_from_subdemuxer function a little simpler. That function was
also kind of ugly, because in one case it read and set flags, and in another it
didn't. It seemed simpler just to move the respective updates into read_header
and read_packet, respectively, so that's what I did.

-Richard

 libavformat/hls.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

wm4 Feb. 3, 2018, 10:35 a.m. UTC | #1
On Fri,  2 Feb 2018 12:59:45 -0800
rshaffer@tunein.com wrote:

> From: Richard Shaffer <rshaffer@tunein.com>
> 
> If a subdemuxer has the updated metadata event flag set, the metadata is copied
> to the corresponding stream. The flag is cleared on the subdemuxer and the
> appropriate event flag is set on the stream.
> ---
> This is iteration #2.

You can pass "-v2" as argument to git send-email to automatically add
"v2" to the subject.

> 
> In this version, we don't try to copy metadata from subdemuxer streams, only
> from the AVFormatContext. The case where metadata is updated on a sub-demuxer
> stream would have probably never happened.

Oh, I thought it was the other way around.

> The above change made the code that was previously in the
> update_metadata_from_subdemuxer function a little simpler. That function was
> also kind of ugly, because in one case it read and set flags, and in another it
> didn't. It seemed simpler just to move the respective updates into read_header
> and read_packet, respectively, so that's what I did.
> 
> -Richard
> 
>  libavformat/hls.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 9bd54c84cc..c578bf86e3 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -1062,6 +1062,7 @@ static void handle_id3(AVIOContext *pb, struct playlist *pls)
>              /* demuxer not yet opened, defer picture attachment */
>              pls->id3_deferred_extra = extra_meta;
>  
> +        ff_id3v2_parse_priv_dict(&metadata, &extra_meta);
>          av_dict_copy(&pls->ctx->metadata, metadata, 0);
>          pls->id3_initial = metadata;
>  
> @@ -1960,6 +1961,7 @@ static int hls_read_header(AVFormatContext *s)
>          if (pls->id3_deferred_extra && pls->ctx->nb_streams == 1) {
>              ff_id3v2_parse_apic(pls->ctx, &pls->id3_deferred_extra);
>              avformat_queue_attached_pictures(pls->ctx);
> +            ff_id3v2_parse_priv(pls->ctx, &pls->id3_deferred_extra);
>              ff_id3v2_free_extra_meta(&pls->id3_deferred_extra);
>              pls->id3_deferred_extra = NULL;
>          }
> @@ -1986,6 +1988,13 @@ static int hls_read_header(AVFormatContext *s)
>          if (ret < 0)
>              goto fail;
>  
> +        /*
> +         * Copy any metadata from playlist to main streams, but do not set
> +         * event flags.
> +         */
> +        if (pls->n_main_streams)
> +            av_dict_copy(&pls->main_streams[0]->metadata, pls->ctx->metadata, 0);
> +
>          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_AUDIO);
>          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_VIDEO);
>          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_SUBTITLE);
> @@ -2170,6 +2179,17 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>              return ret;
>          }
>  
> +        // If sub-demuxer reports updated metadata, copy it to the first stream
> +        // and set its AVSTREAM_EVENT_FLAG_METADATA_UPDATED flag.
> +        if (pls->ctx->event_flags & AVFMT_EVENT_FLAG_METADATA_UPDATED) {
> +            if (pls->n_main_streams) {
> +                st = pls->main_streams[0];
> +                av_dict_copy(&st->metadata, pls->ctx->metadata, 0);
> +                st->event_flags |= AVSTREAM_EVENT_FLAG_METADATA_UPDATED;
> +            }
> +            pls->ctx->event_flags &= ~AVFMT_EVENT_FLAG_METADATA_UPDATED;
> +        }
> +
>          /* check if noheader flag has been cleared by the subdemuxer */
>          if (pls->has_noheader_flag && !(pls->ctx->ctx_flags & AVFMTCTX_NOHEADER)) {
>              pls->has_noheader_flag = 0;

Fine then. If it works ad nobody else has comments, I can push on Monday
or so.
rshaffer@tunein.com Feb. 3, 2018, 6:47 p.m. UTC | #2
On Saturday, February 3, 2018, wm4 <nfxjfg@googlemail.com> wrote:

> On Fri,  2 Feb 2018 12:59:45 -0800
> rshaffer@tunein.com wrote:
>
> > From: Richard Shaffer <rshaffer@tunein.com>
> >
> > If a subdemuxer has the updated metadata event flag set, the metadata is
> copied
> > to the corresponding stream. The flag is cleared on the subdemuxer and
> the
> > appropriate event flag is set on the stream.
> > ---
> > This is iteration #2.
>
> You can pass "-v2" as argument to git send-email to automatically add
> "v2" to the subject.
>
> Oh, thanks. I guess I should have read the man page.

> >
> > In this version, we don't try to copy metadata from subdemuxer streams,
> only
> > from the AVFormatContext. The case where metadata is updated on a
> sub-demuxer
> > stream would have probably never happened.
>
> Oh, I thought it was the other way around.
>
> > The above change made the code that was previously in the
> > update_metadata_from_subdemuxer function a little simpler. That
> function was
> > also kind of ugly, because in one case it read and set flags, and in
> another it
> > didn't. It seemed simpler just to move the respective updates into
> read_header
> > and read_packet, respectively, so that's what I did.
> >
> > -Richard
> >
> >  libavformat/hls.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > index 9bd54c84cc..c578bf86e3 100644
> > --- a/libavformat/hls.c
> > +++ b/libavformat/hls.c
> > @@ -1062,6 +1062,7 @@ static void handle_id3(AVIOContext *pb, struct
> playlist *pls)
> >              /* demuxer not yet opened, defer picture attachment */
> >              pls->id3_deferred_extra = extra_meta;
> >
> > +        ff_id3v2_parse_priv_dict(&metadata, &extra_meta);
> >          av_dict_copy(&pls->ctx->metadata, metadata, 0);
> >          pls->id3_initial = metadata;
> >
> > @@ -1960,6 +1961,7 @@ static int hls_read_header(AVFormatContext *s)
> >          if (pls->id3_deferred_extra && pls->ctx->nb_streams == 1) {
> >              ff_id3v2_parse_apic(pls->ctx, &pls->id3_deferred_extra);
> >              avformat_queue_attached_pictures(pls->ctx);
> > +            ff_id3v2_parse_priv(pls->ctx, &pls->id3_deferred_extra);
> >              ff_id3v2_free_extra_meta(&pls->id3_deferred_extra);
> >              pls->id3_deferred_extra = NULL;
> >          }
> > @@ -1986,6 +1988,13 @@ static int hls_read_header(AVFormatContext *s)
> >          if (ret < 0)
> >              goto fail;
> >
> > +        /*
> > +         * Copy any metadata from playlist to main streams, but do not
> set
> > +         * event flags.
> > +         */
> > +        if (pls->n_main_streams)
> > +            av_dict_copy(&pls->main_streams[0]->metadata,
> pls->ctx->metadata, 0);
> > +
> >          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_AUDIO);
> >          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_VIDEO);
> >          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_SUBTITLE);
> > @@ -2170,6 +2179,17 @@ static int hls_read_packet(AVFormatContext *s,
> AVPacket *pkt)
> >              return ret;
> >          }
> >
> > +        // If sub-demuxer reports updated metadata, copy it to the
> first stream
> > +        // and set its AVSTREAM_EVENT_FLAG_METADATA_UPDATED flag.
> > +        if (pls->ctx->event_flags & AVFMT_EVENT_FLAG_METADATA_UPDATED)
> {
> > +            if (pls->n_main_streams) {
> > +                st = pls->main_streams[0];
> > +                av_dict_copy(&st->metadata, pls->ctx->metadata, 0);
> > +                st->event_flags |= AVSTREAM_EVENT_FLAG_METADATA_
> UPDATED;
> > +            }
> > +            pls->ctx->event_flags &= ~AVFMT_EVENT_FLAG_METADATA_
> UPDATED;
> > +        }
> > +
> >          /* check if noheader flag has been cleared by the subdemuxer */
> >          if (pls->has_noheader_flag && !(pls->ctx->ctx_flags &
> AVFMTCTX_NOHEADER)) {
> >              pls->has_noheader_flag = 0;
>
> Fine then. If it works ad nobody else has comments, I can push on Monday
> or so.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
wm4 Feb. 7, 2018, 11:43 a.m. UTC | #3
On Fri,  2 Feb 2018 12:59:45 -0800
rshaffer@tunein.com wrote:

> From: Richard Shaffer <rshaffer@tunein.com>
> 
> If a subdemuxer has the updated metadata event flag set, the metadata is copied
> to the corresponding stream. The flag is cleared on the subdemuxer and the
> appropriate event flag is set on the stream.
> ---
> This is iteration #2.
> 
> In this version, we don't try to copy metadata from subdemuxer streams, only
> from the AVFormatContext. The case where metadata is updated on a sub-demuxer
> stream would have probably never happened.
> 
> The above change made the code that was previously in the
> update_metadata_from_subdemuxer function a little simpler. That function was
> also kind of ugly, because in one case it read and set flags, and in another it
> didn't. It seemed simpler just to move the respective updates into read_header
> and read_packet, respectively, so that's what I did.
> 
> -Richard
> 
>  libavformat/hls.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 9bd54c84cc..c578bf86e3 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -1062,6 +1062,7 @@ static void handle_id3(AVIOContext *pb, struct playlist *pls)
>              /* demuxer not yet opened, defer picture attachment */
>              pls->id3_deferred_extra = extra_meta;
>  
> +        ff_id3v2_parse_priv_dict(&metadata, &extra_meta);
>          av_dict_copy(&pls->ctx->metadata, metadata, 0);
>          pls->id3_initial = metadata;
>  
> @@ -1960,6 +1961,7 @@ static int hls_read_header(AVFormatContext *s)
>          if (pls->id3_deferred_extra && pls->ctx->nb_streams == 1) {
>              ff_id3v2_parse_apic(pls->ctx, &pls->id3_deferred_extra);
>              avformat_queue_attached_pictures(pls->ctx);
> +            ff_id3v2_parse_priv(pls->ctx, &pls->id3_deferred_extra);
>              ff_id3v2_free_extra_meta(&pls->id3_deferred_extra);
>              pls->id3_deferred_extra = NULL;
>          }
> @@ -1986,6 +1988,13 @@ static int hls_read_header(AVFormatContext *s)
>          if (ret < 0)
>              goto fail;
>  
> +        /*
> +         * Copy any metadata from playlist to main streams, but do not set
> +         * event flags.
> +         */
> +        if (pls->n_main_streams)
> +            av_dict_copy(&pls->main_streams[0]->metadata, pls->ctx->metadata, 0);
> +
>          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_AUDIO);
>          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_VIDEO);
>          add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_SUBTITLE);
> @@ -2170,6 +2179,17 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>              return ret;
>          }
>  
> +        // If sub-demuxer reports updated metadata, copy it to the first stream
> +        // and set its AVSTREAM_EVENT_FLAG_METADATA_UPDATED flag.
> +        if (pls->ctx->event_flags & AVFMT_EVENT_FLAG_METADATA_UPDATED) {
> +            if (pls->n_main_streams) {
> +                st = pls->main_streams[0];
> +                av_dict_copy(&st->metadata, pls->ctx->metadata, 0);
> +                st->event_flags |= AVSTREAM_EVENT_FLAG_METADATA_UPDATED;
> +            }
> +            pls->ctx->event_flags &= ~AVFMT_EVENT_FLAG_METADATA_UPDATED;
> +        }
> +
>          /* check if noheader flag has been cleared by the subdemuxer */
>          if (pls->has_noheader_flag && !(pls->ctx->ctx_flags & AVFMTCTX_NOHEADER)) {
>              pls->has_noheader_flag = 0;

Pushed, with subject prefix slightly adjusted.
diff mbox

Patch

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 9bd54c84cc..c578bf86e3 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1062,6 +1062,7 @@  static void handle_id3(AVIOContext *pb, struct playlist *pls)
             /* demuxer not yet opened, defer picture attachment */
             pls->id3_deferred_extra = extra_meta;
 
+        ff_id3v2_parse_priv_dict(&metadata, &extra_meta);
         av_dict_copy(&pls->ctx->metadata, metadata, 0);
         pls->id3_initial = metadata;
 
@@ -1960,6 +1961,7 @@  static int hls_read_header(AVFormatContext *s)
         if (pls->id3_deferred_extra && pls->ctx->nb_streams == 1) {
             ff_id3v2_parse_apic(pls->ctx, &pls->id3_deferred_extra);
             avformat_queue_attached_pictures(pls->ctx);
+            ff_id3v2_parse_priv(pls->ctx, &pls->id3_deferred_extra);
             ff_id3v2_free_extra_meta(&pls->id3_deferred_extra);
             pls->id3_deferred_extra = NULL;
         }
@@ -1986,6 +1988,13 @@  static int hls_read_header(AVFormatContext *s)
         if (ret < 0)
             goto fail;
 
+        /*
+         * Copy any metadata from playlist to main streams, but do not set
+         * event flags.
+         */
+        if (pls->n_main_streams)
+            av_dict_copy(&pls->main_streams[0]->metadata, pls->ctx->metadata, 0);
+
         add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_AUDIO);
         add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_VIDEO);
         add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_SUBTITLE);
@@ -2170,6 +2179,17 @@  static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
             return ret;
         }
 
+        // If sub-demuxer reports updated metadata, copy it to the first stream
+        // and set its AVSTREAM_EVENT_FLAG_METADATA_UPDATED flag.
+        if (pls->ctx->event_flags & AVFMT_EVENT_FLAG_METADATA_UPDATED) {
+            if (pls->n_main_streams) {
+                st = pls->main_streams[0];
+                av_dict_copy(&st->metadata, pls->ctx->metadata, 0);
+                st->event_flags |= AVSTREAM_EVENT_FLAG_METADATA_UPDATED;
+            }
+            pls->ctx->event_flags &= ~AVFMT_EVENT_FLAG_METADATA_UPDATED;
+        }
+
         /* check if noheader flag has been cleared by the subdemuxer */
         if (pls->has_noheader_flag && !(pls->ctx->ctx_flags & AVFMTCTX_NOHEADER)) {
             pls->has_noheader_flag = 0;