[vlc-devel] [PATCH 4/9] demux: mkv: fix DTS of outgoing blocks

[Filip Roséen]

Attached to this email is an updated commit with a more explicit
commit message. Let me know if you'd like it to be written
differently, because perhaps it is still not clear enough (it is a bit
messy).

On 2018-07-16 13:04, Filip Roséen wrote:

> Do you want me to rewrite the commit message?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180716/22e5ffa8/attachment.html>
-------------- next part --------------
>From 1a2c1ae9edaf53079e85c4a523c0da3227c6d759 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Filip=20Ros=C3=A9en?= <filip at atch.se>
Date: Sun, 15 Jul 2018 01:40:36 +0200
Subject: [PATCH] demux: mkv: fix DTS of outgoing blocks

The previous implementation would most often set the VLC block's DTS
to VLC_TICK_INVALID for matroska-frames within a matroska block
beloning to an audio track (besides for the first frame), which in
turn could then confuse the decoder as it would have to assume that
the block in question belonged to the most recently set PCR.

This works fine in cases where the PCR is updated often enough to
almost match the track's frames, but in cases where another track is
preventing the PCR from updating frequently enough (video track with
long blocks of many matroska-frames), the PCR would be too far in the
past to base the outgoing block's DTS on. As a result, the core would
think the outgoing blocks were coming in too early and employ counter
meassures to prevent unwanted drift.

By setting the DTS to the matroska-block's timestamp, we prevent the
above from happening, resulting in smoother playback.

Included in this patch is a rename of the former i_pts parameter to
BlockDecode: "i_timecode" should make it more clear that we are
dealing with the block's timestamp, which might or might not be the
same as the outgoing frames' pts.

fixes: #13757
---
 modules/demux/mkv/mkv.cpp | 13 +++++++++----
 modules/demux/mkv/mkv.hpp |  2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/modules/demux/mkv/mkv.cpp b/modules/demux/mkv/mkv.cpp
index 220a061e57..304f99dd8c 100644
--- a/modules/demux/mkv/mkv.cpp
+++ b/modules/demux/mkv/mkv.cpp
@@ -511,7 +511,7 @@ static int Seek( demux_t *p_demux, vlc_tick_t i_mk_date, double f_percent, virtu
 
 /* Needed by matroska_segment::Seek() and Seek */
 void BlockDecode( demux_t *p_demux, KaxBlock *block, KaxSimpleBlock *simpleblock,
-                  vlc_tick_t i_pts, vlc_tick_t i_duration, bool b_key_picture,
+                  vlc_tick_t i_timestamp, vlc_tick_t i_duration, bool b_key_picture,
                   bool b_discardable_picture )
 {
     demux_sys_t *p_sys = (demux_sys_t *)p_demux->p_sys;
@@ -538,8 +538,6 @@ void BlockDecode( demux_t *p_demux, KaxBlock *block, KaxSimpleBlock *simpleblock
         return;
     }
 
-    i_pts -= track.i_codec_delay;
-
     if ( track.fmt.i_cat != DATA_ES )
     {
         bool b;
@@ -557,6 +555,10 @@ void BlockDecode( demux_t *p_demux, KaxBlock *block, KaxSimpleBlock *simpleblock
     size_t block_size = internal_block.GetSize();
     const unsigned i_number_frames = internal_block.NumberFrames();
 
+    vlc_tick_t i_track_timestamp = i_timestamp - track.i_codec_delay;
+    vlc_tick_t i_dts = i_track_timestamp;
+    vlc_tick_t i_pts = i_track_timestamp;
+
     for( unsigned int i_frame = 0; i_frame < i_number_frames; i_frame++ )
     {
         block_t *p_block;
@@ -639,6 +641,9 @@ void BlockDecode( demux_t *p_demux, KaxBlock *block, KaxSimpleBlock *simpleblock
             break;
         }
 
+        p_block->i_dts = i_dts;
+        p_block->i_pts = i_pts;
+
         if( track.fmt.i_cat != VIDEO_ES )
         {
             if ( track.fmt.i_cat == DATA_ES )
@@ -648,7 +653,7 @@ void BlockDecode( demux_t *p_demux, KaxBlock *block, KaxSimpleBlock *simpleblock
                 block_Release( p_block );
                 return;
             }
-            p_block->i_dts = p_block->i_pts = i_pts;
+            p_block->i_pts = i_pts;
         }
         else
         {
diff --git a/modules/demux/mkv/mkv.hpp b/modules/demux/mkv/mkv.hpp
index 3e0ae211b9..17a3fd0e67 100644
--- a/modules/demux/mkv/mkv.hpp
+++ b/modules/demux/mkv/mkv.hpp
@@ -120,7 +120,7 @@ enum
 using namespace LIBMATROSKA_NAMESPACE;
 
 void BlockDecode( demux_t *p_demux, KaxBlock *block, KaxSimpleBlock *simpleblock,
-                  vlc_tick_t i_pts, vlc_tick_t i_duration, bool b_key_picture,
+                  vlc_tick_t i_timecode, vlc_tick_t i_duration, bool b_key_picture,
                   bool b_discardable_picture );
 
 class attachment_c
-- 
2.18.0