[07/10] hls: Call avformat_find_stream_info() on the chained demuxers

Message ID 1375038091-92151-7-git-send-email-martin@martin.st
State Committed
Headers show

Commit Message

Martin Storsjö July 28, 2013, 7:01 p.m.
From: Michael Niedermayer <michaelni@gmx.at>

This allows the chained demuxer (or more precisely, the lavf
utility code) to better fill in timestamps on packets from
these, especially for cases where one stream is a raw ADTS
stream.
---
 libavformat/hls.c |    5 +++++
 1 file changed, 5 insertions(+)

Comments

Luca Barbato July 28, 2013, 8:10 p.m. | #1
On 28/07/13 21:01, Martin Storsjö wrote:
> From: Michael Niedermayer <michaelni@gmx.at>
> 
> This allows the chained demuxer (or more precisely, the lavf
> utility code) to better fill in timestamps on packets from
> these, especially for cases where one stream is a raw ADTS
> stream.
> ---
>  libavformat/hls.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 

Looks dangerous, is it _really_ needed?

lu
Martin Storsjö July 28, 2013, 8:13 p.m. | #2
On Sun, 28 Jul 2013, Luca Barbato wrote:

> On 28/07/13 21:01, Martin Storsjö wrote:
>> From: Michael Niedermayer <michaelni@gmx.at>
>> 
>> This allows the chained demuxer (or more precisely, the lavf
>> utility code) to better fill in timestamps on packets from
>> these, especially for cases where one stream is a raw ADTS
>> stream.
>> ---
>>  libavformat/hls.c |    5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>
> Looks dangerous, is it _really_ needed?

Indeed, it's not something I really want, but it's kinda needed to get 
some kind of proper timestamps if one of the streams is ADTS. Otherwise I 
just get pts=dts=NOPTS on all of them, and it's really hard to return 
those in a decent order interleaved with packets from a separate mpegts 
stream.

// Martin
Luca Barbato July 28, 2013, 8:16 p.m. | #3
On 28/07/13 22:13, Martin Storsjö wrote:
> Indeed, it's not something I really want, but it's kinda needed to get
> some kind of proper timestamps if one of the streams is ADTS. Otherwise
> I just get pts=dts=NOPTS on all of them, and it's really hard to return
> those in a decent order interleaved with packets from a separate mpegts
> stream.

Let's extract the only bit really needed and use it.

find_stream_info is one of the sorest part of libav.

lu
Martin Storsjö July 29, 2013, 9:17 a.m. | #4
On Sun, 28 Jul 2013, Luca Barbato wrote:

> On 28/07/13 22:13, Martin Storsjö wrote:
>> Indeed, it's not something I really want, but it's kinda needed to get
>> some kind of proper timestamps if one of the streams is ADTS. Otherwise
>> I just get pts=dts=NOPTS on all of them, and it's really hard to return
>> those in a decent order interleaved with packets from a separate mpegts
>> stream.
>
> Let's extract the only bit really needed and use it.
>
> find_stream_info is one of the sorest part of libav.

The part that is required in this case is decoding a packet, in order to 
set frame_size and sample_rate in the AVCodecContext. When they are set, 
the common lavf "magic" can fill in the timestamps for the packets lacking 
them.

If we can't fill in some sort of timestamps for these streams, we can't 
return the packets from all variants in a nice interleaved way, but return 
all packets from this stream for the current segment before the other 
variants.

If the caller of the hls demuxer (e.g. avplay/avconv) uses 
find_stream_info, it will probably give up before ever seeing any packet 
from the other streams (because it's given e.g. 10 seconds of packets from 
the audio stream before ever getting anything from the video stream), so 
they'll remain unmapped in avplay/avconv.

// Martin
Martin Storsjö July 30, 2013, 9:19 a.m. | #5
On Mon, 29 Jul 2013, Martin Storsjö wrote:

> On Sun, 28 Jul 2013, Luca Barbato wrote:
>
>> On 28/07/13 22:13, Martin Storsjö wrote:
>>> Indeed, it's not something I really want, but it's kinda needed to get
>>> some kind of proper timestamps if one of the streams is ADTS. Otherwise
>>> I just get pts=dts=NOPTS on all of them, and it's really hard to return
>>> those in a decent order interleaved with packets from a separate mpegts
>>> stream.
>> 
>> Let's extract the only bit really needed and use it.
>> 
>> find_stream_info is one of the sorest part of libav.
>
> The part that is required in this case is decoding a packet, in order to set 
> frame_size and sample_rate in the AVCodecContext. When they are set, the 
> common lavf "magic" can fill in the timestamps for the packets lacking them.
>
> If we can't fill in some sort of timestamps for these streams, we can't 
> return the packets from all variants in a nice interleaved way, but return 
> all packets from this stream for the current segment before the other 
> variants.
>
> If the caller of the hls demuxer (e.g. avplay/avconv) uses find_stream_info, 
> it will probably give up before ever seeing any packet from the other streams 
> (because it's given e.g. 10 seconds of packets from the audio stream before 
> ever getting anything from the video stream), so they'll remain unmapped in 
> avplay/avconv.

After some discussion on irc, this was reluctantly ok'd by Luca.

// Martin
Luca Barbato July 30, 2013, 9:21 a.m. | #6
On 30/07/13 11:19, Martin Storsjö wrote:
> On Mon, 29 Jul 2013, Martin Storsjö wrote:
> 
>> On Sun, 28 Jul 2013, Luca Barbato wrote:
>>
>>> On 28/07/13 22:13, Martin Storsjö wrote:
>>>> Indeed, it's not something I really want, but it's kinda needed to get
>>>> some kind of proper timestamps if one of the streams is ADTS. Otherwise
>>>> I just get pts=dts=NOPTS on all of them, and it's really hard to return
>>>> those in a decent order interleaved with packets from a separate mpegts
>>>> stream.
>>>
>>> Let's extract the only bit really needed and use it.
>>>
>>> find_stream_info is one of the sorest part of libav.
>>
>> The part that is required in this case is decoding a packet, in order
>> to set frame_size and sample_rate in the AVCodecContext. When they are
>> set, the common lavf "magic" can fill in the timestamps for the
>> packets lacking them.
>>
>> If we can't fill in some sort of timestamps for these streams, we
>> can't return the packets from all variants in a nice interleaved way,
>> but return all packets from this stream for the current segment before
>> the other variants.
>>
>> If the caller of the hls demuxer (e.g. avplay/avconv) uses
>> find_stream_info, it will probably give up before ever seeing any
>> packet from the other streams (because it's given e.g. 10 seconds of
>> packets from the audio stream before ever getting anything from the
>> video stream), so they'll remain unmapped in avplay/avconv.
> 
> After some discussion on irc, this was reluctantly ok'd by Luca.

If it would break as I'm afraid it would I'd just have one more excuse
to cleanup that thing properly =P.

lu

Patch

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 60a1532..09c97f5 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -527,6 +527,11 @@  static int hls_read_header(AVFormatContext *s)
         ret = avformat_open_input(&v->ctx, v->segments[0]->url, in_fmt, NULL);
         if (ret < 0)
             goto fail;
+
+        v->ctx->ctx_flags &= ~AVFMTCTX_NOHEADER;
+        ret = avformat_find_stream_info(v->ctx, NULL);
+        if (ret < 0)
+            goto fail;
         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++) {