<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="Content-Style-Type" content="text/css" />
<meta name="generator" content="pandoc" />
<title></title>
<style type="text/css">
code{white-space: pre-wrap;}
span.smallcaps{font-variant: small-caps;}
span.underline{text-decoration: underline;}
div.column{display: inline-block; vertical-align: top; width: 50%;}
</style>
</head>
<body>
<p>Hi Steve,</p>
<p>On 2018-07-16 12:12, Steve Lhomme wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> On 2018-07-16 5:19, Filip Roséen wrote:</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> The previous implementation would set the VLC block's DTS to
VLC_TICK_INVALID for matroska-frames within a matroska block, which in</code></pre>
</blockquote>
<pre><code> I don't see that in the code. I only see the DTS set to Invalid for Cook/Atrac3. The
rest of the time it's pts or last_dts + default_duration.</code></pre>
</blockquote>
<ul>
<li>https://git.videolan.org/?p=vlc.git;a=blob;f=modules/demux/mkv/mkv.cpp;h=c259ea630fb1c9ef256c1443f5e538d14cf08970;hb=HEAD#l666</li>
</ul>
<p>It is funny that it is on line <code>666</code>, in either case; <code>i_pts</code> is most often set to <code>VLC_TICK_INVALID</code> after the first frame, effectively making every other frame have <code>VLC_TICK_INVALID</code> as <code>i_dts</code>.</p>
<p>There are exceptions, such as if we have <em>default-duration</em>, or if the blocks are not <em>packetized</em>; maybe I should have mentioned that in the commit message now when you pointed it out - but I really consider those to be exceptions and not the main case.</p>
<p>Do you want me to rewrite the commit message?</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> 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 decoder
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
_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
https://mailman.videolan.org/listinfo/vlc-devel</code></pre>
</blockquote>
<pre><code> _______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
https://mailman.videolan.org/listinfo/vlc-devel</code></pre>
</blockquote>
</body>
</html>