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

[Filip Roséen]

Hi again,

I just now realized that the below commit message somewhat
misleadingly uses the term *"decoder"* in the below quoted section.

On 2018-07-16 05:19, Filip Roséen wrote:

> 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 decoder
> would think the outgoing blocks were coming in too early and employ
> counter meassures to prevent unwanted drift.

It is between the actual *decoder* and the destined *audio output*
this drift prevention is taking place, more specifically in
`aout_DecSynchronize` invoked from `aout_Play`.

Attached to this message is an updated commit that instead uses *"the
core*" to more accurately describe where this drift prevention is
taking place, without going into extensive detail.

Sorry for the inconvience.

Best Regards,\
Filip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180716/4c843bef/attachment.html>
-------------- next part --------------
>From e619e15c0f1f4b1efe4d4a2805e2b14860592aea 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 set the VLC block's DTS to
VLC_TICK_INVALID for matroska-frames within a matroska block, which in
turn would confuse the decoder as it would then 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..23c1b39776 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_timecode, 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_timecode = i_timecode - track.i_codec_delay;
+    vlc_tick_t i_dts = i_track_timecode;
+    vlc_tick_t i_pts = i_track_timecode;
+
     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