applehttp: Expose the stream bitrate via metadata

Message ID 1303300182-3408-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö April 20, 2011, 11:49 a.m.
This helps callers to intelligently switch between bitrate
variants.
---
 libavformat/applehttp.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Comments

Ronald Bultje April 20, 2011, 12:03 p.m. | #1
Hi,

On Wed, Apr 20, 2011 at 7:49 AM, Martin Storsjö <martin@martin.st> wrote:
> This helps callers to intelligently switch between bitrate
> variants.
> ---
>  libavformat/applehttp.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Doesn't AVCodecContext have a bit_rate integer for this purpose?

Ronald
Martin Storsjö April 20, 2011, 12:05 p.m. | #2
On Wed, 20 Apr 2011, Ronald S. Bultje wrote:

> Hi,
> 
> On Wed, Apr 20, 2011 at 7:49 AM, Martin Storsjö <martin@martin.st> wrote:
> > This helps callers to intelligently switch between bitrate
> > variants.
> > ---
> >  libavformat/applehttp.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> Doesn't AVCodecContext have a bit_rate integer for this purpose?

It does, but this is for a slightly different use. The bitrate in 
AVCodecContext is for the actual bitrate of that stream only, while this 
would convey the total bitrate of the whole bitrate variant that this 
stream belongs to. So for that, a different name perhaps would be better, 
like "variant_total_bitrate" or something similar?

// Martin
Ronald Bultje April 20, 2011, 12:33 p.m. | #3
Hi,

On Wed, Apr 20, 2011 at 8:05 AM, Martin Storsjö <martin@martin.st> wrote:
> On Wed, 20 Apr 2011, Ronald S. Bultje wrote:
>> On Wed, Apr 20, 2011 at 7:49 AM, Martin Storsjö <martin@martin.st> wrote:
>> > This helps callers to intelligently switch between bitrate
>> > variants.
>> > ---
>> >  libavformat/applehttp.c |    3 +++
>> >  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> Doesn't AVCodecContext have a bit_rate integer for this purpose?
>
> It does, but this is for a slightly different use. The bitrate in
> AVCodecContext is for the actual bitrate of that stream only, while this
> would convey the total bitrate of the whole bitrate variant that this
> stream belongs to. So for that, a different name perhaps would be better,
> like "variant_total_bitrate" or something similar?

What's the difference?

Ronald
Martin Storsjö April 20, 2011, 1:40 p.m. | #4
On Wed, 20 Apr 2011, Ronald S. Bultje wrote:

> Hi,
> 
> On Wed, Apr 20, 2011 at 8:05 AM, Martin Storsjö <martin@martin.st> wrote:
> > On Wed, 20 Apr 2011, Ronald S. Bultje wrote:
> >> On Wed, Apr 20, 2011 at 7:49 AM, Martin Storsjö <martin@martin.st> wrote:
> >> > This helps callers to intelligently switch between bitrate
> >> > variants.
> >> > ---
> >> >  libavformat/applehttp.c |    3 +++
> >> >  1 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> Doesn't AVCodecContext have a bit_rate integer for this purpose?
> >
> > It does, but this is for a slightly different use. The bitrate in
> > AVCodecContext is for the actual bitrate of that stream only, while this
> > would convey the total bitrate of the whole bitrate variant that this
> > stream belongs to. So for that, a different name perhaps would be better,
> > like "variant_total_bitrate" or something similar?
> 
> What's the difference?

As Vladimir explained on irc - it's the total bitrate of all streams 
within the same bitrate variants. If I've got a stream with two variants, 
100 kbps and 500 kbps, both having audio and video, I'd set "100 kbps" as 
variant_total_bitrate on the two streams from the first variant and "500 
kbps" on the other two. The audio stream bitrate may well be the same, 
e.g. 32 kbps in both of them, while the video would be 68 in the former 
and 468 in the latter.

Therefore, when switching, the caller needs to know which streams belong 
to the same bitrate variant, so that he doesn't accidentally pull from 
more than one variant at a time, and preferrably also know what their 
total bitrate is.

// Martin
Martin Storsjö April 20, 2011, 1:44 p.m. | #5
On Wed, 20 Apr 2011, Martin Storsjö wrote:

> On Wed, 20 Apr 2011, Ronald S. Bultje wrote:
> 
> > Hi,
> > 
> > On Wed, Apr 20, 2011 at 8:05 AM, Martin Storsjö <martin@martin.st> wrote:
> > > On Wed, 20 Apr 2011, Ronald S. Bultje wrote:
> > >> On Wed, Apr 20, 2011 at 7:49 AM, Martin Storsjö <martin@martin.st> wrote:
> > >> > This helps callers to intelligently switch between bitrate
> > >> > variants.
> > >> > ---
> > >> >  libavformat/applehttp.c |    3 +++
> > >> >  1 files changed, 3 insertions(+), 0 deletions(-)
> > >>
> > >> Doesn't AVCodecContext have a bit_rate integer for this purpose?
> > >
> > > It does, but this is for a slightly different use. The bitrate in
> > > AVCodecContext is for the actual bitrate of that stream only, while this
> > > would convey the total bitrate of the whole bitrate variant that this
> > > stream belongs to. So for that, a different name perhaps would be better,
> > > like "variant_total_bitrate" or something similar?
> > 
> > What's the difference?
> 
> As Vladimir explained on irc - it's the total bitrate of all streams 
> within the same bitrate variants. If I've got a stream with two variants, 
> 100 kbps and 500 kbps, both having audio and video, I'd set "100 kbps" as 
> variant_total_bitrate on the two streams from the first variant and "500 
> kbps" on the other two. The audio stream bitrate may well be the same, 
> e.g. 32 kbps in both of them, while the video would be 68 in the former 
> and 468 in the latter.
> 
> Therefore, when switching, the caller needs to know which streams belong 
> to the same bitrate variant, so that he doesn't accidentally pull from 
> more than one variant at a time, and preferrably also know what their 
> total bitrate is.

