[vlc-devel] [PATCH] Preliminary dual subtitles support.
Thomas Guillem
thomas at gllm.fr
Wed Apr 24 11:51:33 CEST 2019
On Wed, Apr 24, 2019, at 10:17, Thomas Guillem wrote:
> Hello,
>
> Thanks a lot for working on this really wanted feature.
>
> On Tue, Apr 23, 2019, at 12:43, Roland Bewick wrote:
> > Secondary subtitle track will be displayed at the top of the video.
> > Screenshot: http://prntscr.com/nfok5k
> >
> > Usage:
> > - Select two subtitle tracks.
> > - If two subtitle tracks are already selected and a third is selected,
> > the previous secondary subtitle track will be unselected.
> > - To change the primary sub track,
> > make sure to deselect the secondary one first.
>
> This feature should be disabled by default. I think most people will
> expect to unselect the previous subtitle when selecting a new one. You
> should add a a new vlc option (cf. src/libvlc-module.c) to enable it.
OK, maybe not. The UI/player could take care of unselecting the previous SPU if we want only one SPU at a time.
>
> >
> > Limitations:
> > - Only SRT and USF subtitles are supported.
> > I can update the other codecs if my chosen solution is accepted.
>
> Solution looks OK to me, but I'm no spu export. Francois ?
>
> > - Primary and secondary subtitles share track synchronization.
> > This must be fixed.
>
> Why not having one clock per spu ?
>
> > - Hotkeys haven't been done yet.
> > A single hotkey could be added to switch state between
> > manipulating the primary or secondary subtitle track.
> > - Legacy / VLC < 4.0 QT GUI isn't supported.
> > I can add support for this so that people can still use their skins.
> > - Primary and secondary subtitles share the same style (font size, etc).
> > - Remote control is not supported.
> >
> > Untested on MAC interface.
>
> It should work on every OS.
>
> > ---
> > modules/codec/subsdec.c | 4 +-
> > modules/codec/subsusf.c | 5 +-
> > src/input/es_out.c | 84 +++++++++++++++++++++++++++--
> > src/video_output/video_output.c | 12 +++--
> > src/video_output/vout_subpictures.c | 2 +-
> > 5 files changed, 93 insertions(+), 14 deletions(-)
> >
> > diff --git a/modules/codec/subsdec.c b/modules/codec/subsdec.c
> > index 0016332906..a733dffce2 100644
> > --- a/modules/codec/subsdec.c
> > +++ b/modules/codec/subsdec.c
> > @@ -464,9 +464,9 @@ static subpicture_t *ParseText( decoder_t *p_dec,
> > block_t *p_block )
> > int i_inline_align = -1;
> > p_spu_sys->region.p_segments = ParseSubtitles( &i_inline_align,
> > psz_subtitle );
> > free( psz_subtitle );
> > - if( p_sys->i_align >= 0 ) /* bottom ; left, right or centered */
> > + if( p_sys->i_align >= 0 ) /* user-set alignment */
> > {
> > - p_spu_sys->region.align = SUBPICTURE_ALIGN_BOTTOM |
> > p_sys->i_align;
> > + p_spu_sys->region.align = p_sys->i_align;
> > p_spu_sys->region.inner_align = p_sys->i_align;
> > }
> > else if( i_inline_align >= 0 )
> > diff --git a/modules/codec/subsusf.c b/modules/codec/subsusf.c
> > index c02c52b52f..faef159576 100644
> > --- a/modules/codec/subsusf.c
> > +++ b/modules/codec/subsusf.c
> > @@ -436,12 +436,11 @@ static subpicture_region_t *CreateTextRegion(
> > decoder_t *p_dec,
> > }
> > }
> >
> > - /* Set default or user align/magin.
> > + /* Set default or user align/margin.
> > * Style overriden if no user value. */
> > p_text_region->i_x = i_sys_align > 0 ? 20 : 0;
> > p_text_region->i_y = 10;
> > - p_text_region->i_align = SUBPICTURE_ALIGN_BOTTOM |
> > - ((i_sys_align > 0) ? i_sys_align : 0);
> > + p_text_region->i_align = i_sys_align >= 0 ? i_sys_align :
> > SUBPICTURE_ALIGN_BOTTOM;
> >
> > if( p_ssa_style )
> > {
> > diff --git a/src/input/es_out.c b/src/input/es_out.c
> > index 3352602849..835975fa1f 100644
> > --- a/src/input/es_out.c
> > +++ b/src/input/es_out.c
> > @@ -142,6 +142,7 @@ typedef struct
> > {
> > int i_count; /* es count */
> > es_out_id_t *p_main_es; /* current main es */
> > + es_out_id_t *p_secondary_es; /* current secondary es (SPU_ES only) */
>
> Do you really need this variable ? We can get multiple VIDEO ES
> selected without having a reference to the "secondary" one.
>
> > enum es_out_policy_e e_policy;
> >
> > /* Parameters used for es selection */
> > @@ -300,6 +301,7 @@ static void EsOutPropsInit( es_out_es_props_t
> > *p_props,
> > p_props->i_channel = (psz_trackvar) ? var_GetInteger( p_input,
> > psz_trackvar ): -1;
> > p_props->i_demux_id = -1;
> > p_props->p_main_es = NULL;
> > + p_props->p_secondary_es = NULL;
> >
> > if( !input_priv(p_input)->b_preparsing && psz_langvar )
> > {
> > @@ -343,7 +345,7 @@ es_out_t *input_EsOutNew( input_thread_t *p_input,
> > float rate )
> > "video-track-id", "video-track", NULL, NULL );
> > EsOutPropsInit( &p_sys->audio, true, p_input,
> > ES_OUT_ES_POLICY_EXCLUSIVE,
> > "audio-track-id", "audio-track", "audio-language",
> > "audio" );
> > - EsOutPropsInit( &p_sys->sub, false, p_input,
> > ES_OUT_ES_POLICY_EXCLUSIVE,
> > + EsOutPropsInit( &p_sys->sub, false, p_input,
> > ES_OUT_ES_POLICY_SIMULTANEOUS,
> > "sub-track-id", "sub-track", "sub-language", "sub"
>
> So, the ES_OUT_ES_POLICY_SIMULTANEOUS should depend on a VLC option.
>
> > );
> >
> > p_sys->i_group_id = var_GetInteger( p_input, "program" );
> > @@ -1083,6 +1085,7 @@ static void EsOutProgramSelect( es_out_t *out,
> > es_out_pgrm_t *p_pgrm )
> > p_sys->audio.p_main_es = NULL;
> > p_sys->video.p_main_es = NULL;
> > p_sys->sub.p_main_es = NULL;
> > + p_sys->sub.p_secondary_es = NULL;
> > }
> >
> > msg_Dbg( p_input, "selecting program id=%d", p_pgrm->i_id );
> > @@ -1805,6 +1808,46 @@ static bool EsIsSelected( es_out_id_t *es )
> > return es->p_dec != NULL;
> > }
> > }
> > +
> > +/**
> > + Set the alignment used by the chosen subtitle codec.
> > + This solution works for subsdec and subsusf formats. (SRT + MKV
> > Embedded).
> > +
> > + TODO: It would be nice to have a unified way of setting the
> > subtitle alignment.
> > + The other codecs could be updated to follow subsusf's example.
> > +*/
> > +static void EsOutSetDecoderSubtitleAlignment( es_out_t *out,
> > es_out_id_t *p_es )
> > +{
> > + es_out_sys_t *p_sys = container_of(out, es_out_sys_t, out);
> > + es_out_es_props_t *p_esprops = GetPropsByCat( p_sys,
> > p_es->fmt.i_cat );
> > + input_thread_t *p_input = p_sys->p_input;
> > +
> > + if( p_es->fmt.i_cat == SPU_ES )
> > + {
> > + int i_subtitle_alignment = SUBPICTURE_ALIGN_BOTTOM;
> > + if( p_esprops->p_main_es != NULL && p_es !=
> > p_esprops->p_main_es )
> > + {
> > + /* Show secondary sub track on the top of the video */
> > + i_subtitle_alignment = SUBPICTURE_ALIGN_TOP;
> > + }
> > + /* Temporarily create this variable as only the configuration
> > exists */
> > + var_Create( p_input, "subsdec-align", VLC_VAR_INTEGER);
> > + var_SetInteger( p_input, "subsdec-align",
> > i_subtitle_alignment);
> > + }
> > +}
> > +
> > +/* we don't need these alignment variables anymore (already read by
> > the codec) */
> > +static void EsOutReleaseDecoderAlignmentVariables( es_out_t *out,
> > es_out_id_t *p_es )
> > +{
> > + es_out_sys_t *p_sys = container_of(out, es_out_sys_t, out);
> > + input_thread_t *p_input = p_sys->p_input;
> > +
> > + if( p_es->fmt.i_cat == SPU_ES )
> > + {
> > + var_Destroy( p_input, "subsdec-align" );
> > + }
> > +}
> > +
> > static void EsOutCreateDecoder( es_out_t *out, es_out_id_t *p_es )
> > {
> > es_out_sys_t *p_sys = container_of(out, es_out_sys_t, out);
> > @@ -1821,8 +1864,13 @@ static void EsOutCreateDecoder( es_out_t *out,
> > es_out_id_t *p_es )
> > if( !p_es->p_clock )
> > return;
> >
> > + EsOutSetDecoderSubtitleAlignment( out, p_es );
> > +
> > dec = input_DecoderNew( p_input, &p_es->fmt, p_es->p_clock,
> > input_priv(p_input)->p_sout );
> > +
> > + EsOutReleaseDecoderAlignmentVariables( out, p_es );
> > +
> > if( dec != NULL )
> > {
> > input_DecoderChangeRate( dec, p_sys->rate );
> > @@ -1998,6 +2046,7 @@ static void EsDeleteCCChannels( es_out_t *out,
> > es_out_id_t *parent )
> > static void EsOutUnselectEs( es_out_t *out, es_out_id_t *es, bool
> > b_update )
> > {
> > es_out_sys_t *p_sys = container_of(out, es_out_sys_t, out);
> > + es_out_es_props_t *p_esprops = GetPropsByCat( p_sys, es->fmt.i_cat
> > );
> > input_thread_t *p_input = p_sys->p_input;
> >
> > if( !EsIsSelected( es ) )
> > @@ -2006,6 +2055,15 @@ static void EsOutUnselectEs( es_out_t *out,
> > es_out_id_t *es, bool b_update )
> > return;
> > }
> >
> > + if( es == p_esprops->p_main_es && es->fmt.i_cat == SPU_ES )
> > + {
> > + p_esprops->p_main_es = NULL;
> > + }
> > + else if( es == p_esprops->p_secondary_es )
> > + {
> > + p_esprops->p_secondary_es = NULL;
> > + }
> > +
> > if( es->p_master )
> > {
> > if( es->p_master->p_dec )
> > @@ -2035,7 +2093,7 @@ static void EsOutUnselectEs( es_out_t *out,
> > es_out_id_t *es, bool b_update )
> > *
> > * \param out The es_out structure
> > * \param es es_out_id structure
> > - * \param b_force ...
> > + * \param b_force force the selection of the ES even if it is lower
> > priority
> > * \return nothing
> > */
> > static void EsOutSelect( es_out_t *out, es_out_id_t *es, bool b_force )
> > @@ -2049,8 +2107,11 @@ static void EsOutSelect( es_out_t *out,
> > es_out_id_t *es, bool b_force )
> > return;
> > }
> >
> > + /* Only auto-unselect SPU ES when looking for a higher priority ES
> > */
> > + bool b_auto_unselect_spu = es->fmt.i_cat == SPU_ES && !b_force;
> > +
> > bool b_auto_unselect = p_esprops && p_sys->i_mode ==
> > ES_OUT_MODE_AUTO &&
> > - p_esprops->e_policy ==
> > ES_OUT_ES_POLICY_EXCLUSIVE &&
> > + ( p_esprops->e_policy ==
> > ES_OUT_ES_POLICY_EXCLUSIVE || b_auto_unselect_spu ) &&
> > p_esprops->p_main_es &&
> > p_esprops->p_main_es != es;
> >
> > if( p_sys->i_mode == ES_OUT_MODE_ALL || b_force )
> > @@ -2059,6 +2120,11 @@ static void EsOutSelect( es_out_t *out,
> > es_out_id_t *es, bool b_force )
> > {
> > if( b_auto_unselect )
> > EsOutUnselectEs( out, p_esprops->p_main_es, true );
> > + else if( b_force && p_esprops->p_secondary_es &&
> > p_esprops->p_secondary_es != es )
> > + {
> > + /* Auto unselect secondary subtrack */
> > + EsOutUnselectEs( out, p_esprops->p_secondary_es, true
> > );
> > + }
> >
> > EsOutSelectEs( out, es );
> > }
> > @@ -2172,7 +2238,17 @@ static void EsOutSelect( es_out_t *out,
> > es_out_id_t *es, bool b_force )
> >
> > /* FIXME TODO handle priority here */
> > if( p_esprops && p_sys->i_mode == ES_OUT_MODE_AUTO && EsIsSelected( es ) )
> > - p_esprops->p_main_es = es;
> > + {
> > + if( p_esprops->p_main_es == NULL || es->fmt.i_cat != SPU_ES )
> > + {
> > + p_esprops->p_main_es = es;
> > + }
> > + else
> > + {
> > + /* Only set a secondary SPU if we already have a primary one */
> > + p_esprops->p_secondary_es = es;
> > + }
> > + }
> > }
> >
> > static void EsOutCreateCCChannels( es_out_t *out, vlc_fourcc_t codec,
> > uint64_t i_bitmap,
> > diff --git a/src/video_output/video_output.c
> > b/src/video_output/video_output.c
> > index 9604522825..df5ff3d327 100644
> > --- a/src/video_output/video_output.c
> > +++ b/src/video_output/video_output.c
> > @@ -271,10 +271,14 @@ int vout_RegisterSubpictureChannel( vout_thread_t
> > *vout )
> >
> > void vout_SetSubpictureClock( vout_thread_t *vout, vlc_clock_t *clock )
> > {
> > - vlc_mutex_lock(&vout->p->spu_lock);
> > - if (vout->p->spu)
> > - spu_clock_Set(vout->p->spu, clock);
> > - vlc_mutex_unlock(&vout->p->spu_lock);
> > + /* Currently all subtitle tracks share the same clock, so it must
> > be valid. */
> > + if ( clock != NULL )
> > + {
> > + vlc_mutex_lock(&vout->p->spu_lock);
> > + if (vout->p->spu)
> > + spu_clock_Set(vout->p->spu, clock);
> > + vlc_mutex_unlock(&vout->p->spu_lock);
>
> Why ? Is the secondary SPU also created via EsOutCreateDecoder(), in
> that case, it should have its own clock, no ?
>
> > + }
> > }
> >
> > void vout_FlushSubpictureChannel( vout_thread_t *vout, int channel )
> > diff --git a/src/video_output/vout_subpictures.c
> > b/src/video_output/vout_subpictures.c
> > index 6b106534d3..8b530d3c2c 100644
> > --- a/src/video_output/vout_subpictures.c
> > +++ b/src/video_output/vout_subpictures.c
> > @@ -476,7 +476,7 @@ static void SpuRegionPlace(int *x, int *y,
> > *y = region->i_y;
> > } else {
> > if (region->i_align & SUBPICTURE_ALIGN_TOP)
> > - *y = region->i_y;
> > + *y = region->i_y + ( subpic->b_subtitle ?
> > region->fmt.i_visible_height : 0 );
> > else if (region->i_align & SUBPICTURE_ALIGN_BOTTOM)
> > *y = subpic->i_original_picture_height -
> > region->fmt.i_visible_height - region->i_y;
> > else
> > --
> > 2.17.1
>
> Regards,
>
> >
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list