[vlc-devel] [RFC PATCH] demux/mkv: do not expose hidden chapters

[Filip Roséen]

Hi Denis,

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.

For future reference, this wanted changes in regards of the original patch
can be found here:

  - https://mailman.videolan.org/pipermail/vlc-devel/2016-July/108737.html

On 16/07/25 11:14, Denis Charmet wrote:

>      On 2016-07-24 00:56, Filip Roséen wrote:
> >     A few questions:
> >
> >         - What was the original purpose/difference between;
> >             - chapter_item_c::b_user_display, and;
> 
>      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
> 
> >             - chapter_item_c::b_display_seekpoint,
> 
>      It means that the chapter isn't hidden (or that there is a chapter_command). I cannot
>      tell for chapter commands...
> 
> >           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 )
> 
>      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.
> 
> >     +        {
> >     +            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");
> 
>      remove that it will never happen
> 
> >
> >     -        /* 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;
> 
>      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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160803/bd05eeea/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-demux-mkv-do-not-expose-hidden-chapters.patch
Type: text/x-diff
Size: 3093 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160803/bd05eeea/attachment.patch>