<!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>Let me know and I adjust this patch to the way you imply, I have some opinions on the matter, but the most important thing is fixing <code>GotoAndPlay</code> and related, so I am very flexible.</p>
<p>Also, thanks for the very detailed review!</p>
<p>On 2018-07-16 12:34, Steve Lhomme wrote:</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> + if( !( i_flags & MKV_DEMUX_SEEK_TRIGGERED_EXTERNALLY ) )
+ i_flags |= MKV_DEMUX_SEEK_PRECISE;
+</code></pre>
</blockquote>
<pre><code> Are we sure about that ? We could decide to use fast-seeking internally as well.</code></pre>
</blockquote>
<p>Under what circumstances would we want to enable fast-seeking internally? I thought about it, but it could cause a number of issues due to it being inexact.</p>
<p>Just consider <code>GotoAndPlay</code> as an example, if there are such commands in the file we could potentially end up in a state where we keep jumping back and forth, and playback would break completely. Same thing could happen for the user, but that is upon request, I don’t think we want to end up in such case implicitly while following the spec.</p>
<p>Though, I could remove the <code>if</code>, maybe there is a case I have not thought about where it would actually make sense.</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> + if( !( i_flags & MKV_DEMUX_SEEK_TRIGGERED_EXTERNALLY ) )
+ es_out_Control( sys.demuxer.out, ES_OUT_RESET_PCR );
+</code></pre>
</blockquote>
<pre><code> Is this replacing the call from UpdateCurrentToChapter() ? Do we want to reset the PCR
when switching to the next hard-linked segment ? (ie basically the exact continuation of
the previous segment). It *is* an internal seek. Will it cause buffer flushing which is
definitely not wanted here.</code></pre>
</blockquote>
<p>I compared the playback experience from current <code>master</code> and could not notice any negative changes, tried several files, and from various mediums (to test IO related things).</p>
<p>The one thing I did find was that if it takes a while to open the new segment (without PCR reset), we would end up in a state where blocks would be coming in late (from the core/decoder’s perspective), and we would have issues with either drifting or drops.</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> p_current_vchapter = p_vchapter;
- /* only use for soft linking, hard linking should be continous */</code></pre>
</blockquote>
<pre><code> I think this is this explanation on why we only do the RESET_PCR in some case when we
seek internally but not others.</code></pre>
</blockquote>
<p>I started working on making things unconditionally smoother, but those patches are currently in super-draft mode. If you want me to patch the patch and retain the old behavior for this specific case, I’d be happy to do that - I want to fix the issue in a more robust way anyhow.</p>
<p>I also originally had that we would only reset the PCR if we are seeking backwards, as forward would be handled but setting the next display time, but I felt it was too hackish.</p>
</body>
</html>