[08/10] hls: Respect the different stream time bases when comparing dts

Message ID 1375038091-92151-8-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>

Also adjust the streams timestamps according to their start
timestamp when comparing. This helps getting correctly interleaved
packets if one stream lacks timestamps (such as a plain ADTS
stream when the other variants are full mpegts) when the others
have timestamps that don't start from zero.

This probably doesn't work properly if such a stream is
temporarily disabled (via the discard flags) and then reenabled,
and such streams are hard to correctly sync against the other
streams as well - but this works better than before at least.

The segment number restriction makes sure all variants advance
roughly at the same pace as well.
---
 libavformat/hls.c |   28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

Comments

Luca Barbato July 28, 2013, 8:15 p.m. | #1
On 28/07/13 21:01, Martin Storsjö wrote:
> From: Michael Niedermayer <michaelni@gmx.at>
> 
> Also adjust the streams timestamps according to their start
> timestamp when comparing. This helps getting correctly interleaved
> packets if one stream lacks timestamps (such as a plain ADTS
> stream when the other variants are full mpegts) when the others
> have timestamps that don't start from zero.
> 
> This probably doesn't work properly if such a stream is
> temporarily disabled (via the discard flags) and then reenabled,
> and such streams are hard to correctly sync against the other
> streams as well - but this works better than before at least.
> 
> The segment number restriction makes sure all variants advance
> roughly at the same pace as well.
> ---
>  libavformat/hls.c |   28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 09c97f5..7cde121 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -649,11 +649,31 @@ start:
>          /* Check if this stream still is on an earlier segment number, or
>           * has the packet with the lowest dts */
>          if (var->pkt.data) {
> -            if (minvariant < 0 ||
> -                var->cur_seq_no <  c->variants[minvariant]->cur_seq_no ||
> -               (var->cur_seq_no == c->variants[minvariant]->cur_seq_no &&
> -                var->pkt.dts    <  c->variants[minvariant]->pkt.dts))
> +            struct variant *minvar = c->variants[minvariant];
                               ^^^^^^

> +            if (minvariant < 0 || var->cur_seq_no < minvar->cur_seq_no) {
>                  minvariant = i;
> +            } else if (var->cur_seq_no == minvar->cur_seq_no) {
> +                struct variant *minvar = c->variants[minvariant];
                                   ^^^^^^
why?

> +                int64_t dts     =    var->pkt.dts;
> +                int64_t mindts  = minvar->pkt.dts;
> +                AVStream *st    =    var->ctx->streams[var->pkt.stream_index];
> +                AVStream *minst = minvar->ctx->streams[minvar->pkt.stream_index];
> +
> +                if (dts == AV_NOPTS_VALUE) {
> +                    minvariant = i;
> +                } else if (mindts == AV_NOPTS_VALUE) {
> +                    // keep current choice

maybe is redundant?

> +                } else {
> +                    if (st->start_time    != AV_NOPTS_VALUE)
> +                        dts    -= st->start_time;
> +                    if (minst->start_time != AV_NOPTS_VALUE)
> +                        mindts -= minst->start_time;
> +
> +                    if (av_compare_ts(dts, st->time_base,
> +                                      mindts, minst->time_base) < 0)
> +                        minvariant = i;
> +                }
> +            }
>          }
>      }
>      if (c->end_of_segment) {
> 

beside that seems fine.

lu
Martin Storsjö July 28, 2013, 8:21 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>
>> 
>> Also adjust the streams timestamps according to their start
>> timestamp when comparing. This helps getting correctly interleaved
>> packets if one stream lacks timestamps (such as a plain ADTS
>> stream when the other variants are full mpegts) when the others
>> have timestamps that don't start from zero.
>> 
>> This probably doesn't work properly if such a stream is
>> temporarily disabled (via the discard flags) and then reenabled,
>> and such streams are hard to correctly sync against the other
>> streams as well - but this works better than before at least.
>> 
>> The segment number restriction makes sure all variants advance
>> roughly at the same pace as well.
>> ---
>>  libavformat/hls.c |   28 ++++++++++++++++++++++++----
>>  1 file changed, 24 insertions(+), 4 deletions(-)
>> 
>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>> index 09c97f5..7cde121 100644
>> --- a/libavformat/hls.c
>> +++ b/libavformat/hls.c
>> @@ -649,11 +649,31 @@ start:
>>          /* Check if this stream still is on an earlier segment number, or
>>           * has the packet with the lowest dts */
>>          if (var->pkt.data) {
>> -            if (minvariant < 0 ||
>> -                var->cur_seq_no <  c->variants[minvariant]->cur_seq_no ||
>> -               (var->cur_seq_no == c->variants[minvariant]->cur_seq_no &&
>> -                var->pkt.dts    <  c->variants[minvariant]->pkt.dts))
>> +            struct variant *minvar = c->variants[minvariant];
>                               ^^^^^^
>
>> +            if (minvariant < 0 || var->cur_seq_no < minvar->cur_seq_no) {
>>                  minvariant = i;
>> +            } else if (var->cur_seq_no == minvar->cur_seq_no) {
>> +                struct variant *minvar = c->variants[minvariant];
>                                   ^^^^^^
> why?

Oh, oops, this one isn't needed.

>> +                int64_t dts     =    var->pkt.dts;
>> +                int64_t mindts  = minvar->pkt.dts;
>> +                AVStream *st    =    var->ctx->streams[var->pkt.stream_index];
>> +                AVStream *minst = minvar->ctx->streams[minvar->pkt.stream_index];
>> +
>> +                if (dts == AV_NOPTS_VALUE) {
>> +                    minvariant = i;
>> +                } else if (mindts == AV_NOPTS_VALUE) {
>> +                    // keep current choice
>
> maybe is redundant?

Well I guess I could remove this else if and change the last else branch 
into "else if (mindts != AV_NOPTS_VALUE) {", that's probably better.

// Martin

Patch

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 09c97f5..7cde121 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -649,11 +649,31 @@  start:
         /* Check if this stream still is on an earlier segment number, or
          * has the packet with the lowest dts */
         if (var->pkt.data) {
-            if (minvariant < 0 ||
-                var->cur_seq_no <  c->variants[minvariant]->cur_seq_no ||
-               (var->cur_seq_no == c->variants[minvariant]->cur_seq_no &&
-                var->pkt.dts    <  c->variants[minvariant]->pkt.dts))
+            struct variant *minvar = c->variants[minvariant];
+            if (minvariant < 0 || var->cur_seq_no < minvar->cur_seq_no) {
                 minvariant = i;
+            } else if (var->cur_seq_no == minvar->cur_seq_no) {
+                struct variant *minvar = c->variants[minvariant];
+                int64_t dts     =    var->pkt.dts;
+                int64_t mindts  = minvar->pkt.dts;
+                AVStream *st    =    var->ctx->streams[var->pkt.stream_index];
+                AVStream *minst = minvar->ctx->streams[minvar->pkt.stream_index];
+
+                if (dts == AV_NOPTS_VALUE) {
+                    minvariant = i;
+                } else if (mindts == AV_NOPTS_VALUE) {
+                    // keep current choice
+                } else {
+                    if (st->start_time    != AV_NOPTS_VALUE)
+                        dts    -= st->start_time;
+                    if (minst->start_time != AV_NOPTS_VALUE)
+                        mindts -= minst->start_time;
+
+                    if (av_compare_ts(dts, st->time_base,
+                                      mindts, minst->time_base) < 0)
+                        minvariant = i;
+                }
+            }
         }
     }
     if (c->end_of_segment) {