[vlc-devel] [PATCH] Preliminary dual subtitles support.
Thomas Guillem
thomas at gllm.fr
Wed Apr 24 10:17:27 CEST 2019
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.
>
> 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
More information about the vlc-devel
mailing list