To add more to the explanation - currently, the info about which AVStream 
belongs to which variant stream actually is available, it's stored in 
AVStream->id, where the int value says which variant it is a part of - all 
AVStreams with identical id belong together. But someone from XBMC asked 
me yesterday to provide the variant's total bitrate, too, which is useful 
for making better decisions on switching variants.

// Martin
Ronald Bultje April 20, 2011, 2:21 p.m. | #6
Hi,

On Wed, Apr 20, 2011 at 9:44 AM, Martin Storsjö <martin@martin.st> wrote:
> To add more to the explanation - currently, the info about which AVStream
> belongs to which variant stream actually is available, it's stored in
> AVStream->id, where the int value says which variant it is a part of - all
> AVStreams with identical id belong together. But someone from XBMC asked
> me yesterday to provide the variant's total bitrate, too, which is useful
> for making better decisions on switching variants.

At this point, do we know the bitrates of the individual streams? We
could simply put that in AVStream->codec->bit_rate and let callers add
it up...

Ronald
Martin Storsjö April 20, 2011, 5:47 p.m. | #7
On Wed, 20 Apr 2011, Ronald S. Bultje wrote:

> On Wed, Apr 20, 2011 at 9:44 AM, Martin Storsjö <martin@martin.st> wrote:
> > To add more to the explanation - currently, the info about which AVStream
> > belongs to which variant stream actually is available, it's stored in
> > AVStream->id, where the int value says which variant it is a part of - all
> > AVStreams with identical id belong together. But someone from XBMC asked
> > me yesterday to provide the variant's total bitrate, too, which is useful
> > for making better decisions on switching variants.
> 
> At this point, do we know the bitrates of the individual streams? We
> could simply put that in AVStream->codec->bit_rate and let callers add
> it up...

Only if the chained mpegts demuxer sets it, and it appears it doesn't. 
Also, within the applehttp metadata, there is an explicit bitrate field 
saying what the nominal bitrate of that variant is, we should pass it 
through somewhere at least.

So, would a "variant_bitrate" metadata key be acceptable? Anton?

// Martin
Anton Khirnov April 20, 2011, 7:59 p.m. | #8
On Wed, 20 Apr 2011 20:47:11 +0300 (EEST), Martin Storsjö <martin@martin.st> wrote:
> On Wed, 20 Apr 2011, Ronald S. Bultje wrote:
> 
> > On Wed, Apr 20, 2011 at 9:44 AM, Martin Storsjö <martin@martin.st> wrote:
> > > To add more to the explanation - currently, the info about which AVStream
> > > belongs to which variant stream actually is available, it's stored in
> > > AVStream->id, where the int value says which variant it is a part of - all
> > > AVStreams with identical id belong together. But someone from XBMC asked
> > > me yesterday to provide the variant's total bitrate, too, which is useful
> > > for making better decisions on switching variants.
> > 
> > At this point, do we know the bitrates of the individual streams? We
> > could simply put that in AVStream->codec->bit_rate and let callers add
> > it up...
> 
> Only if the chained mpegts demuxer sets it, and it appears it doesn't. 
> Also, within the applehttp metadata, there is an explicit bitrate field 
> saying what the nominal bitrate of that variant is, we should pass it 
> through somewhere at least.
> 
> So, would a "variant_bitrate" metadata key be acceptable? Anton?

Sure, if you document it in some public place.

btw I wonder if we should add numeric (and possibly other) metadata types.

Patch

diff --git a/libavformat/applehttp.c b/libavformat/applehttp.c
index df4494a..ac680f4 100644
--- a/libavformat/applehttp.c
+++ b/libavformat/applehttp.c
@@ -367,6 +367,7 @@  static int applehttp_read_header(AVFormatContext *s, AVFormatParameters *ap)
     for (i = 0; i < c->n_variants; i++) {
         struct variant *v = c->variants[i];
         AVInputFormat *in_fmt = NULL;
+        char bitrate_str[20];
         if (v->n_segments == 0)
             continue;
 
@@ -393,6 +394,7 @@  static int applehttp_read_header(AVFormatContext *s, AVFormatParameters *ap)
         if (ret < 0)
             goto fail;
         v->stream_offset = stream_offset;
+        snprintf(bitrate_str, sizeof(bitrate_str), "%d", v->bandwidth);
         /* Create new AVStreams for each stream in this variant */
         for (j = 0; j < v->ctx->nb_streams; j++) {
             AVStream *st = av_new_stream(s, i);
@@ -401,6 +403,7 @@  static int applehttp_read_header(AVFormatContext *s, AVFormatParameters *ap)
                 goto fail;
             }
             avcodec_copy_context(st->codec, v->ctx->streams[j]->codec);
+            av_metadata_set2(&st->metadata, "bitrate", bitrate_str, 0);
         }
         stream_offset += v->ctx->nb_streams;
     }