[FFmpeg-devel] mov: Make sure PTS are both monotonically increasing, and unique

Submitted by Derek Buitenhuis on May 15, 2018, 7:44 p.m.

Details

Message ID 1526413487-24657-1-git-send-email-derek.buitenhuis@gmail.com
State New
Headers show

Commit Message

Derek Buitenhuis May 15, 2018, 7:44 p.m.
We already did this for audio, but it should be done for video too.
If we don't, seeking back to the start of the file, for example, can
become quite broken, since the first N packets will have repeating
and nonmonotonic PTS, yet they need to be decoded even if they are
to be discarded.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavformat/mov.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Tomas Härdin May 15, 2018, 8:10 p.m.
tis 2018-05-15 klockan 20:44 +0100 skrev Derek Buitenhuis:
> We already did this for audio, but it should be done for video too.
> If we don't, seeking back to the start of the file, for example, can
> become quite broken, since the first N packets will have repeating
> and nonmonotonic PTS, yet they need to be decoded even if they are
> to be discarded.

Why don't we do this for every format?

/Tomas
Derek Buitenhuis May 15, 2018, 8:21 p.m.
On Tue, May 15, 2018 at 9:10 PM, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> Why don't we do this for every format?

In this case, it is specific to MOV and MP4, since it's caused by the
way we apply edit lists at the packet level (reordering, marking them
as discard, changing timestamps, etc.). Previously we didn't much
care to set the discard-flagged packet timestamps properly, which is
what this fixes. It's not a high-level vsync sort of thing.

- Derek
Tomas Härdin May 15, 2018, 8:26 p.m.
tis 2018-05-15 klockan 21:21 +0100 skrev Derek Buitenhuis:
> On Tue, May 15, 2018 at 9:10 PM, Tomas Härdin <tjoppen@acc.umu.se>
> wrote:
> > Why don't we do this for every format?
> 
> In this case, it is specific to MOV and MP4, since it's caused by the
> way we apply edit lists at the packet level (reordering, marking them
> as discard, changing timestamps, etc.). Previously we didn't much
> care to set the discard-flagged packet timestamps properly, which is
> what this fixes. It's not a high-level vsync sort of thing.

I see. Well, I don't remember too much about MOV these day so I'll
leave this to someone else to actually comment on :o

/Tomas
Derek Buitenhuis May 17, 2018, 2:03 p.m.
On Tue, May 15, 2018 at 8:44 PM, Derek Buitenhuis
<derek.buitenhuis@gmail.com> wrote:
> We already did this for audio, but it should be done for video too.
> If we don't, seeking back to the start of the file, for example, can
> become quite broken, since the first N packets will have repeating
> and nonmonotonic PTS, yet they need to be decoded even if they are
> to be discarded.
>
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
>  libavformat/mov.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Ping.

Is nobody outside Sasi able to review code in this part of
mov.c? That is slightly worrying to me.

- Derek

Patch hide | download patch | download mbox

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 1975011..a74983f 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3579,7 +3579,7 @@  static void mov_fix_index(MOVContext *mov, AVStream *st)
                     flags |= AVINDEX_DISCARD_FRAME;
                     av_log(mov->fc, AV_LOG_DEBUG, "drop a frame at curr_cts: %"PRId64" @ %"PRId64"\n", curr_cts, index);
 
-                    if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && edit_list_start_encountered == 0) {
+                    if (edit_list_start_encountered == 0) {
                         num_discarded_begin++;
                         frame_duration_buffer = av_realloc(frame_duration_buffer,
                                                            num_discarded_begin * sizeof(int64_t));
@@ -3605,7 +3605,7 @@  static void mov_fix_index(MOVContext *mov, AVStream *st)
                     edit_list_start_encountered = 1;
                     // Make timestamps strictly monotonically increasing for audio, by rewriting timestamps for
                     // discarded packets.
-                    if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && frame_duration_buffer) {
+                    if (frame_duration_buffer) {
                         fix_index_entry_timestamps(st, st->nb_index_entries, edit_list_dts_counter,
                                                    frame_duration_buffer, num_discarded_begin);
                         av_freep(&frame_duration_buffer);