[vlc-devel] [PATCH 2/2] vout: spu: prerender text
Thomas Guillem
thomas at gllm.fr
Fri Jul 19 07:54:09 CEST 2019
Hello, what about my other remarks:
- "You should move it in a seperate commit."
- "Rename to spu_prerender_Sync/Cancel for consistency with others functions of this file."
And about my signalling remark:
"Why are you signalling here ? The prerender thread is only waiting for sys->prerender_vector.size != 0, and not for prerender_chroma_list."
Your Enqueue() function push into the vector but doesn't signal. Then your Wake() function set the chroma and signal.
This seems problematic to me. What about spurius wake that could happen between Enqueue() and Wake() ?
You should make sure that the thread is waiting for a valid vector size and a valid chroma too.
On Thu, Jul 18, 2019, at 21:18, Francois Cartegnie wrote:
> ---
> src/video_output/vout_subpictures.c | 340 ++++++++++++++++++++++------
> 1 file changed, 266 insertions(+), 74 deletions(-)
>
> diff --git a/src/video_output/vout_subpictures.c
> b/src/video_output/vout_subpictures.c
> index ec1694a021..d93d232406 100644
> --- a/src/video_output/vout_subpictures.c
> +++ b/src/video_output/vout_subpictures.c
> @@ -69,6 +69,7 @@ struct spu_channel {
> };
>
> typedef struct VLC_VECTOR(struct spu_channel) spu_channel_vector;
> +typedef struct VLC_VECTOR(subpicture_t *) spu_prerender_vector;
>
> struct spu_private_t {
> vlc_mutex_t lock; /* lock to protect all followings fields */
> @@ -106,12 +107,28 @@ struct spu_private_t {
> char *filter_chain_update;
> vlc_mutex_t filter_chain_lock;
> filter_chain_t *filter_chain;
> + /**/
> + struct
> + {
> + vlc_thread_t thread;
> + vlc_mutex_t lock;
> + vlc_cond_t cond;
> + vlc_cond_t output_cond;
> + spu_prerender_vector vector;
> + subpicture_t *p_processed;
> + video_format_t fmtsrc;
> + video_format_t fmtdst;
> + const vlc_fourcc_t *chroma_list;
> + } prerender;
>
> /* */
> vlc_tick_t last_sort_date;
> vout_thread_t *vout;
> };
>
> +static void spu_PrerenderSync(spu_private_t *, const subpicture_t *);
> +static void spu_PrerenderCancel(spu_private_t *, const subpicture_t *);
> +
> static void spu_channel_Init(struct spu_channel *channel, size_t id,
> enum vlc_vout_order order, vlc_clock_t
> *clock)
> {
> @@ -144,11 +161,12 @@ static void spu_channel_DeleteAt(struct
> spu_channel *channel, size_t index)
> vlc_vector_remove(&channel->entries, index);
> }
>
> -static void spu_channel_Clean(struct spu_channel *channel)
> +static void spu_channel_Clean(spu_private_t *sys, struct spu_channel *channel)
> {
> for (size_t i = 0; i < channel->entries.size; i++)
> {
> assert(channel->entries.data[i].subpic);
> + spu_PrerenderCancel(sys, channel->entries.data[i].subpic);
> subpicture_Delete(channel->entries.data[i].subpic);
> }
> vlc_vector_destroy(&channel->entries);
> @@ -301,6 +319,16 @@ static void SpuRenderText(spu_t *spu,
>
> assert(region->fmt.i_chroma == VLC_CODEC_TEXT);
>
> + //assume rendered text is in sRGB if nothing is set
> + if (region->fmt.transfer == TRANSFER_FUNC_UNDEF)
> + region->fmt.transfer = TRANSFER_FUNC_SRGB;
> + if (region->fmt.primaries == COLOR_PRIMARIES_UNDEF)
> + region->fmt.primaries = COLOR_PRIMARIES_SRGB;
> + if (region->fmt.space == COLOR_SPACE_UNDEF)
> + region->fmt.space = COLOR_SPACE_SRGB;
> + if (region->fmt.color_range == COLOR_RANGE_UNDEF)
> + region->fmt.color_range = COLOR_RANGE_FULL;
> +
> if ( region->p_text )
> text->pf_render(text, region, region, chroma_list);
> }
> @@ -719,6 +747,7 @@ spu_SelectSubpictures(spu_t *spu, vlc_tick_t system_now,
>
> if (is_rejeted)
> {
> + spu_PrerenderCancel(sys, current);
> subpicture_Delete(current);
> vlc_vector_remove(&channel->entries, index);
> }
> @@ -764,15 +793,6 @@ static void SpuRenderRegion(spu_t *spu,
>
> /* Render text region */
> if (region->fmt.i_chroma == VLC_CODEC_TEXT) {
> - // assume rendered text is in sRGB if nothing is set
> - if (region->fmt.transfer == TRANSFER_FUNC_UNDEF)
> - region->fmt.transfer = TRANSFER_FUNC_SRGB;
> - if (region->fmt.primaries == COLOR_PRIMARIES_UNDEF)
> - region->fmt.primaries = COLOR_PRIMARIES_SRGB;
> - if (region->fmt.space == COLOR_SPACE_UNDEF)
> - region->fmt.space = COLOR_SPACE_SRGB;
> - if (region->fmt.color_range == COLOR_RANGE_UNDEF)
> - region->fmt.color_range = COLOR_RANGE_FULL;
>
> SpuRenderText(spu, region, chroma_list);
>
> @@ -1373,10 +1393,200 @@ static int SubSourceDelProxyCallbacks(filter_t
> *filter, void *opaque)
> return VLC_SUCCESS;
> }
>
> +static void spu_PrerenderWake(spu_private_t *sys,
> + const video_format_t *fmt_dst,
> + const video_format_t *fmt_src,
> + const vlc_fourcc_t *chroma_list)
> +{
> + vlc_mutex_lock(&sys->prerender.lock);
> + if(!video_format_IsSimilar(fmt_dst, &sys->prerender.fmtdst))
> + {
> + video_format_Clean(&sys->prerender.fmtdst);
> + video_format_Copy(&sys->prerender.fmtdst, fmt_dst);
> + }
> + if(!video_format_IsSimilar(fmt_src, &sys->prerender.fmtsrc))
> + {
> + video_format_Clean(&sys->prerender.fmtsrc);
> + video_format_Copy(&sys->prerender.fmtsrc, fmt_src);
> + }
> + sys->prerender.chroma_list = chroma_list;
> + vlc_cond_signal(&sys->prerender.cond);
> + vlc_mutex_unlock(&sys->prerender.lock);
> +}
> +
> +static void spu_PrerenderEnqueue(spu_private_t *sys, subpicture_t
> *p_subpic)
> +{
> + vlc_mutex_lock(&sys->prerender.lock);
> + vlc_vector_push(&sys->prerender.vector, p_subpic);
> + vlc_mutex_unlock(&sys->prerender.lock);
> +}
> +
> +static void spu_PrerenderCancel(spu_private_t *sys, const subpicture_t
> *p_subpic)
> +{
> + vlc_mutex_lock(&sys->prerender.lock);
> + ssize_t i_idx;
> + vlc_vector_index_of(&sys->prerender.vector, p_subpic, &i_idx);
> + if(i_idx >= 0)
> + vlc_vector_remove(&sys->prerender.vector, i_idx);
> + else while(sys->prerender.p_processed == p_subpic)
> + vlc_cond_wait(&sys->prerender.output_cond,
> &sys->prerender.lock);
> + vlc_mutex_unlock(&sys->prerender.lock);
> +}
> +
> +static void spu_PrerenderSync(spu_private_t *sys, const subpicture_t
> *p_subpic)
> +{
> + vlc_mutex_lock(&sys->prerender.lock);
> + ssize_t i_idx;
> + vlc_vector_index_of(&sys->prerender.vector, p_subpic, &i_idx);
> + while(i_idx >= 0 || sys->prerender.p_processed == p_subpic)
> + {
> + vlc_cond_wait(&sys->prerender.output_cond,
> &sys->prerender.lock);
> + vlc_vector_index_of(&sys->prerender.vector, p_subpic, &i_idx);
> + }
> + vlc_mutex_unlock(&sys->prerender.lock);
> +}
> +
> +static void spu_PrerenderText(spu_t *spu, subpicture_t *p_subpic)
> +{
> + spu_private_t *sys = spu->p;
> +
> + if (p_subpic->i_original_picture_width <= 0 ||
> + p_subpic->i_original_picture_height <= 0) {
> + if (p_subpic->i_original_picture_width > 0 ||
> + p_subpic->i_original_picture_height > 0)
> + msg_Err(spu, "original picture size %dx%d is unsupported",
> + p_subpic->i_original_picture_width,
> + p_subpic->i_original_picture_height);
> + else
> + msg_Warn(spu, "original picture size is undefined");
> +
> + p_subpic->i_original_picture_width =
> sys->prerender.fmtsrc.i_visible_width;
> + p_subpic->i_original_picture_height =
> sys->prerender.fmtsrc.i_visible_height;
> + }
> +
> + /* FIXME aspect ratio ? */
> + sys->text->fmt_out.video.i_width =
> + sys->text->fmt_out.video.i_visible_width =
> p_subpic->i_original_picture_width;
> +
> + sys->text->fmt_out.video.i_height =
> + sys->text->fmt_out.video.i_visible_height =
> p_subpic->i_original_picture_height;
> +
> + subpicture_Update(p_subpic,
> + &sys->prerender.fmtsrc, &sys->prerender.fmtdst,
> + p_subpic->b_subtitle ? p_subpic->i_start :
> vlc_tick_now());
> +
> + subpicture_region_t *region;
> + for (region = p_subpic->p_region; region != NULL; region =
> region->p_next)
> + {
> + if(region->fmt.i_chroma != VLC_CODEC_TEXT)
> + continue;
> + vlc_tick_t t = vlc_tick_now();
> + SpuRenderText(spu, region, sys->prerender.chroma_list);
> + }
> +}
> +
> +static void * spu_PrerenderThread(void *priv)
> +{
> + spu_t *spu = priv;
> + spu_private_t *sys = spu->p;
> +
> + vlc_mutex_lock(&sys->prerender.lock);
> + for( ;; )
> + {
> + mutex_cleanup_push(&sys->prerender.lock);
> + while(sys->prerender.vector.size == 0)
> + vlc_cond_wait(&sys->prerender.cond, &sys->prerender.lock);
> +
> + size_t i_idx = 0;
> + sys->prerender.p_processed = sys->prerender.vector.data[0];
> + for(size_t i=1; i<sys->prerender.vector.size; i++)
> + {
> + if(sys->prerender.p_processed->i_start >
> sys->prerender.vector.data[i]->i_start)
> + {
> + sys->prerender.p_processed =
> sys->prerender.vector.data[i];
> + i_idx = i;
> + }
> + }
> + vlc_vector_remove(&sys->prerender.vector, i_idx);
> + vlc_mutex_unlock(&sys->prerender.lock);
> + vlc_cleanup_pop();
> +
> + int canc = vlc_savecancel();
> + spu_PrerenderText(spu, sys->prerender.p_processed);
> + vlc_restorecancel(canc);
> +
> + vlc_mutex_lock(&sys->prerender.lock);
> + sys->prerender.p_processed = NULL;
> + vlc_cond_signal(&sys->prerender.output_cond);
> + }
> +
> + return NULL;
> +}
> +
>
> /*****************************************************************************
> * Public API
>
> *****************************************************************************/
>
> +static void spu_Cleanup(spu_t *spu)
> +{
> + spu_private_t *sys = spu->p;
> +
> + if (sys->text)
> + FilterRelease(sys->text);
> +
> + if (sys->scale_yuvp)
> + FilterRelease(sys->scale_yuvp);
> +
> + if (sys->scale)
> + FilterRelease(sys->scale);
> +
> + filter_chain_ForEach(sys->source_chain, SubSourceClean, spu);
> + if (sys->vout)
> + filter_chain_ForEach(sys->source_chain,
> + SubSourceDelProxyCallbacks, sys->vout);
> + filter_chain_Delete(sys->source_chain);
> + free(sys->source_chain_current);
> + if (sys->vout)
> + filter_chain_ForEach(sys->filter_chain,
> + SubFilterDelProxyCallbacks, sys->vout);
> + filter_chain_Delete(sys->filter_chain);
> + free(sys->filter_chain_current);
> + vlc_mutex_destroy(&sys->filter_chain_lock);
> + free(sys->source_chain_update);
> + free(sys->filter_chain_update);
> +
> + /* Destroy all remaining subpictures */
> + for (size_t i = 0; i < sys->channels.size; ++i)
> + spu_channel_Clean(sys, &sys->channels.data[i]);
> +
> + vlc_vector_destroy(&sys->channels);
> +
> + vlc_mutex_destroy(&sys->lock);
> +
> + vlc_mutex_destroy(&sys->prerender.lock);
> + vlc_cond_destroy(&sys->prerender.cond);
> + vlc_cond_destroy(&sys->prerender.output_cond);
> + vlc_vector_clear(&sys->prerender.vector);
> + video_format_Clean(&sys->prerender.fmtdst);
> + video_format_Clean(&sys->prerender.fmtsrc);
> +}
> +
> +/**
> + * Destroy the subpicture unit
> + *
> + * \param p_this the parent object which destroys the subpicture unit
> + */
> +void spu_Destroy(spu_t *spu)
> +{
> + spu_private_t *sys = spu->p;
> + /* stop prerendering */
> + vlc_cancel(sys->prerender.thread);
> + vlc_join(sys->prerender.thread, NULL);
> + /* delete filters and free resources */
> + spu_Cleanup(spu);
> + vlc_object_delete(spu);
> +}
> +
> #undef spu_Create
> /**
> * Creates the subpicture unit
> @@ -1447,51 +1657,22 @@ spu_t *spu_Create(vlc_object_t *object,
> vout_thread_t *vout)
> sys->last_sort_date = -1;
> sys->vout = vout;
>
> - return spu;
> -}
> -
> -/**
> - * Destroy the subpicture unit
> - *
> - * \param p_this the parent object which destroys the subpicture unit
> - */
> -void spu_Destroy(spu_t *spu)
> -{
> - spu_private_t *sys = spu->p;
> -
> - if (sys->text)
> - FilterRelease(sys->text);
> -
> - if (sys->scale_yuvp)
> - FilterRelease(sys->scale_yuvp);
> -
> - if (sys->scale)
> - FilterRelease(sys->scale);
> -
> - filter_chain_ForEach(sys->source_chain, SubSourceClean, spu);
> - if (sys->vout)
> - filter_chain_ForEach(sys->source_chain,
> - SubSourceDelProxyCallbacks, sys->vout);
> - filter_chain_Delete(sys->source_chain);
> - free(sys->source_chain_current);
> - if (sys->vout)
> - filter_chain_ForEach(sys->filter_chain,
> - SubFilterDelProxyCallbacks, sys->vout);
> - filter_chain_Delete(sys->filter_chain);
> - free(sys->filter_chain_current);
> - vlc_mutex_destroy(&sys->filter_chain_lock);
> - free(sys->source_chain_update);
> - free(sys->filter_chain_update);
> -
> - /* Destroy all remaining subpictures */
> - for (size_t i = 0; i < sys->channels.size; ++i)
> - spu_channel_Clean(&sys->channels.data[i]);
> -
> - vlc_vector_destroy(&sys->channels);
> -
> - vlc_mutex_destroy(&sys->lock);
> + vlc_mutex_init(&sys->prerender.lock);
> + vlc_cond_init(&sys->prerender.cond);
> + vlc_cond_init(&sys->prerender.output_cond);
> + vlc_vector_init(&sys->prerender.vector);
> + video_format_Init(&sys->prerender.fmtdst, 0);
> + video_format_Init(&sys->prerender.fmtsrc, 0);
> + sys->prerender.chroma_list = NULL;
> + sys->prerender.p_processed = NULL;
> + if(vlc_clone(&sys->prerender.thread, spu_PrerenderThread, spu,
> VLC_THREAD_PRIORITY_VIDEO))
> + {
> + spu_Cleanup(spu);
> + vlc_object_delete(spu);
> + spu = NULL;
> + }
>
> - vlc_object_delete(spu);
> + return spu;
> }
>
> /**
> @@ -1652,9 +1833,27 @@ void spu_PutSubpicture(spu_t *spu, subpicture_t *subpic)
> subpicture_Delete(subpic);
> return;
> }
> + spu_PrerenderEnqueue(sys, subpic);
> vlc_mutex_unlock(&sys->lock);
> }
>
> +static const vlc_fourcc_t chroma_list_default_yuv[] = {
> + VLC_CODEC_YUVA,
> + VLC_CODEC_RGBA,
> + VLC_CODEC_ARGB,
> + VLC_CODEC_BGRA,
> + VLC_CODEC_YUVP,
> + 0,
> +};
> +static const vlc_fourcc_t chroma_list_default_rgb[] = {
> + VLC_CODEC_RGBA,
> + VLC_CODEC_ARGB,
> + VLC_CODEC_BGRA,
> + VLC_CODEC_YUVA,
> + VLC_CODEC_YUVP,
> + 0,
> +};
> +
> subpicture_t *spu_Render(spu_t *spu,
> const vlc_fourcc_t *chroma_list,
> const video_format_t *fmt_dst,
> @@ -1691,27 +1890,13 @@ subpicture_t *spu_Render(spu_t *spu,
> /* Run subpicture sources */
> filter_chain_SubSource(sys->source_chain, spu, system_now);
>
> - static const vlc_fourcc_t chroma_list_default_yuv[] = {
> - VLC_CODEC_YUVA,
> - VLC_CODEC_RGBA,
> - VLC_CODEC_ARGB,
> - VLC_CODEC_BGRA,
> - VLC_CODEC_YUVP,
> - 0,
> - };
> - static const vlc_fourcc_t chroma_list_default_rgb[] = {
> - VLC_CODEC_RGBA,
> - VLC_CODEC_ARGB,
> - VLC_CODEC_BGRA,
> - VLC_CODEC_YUVA,
> - VLC_CODEC_YUVP,
> - 0,
> - };
> -
> if (!chroma_list || *chroma_list == 0)
> chroma_list = vlc_fourcc_IsYUV(fmt_dst->i_chroma) ?
> chroma_list_default_yuv
> :
> chroma_list_default_rgb;
>
> + /* wake up prerenderer, we have some video size and chroma */
> + spu_PrerenderWake(sys, fmt_dst, fmt_src, chroma_list);
> +
> vlc_mutex_lock(&sys->lock);
>
> size_t subpicture_count;
> @@ -1730,6 +1915,9 @@ subpicture_t *spu_Render(spu_t *spu,
> for (size_t i = 0; i < subpicture_count; i++) {
> spu_render_entry_t *entry = &subpicture_array[i];
> subpicture_t *subpic = entry->subpic;
> +
> + spu_PrerenderSync(sys, entry->subpic);
> +
> if (!subpic->updater.pf_validate)
> continue;
>
> @@ -1789,10 +1977,14 @@ ssize_t spu_RegisterChannel(spu_t *spu)
> return spu_RegisterChannelInternal(spu, NULL, NULL);
> }
>
> -static void spu_channel_Clear(struct spu_channel *channel)
> +static void spu_channel_Clear(spu_private_t *sys,
> + struct spu_channel *channel)
> {
> for (size_t i = 0; i < channel->entries.size; i++)
> + {
> + spu_PrerenderCancel(sys, channel->entries.data[i].subpic);
> spu_channel_DeleteAt(channel, i);
> + }
> }
>
> void spu_ClearChannel(spu_t *spu, size_t channel_id)
> @@ -1800,7 +1992,7 @@ void spu_ClearChannel(spu_t *spu, size_t channel_id)
> spu_private_t *sys = spu->p;
> vlc_mutex_lock(&sys->lock);
> struct spu_channel *channel = spu_GetChannel(spu, channel_id);
> - spu_channel_Clear(channel);
> + spu_channel_Clear(sys, channel);
> if (channel->clock)
> {
> vlc_clock_Reset(channel->clock);
> @@ -1815,7 +2007,7 @@ void spu_UnregisterChannel(spu_t *spu, size_t channel_id)
>
> vlc_mutex_lock(&sys->lock);
> struct spu_channel *channel = spu_GetChannel(spu, channel_id);
> - spu_channel_Clean(channel);
> + spu_channel_Clean(sys, channel);
> vlc_vector_remove(&sys->channels, channel_id);
> vlc_mutex_unlock(&sys->lock);
> }
> --
> 2.20.1
>
> _______________________________________________
> 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