use ASF preroll as st->start_time

Message ID 4D821620.4050006@gmail.com
State Committed
Headers show

Commit Message

Vladimir Pantelic March 17, 2011, 2:09 p.m.
Måns Rullgård wrote:
> Vladimir Pantelic<vladoman@gmail.com>  writes:
>
>>  Vladimir Pantelic wrote:
>>>  Måns Rullgård wrote:
>>>>   Vladimir Pantelic<vladoman@gmail.com>    writes:
>>>
>>>>>    So, maybe using preroll as start_time is bad, but so is not doing it
>>>>>    as we can see above. I tend to rather follow the spec.
>>>>
>>>>   The alternative would be to subtract the preroll value from all
>>>>   timestamps in the demuxer and add it back in the muxer.  Your patch
>>>>   seems much simpler.
>>>
>>>  Reading the spec again, I think we should rather subtract the preroll
>>>  in the demuxer:
>>>
>>>  "...player software *must* subtract the value in the preroll field
>>>  from the play duration and presentation times to calculate their
>>>  actual values..."
>>
>>  now I set st->start_time to 0 and subtract the preroll, this way we
>>  output 0 based timestamps and prevent ffmpeg.c from guessing the wrong
>>  timestamps.
>>
>>   From 1c72c47cec36d6548db0f9a622dc2430e5d5bd93 Mon Sep 17 00:00:00 2001
>>  From: Vladimir Pantelic<vladoman@gmail.com>
>>  Date: Thu, 17 Mar 2011 14:56:14 +0100
>>  Subject: [PATCH] make ASF demuxer subtract the preroll value and thus output 0 based timestamps
>>
>>  ---
>>   libavformat/asfdec.c |    3 ++-
>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>
>>  diff --git a/libavformat/asfdec.c b/libavformat/asfdec.c
>>  index cbcd576..3d68527 100644
>>  --- a/libavformat/asfdec.c
>>  +++ b/libavformat/asfdec.c
>>  @@ -236,6 +236,7 @@ static int asf_read_stream_properties(AVFormatContext *s, int64_t size)
>>       if (!asf_st)
>>           return AVERROR(ENOMEM);
>>       st->priv_data = asf_st;
>>  +    st->start_time = 0;
>>       start_time = asf->hdr.preroll;
>>
>>       asf_st->stream_language_index = 128; // invalid stream index means no language info
>>  @@ -960,7 +961,7 @@ static int ff_asf_parse_packet(AVFormatContext *s, AVIOContext *pb, AVPacket *pk
>>               /* new packet */
>>               av_new_packet(&asf_st->pkt, asf->packet_obj_size);
>>               asf_st->seq = asf->packet_seq;
>>  -            asf_st->pkt.dts = asf->packet_frag_timestamp;
>>  +            asf_st->pkt.dts = asf->packet_frag_timestamp - asf->hdr.preroll;
>>               asf_st->pkt.stream_index = asf->stream_index;
>>               asf_st->pkt.pos =
>>               asf_st->packet_pos= asf->packet_pos;
>
> No PTS?

asf has only DTS

> The silly note about ASF in the start_time doxy should still go away.

done

Comments

Mans Rullgard March 17, 2011, 2:15 p.m. | #1
Vladimir Pantelic <vladoman@gmail.com> writes:

> Måns Rullgård wrote:
>> Vladimir Pantelic<vladoman@gmail.com>  writes:
>>
>>>  Vladimir Pantelic wrote:
>>>>  Måns Rullgård wrote:
>>>>>   Vladimir Pantelic<vladoman@gmail.com>    writes:
>>>>
>>>>>>    So, maybe using preroll as start_time is bad, but so is not doing it
>>>>>>    as we can see above. I tend to rather follow the spec.
>>>>>
>>>>>   The alternative would be to subtract the preroll value from all
>>>>>   timestamps in the demuxer and add it back in the muxer.  Your patch
>>>>>   seems much simpler.
>>>>
>>>>  Reading the spec again, I think we should rather subtract the preroll
>>>>  in the demuxer:
>>>>
>>>>  "...player software *must* subtract the value in the preroll field
>>>>  from the play duration and presentation times to calculate their
>>>>  actual values..."
>>>
>>>  now I set st->start_time to 0 and subtract the preroll, this way we
>>>  output 0 based timestamps and prevent ffmpeg.c from guessing the wrong
>>>  timestamps.
>>>
>>>   From 1c72c47cec36d6548db0f9a622dc2430e5d5bd93 Mon Sep 17 00:00:00 2001
>>>  From: Vladimir Pantelic<vladoman@gmail.com>
>>>  Date: Thu, 17 Mar 2011 14:56:14 +0100
>>>  Subject: [PATCH] make ASF demuxer subtract the preroll value and thus output 0 based timestamps
>>>
>>>  ---
>>>   libavformat/asfdec.c |    3 ++-
>>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>>  diff --git a/libavformat/asfdec.c b/libavformat/asfdec.c
>>>  index cbcd576..3d68527 100644
>>>  --- a/libavformat/asfdec.c
>>>  +++ b/libavformat/asfdec.c
>>>  @@ -236,6 +236,7 @@ static int asf_read_stream_properties(AVFormatContext *s, int64_t size)
>>>       if (!asf_st)
>>>           return AVERROR(ENOMEM);
>>>       st->priv_data = asf_st;
>>>  +    st->start_time = 0;
>>>       start_time = asf->hdr.preroll;
>>>
>>>       asf_st->stream_language_index = 128; // invalid stream index means no language info
>>>  @@ -960,7 +961,7 @@ static int ff_asf_parse_packet(AVFormatContext *s, AVIOContext *pb, AVPacket *pk
>>>               /* new packet */
>>>               av_new_packet(&asf_st->pkt, asf->packet_obj_size);
>>>               asf_st->seq = asf->packet_seq;
>>>  -            asf_st->pkt.dts = asf->packet_frag_timestamp;
>>>  +            asf_st->pkt.dts = asf->packet_frag_timestamp - asf->hdr.preroll;
>>>               asf_st->pkt.stream_index = asf->stream_index;
>>>               asf_st->pkt.pos =
>>>               asf_st->packet_pos= asf->packet_pos;
>>
>> No PTS?
>
> asf has only DTS
>
>> The silly note about ASF in the start_time doxy should still go away.
>
> done
>
>
> From e734562dded073e36f83534c2e504805c07abcd6 Mon Sep 17 00:00:00 2001
> From: Vladimir Pantelic <vladoman@gmail.com>
> Date: Thu, 17 Mar 2011 14:56:14 +0100
> Subject: [PATCH] make ASF demuxer subtract the preroll value and thus output 0 based timestamps
>
> ---
>  libavformat/asfdec.c   |    3 ++-
>  libavformat/avformat.h |    2 --
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/asfdec.c b/libavformat/asfdec.c
> index cbcd576..3d68527 100644
> --- a/libavformat/asfdec.c
> +++ b/libavformat/asfdec.c
> @@ -236,6 +236,7 @@ static int asf_read_stream_properties(AVFormatContext *s, int64_t size)
>      if (!asf_st)
>          return AVERROR(ENOMEM);
>      st->priv_data = asf_st;
> +    st->start_time = 0;
>      start_time = asf->hdr.preroll;
>  
>      asf_st->stream_language_index = 128; // invalid stream index means no language info
> @@ -960,7 +961,7 @@ static int ff_asf_parse_packet(AVFormatContext *s, AVIOContext *pb, AVPacket *pk
>              /* new packet */
>              av_new_packet(&asf_st->pkt, asf->packet_obj_size);
>              asf_st->seq = asf->packet_seq;
> -            asf_st->pkt.dts = asf->packet_frag_timestamp;
> +            asf_st->pkt.dts = asf->packet_frag_timestamp - asf->hdr.preroll;
>              asf_st->pkt.stream_index = asf->stream_index;
>              asf_st->pkt.pos =
>              asf_st->packet_pos= asf->packet_pos;
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index ce50053..ca179d2 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -534,8 +534,6 @@ typedef struct AVStream {
>       * Only set this if you are absolutely 100% sure that the value you set
>       * it to really is the pts of the first frame.
>       * This may be undefined (AV_NOPTS_VALUE).
> -     * @note The ASF header does NOT contain a correct start_time the ASF
> -     * demuxer must NOT set this.
>       */
>      int64_t start_time;
>  
> -- 
> 1.6.0.2

Looks fine.  Queued, will push later.

Patch

From e734562dded073e36f83534c2e504805c07abcd6 Mon Sep 17 00:00:00 2001
From: Vladimir Pantelic <vladoman@gmail.com>
Date: Thu, 17 Mar 2011 14:56:14 +0100
Subject: [PATCH] make ASF demuxer subtract the preroll value and thus output 0 based timestamps

---
 libavformat/asfdec.c   |    3 ++-
 libavformat/avformat.h |    2 --
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/libavformat/asfdec.c b/libavformat/asfdec.c
index cbcd576..3d68527 100644
--- a/libavformat/asfdec.c
+++ b/libavformat/asfdec.c
@@ -236,6 +236,7 @@  static int asf_read_stream_properties(AVFormatContext *s, int64_t size)
     if (!asf_st)
         return AVERROR(ENOMEM);
     st->priv_data = asf_st;
+    st->start_time = 0;
     start_time = asf->hdr.preroll;
 
     asf_st->stream_language_index = 128; // invalid stream index means no language info
@@ -960,7 +961,7 @@  static int ff_asf_parse_packet(AVFormatContext *s, AVIOContext *pb, AVPacket *pk
             /* new packet */
             av_new_packet(&asf_st->pkt, asf->packet_obj_size);
             asf_st->seq = asf->packet_seq;
-            asf_st->pkt.dts = asf->packet_frag_timestamp;
+            asf_st->pkt.dts = asf->packet_frag_timestamp - asf->hdr.preroll;
             asf_st->pkt.stream_index = asf->stream_index;
             asf_st->pkt.pos =
             asf_st->packet_pos= asf->packet_pos;
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index ce50053..ca179d2 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -534,8 +534,6 @@  typedef struct AVStream {
      * Only set this if you are absolutely 100% sure that the value you set
      * it to really is the pts of the first frame.
      * This may be undefined (AV_NOPTS_VALUE).
-     * @note The ASF header does NOT contain a correct start_time the ASF
-     * demuxer must NOT set this.
      */
     int64_t start_time;
 
-- 
1.6.0.2