<p>Hi Denis,</p>
<p>See the attached patch for a modification of the original patch submitted that hopefully is in line with what you were both expecting and wanted.</p>
<p>For future reference, this wanted changes in regards of the original patch can be found here:</p>
<ul>
<li>https://mailman.videolan.org/pipermail/vlc-devel/2016-July/108737.html</li>
</ul>
<p>On 16/07/25 11:14, Denis Charmet wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> On 2016-07-24 00:56, 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>A few questions:

    - What was the original purpose/difference between;
        - chapter_item_c::b_user_display, and;</code></pre>
</blockquote>
<pre><code> It means that the chapter has at least one Chapter Display string so we have actually
 something to display as a name in the seekpoint</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code>        - chapter_item_c::b_display_seekpoint,</code></pre>
</blockquote>
<pre><code> It means that the chapter isn't hidden (or that there is a chapter_command). I cannot
 tell for chapter commands...</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code>      other than that one defaults to false, and the other to true?

    - Can this patch be pushed as is (prior to a clean-up of relevant
      sections around it)?

Thank you!
---
 modules/demux/mkv/virtual_segment.cpp | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/modules/demux/mkv/virtual_segment.cpp
b/modules/demux/mkv/virtual_segment.cpp
index 5d30d7e..2bce464 100644
--- a/modules/demux/mkv/virtual_segment.cpp
+++ b/modules/demux/mkv/virtual_segment.cpp
@@ -592,22 +592,24 @@ int virtual_chapter_c::PublishChapters(
input_title_t & title, int & i_user_chap
          ( ( sub_vchapters.size() > 0 && i_mk_virtual_start_time !=
sub_vchapters[0]->i_mk_virtual_start_time) ||
            sub_vchapters.size() == 0 ) ) || !p_chapter )
     {
-        seekpoint_t *sk = vlc_seekpoint_New();
+        if( p_chapter && p_chapter->b_user_display )</code></pre>
</blockquote>
<pre><code> While I don't mind the fact that you don't publish dummy chapters (and yes I should have
 removed that a long time ago) I'd rather have you remove the "|| !p_chapter" above.

 Besides since I don't know how chapter commands work this might break chapters which
 have a codecName infered from chapter commands but no Chapter display string.</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code>+        {
+            seekpoint_t *sk = vlc_seekpoint_New();

-        sk->i_time_offset = i_mk_virtual_start_time;
-        if( p_chapter )
-            sk->psz_name = strdup( p_chapter->psz_name.c_str() );
-        else
-            sk->psz_name = strdup("dummy chapter");
+            sk->i_time_offset = i_mk_virtual_start_time;
+            if( p_chapter )
+                sk->psz_name = strdup( p_chapter->psz_name.c_str() );
+            else
+                sk->psz_name = strdup("dummy chapter");</code></pre>
</blockquote>
<pre><code> remove that it will never happen</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code>-        /* A start time of '0' is ok. A missing ChapterTime element
is ok, too, because '0' is its default value. */
-        title.i_seekpoint++;
-        title.seekpoint = (seekpoint_t**)xrealloc( title.seekpoint,
-                                 title.i_seekpoint * sizeof( seekpoint_t* ) );
-        title.seekpoint[title.i_seekpoint-1] = sk;
+            /* A start time of '0' is ok. A missing ChapterTime
element is ok, too, because '0' is its default value. */
+            title.i_seekpoint++;
+            title.seekpoint = (seekpoint_t**)xrealloc( title.seekpoint,
+              title.i_seekpoint * sizeof( seekpoint_t* ) );
+            title.seekpoint[title.i_seekpoint-1] = sk;

-        if ( (p_chapter && p_chapter->b_user_display ) ||  !p_chapter )
             i_user_chapters++;
+        }
     }
     i_seekpoint_num = i_user_chapters;</code></pre>
</blockquote>
<pre><code> Regards,
 -- 
 Denis Charmet - TypX
 Le mauvais esprit est un art de vivre
 _______________________________________________
 vlc-devel mailing list
 To unsubscribe or modify your subscription options:
 https://mailman.videolan.org/listinfo/vlc-devel</code></pre>
</blockquote>