[vlc-devel] [PATCH 6/6] vout: spu: remove max subpicture limit

Roland Bewick roland.bewick at gmail.com
Fri Jun 7 08:20:38 CEST 2019


On 7/06/2019 1:10 PM, Thomas Guillem wrote:
> On Fri, Jun 7, 2019, at 04:37, Roland Bewick wrote:
>> On 6/06/2019 8:19 PM, Thomas Guillem wrote:
>>> Use a vlc_vector instead of a hard-coded maximum size.
>>> ---
>>>    src/video_output/vout_subpictures.c | 106 +++++++++++++++-------------
>>>    1 file changed, 57 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/src/video_output/vout_subpictures.c b/src/video_output/vout_subpictures.c
>>> index a4430eed4d..8c5a39638d 100644
>>> --- a/src/video_output/vout_subpictures.c
>>> +++ b/src/video_output/vout_subpictures.c
>>> @@ -49,9 +49,6 @@
>>>     * Local prototypes
>>>     *****************************************************************************/
>>>    
>>> -/* Number of simultaneous subpictures */
>>> -#define VOUT_MAX_SUBPICTURES (__MAX(VOUT_MAX_PICTURES, SPU_MAX_PREPARE_TIME/5000))
>> I think you can also remove SPU_MAX_PREPARE_TIME
> Indeed,
>
>>> -
>>>    /* Hold of subpicture with converted ts */
>>>    typedef struct {
>>>        subpicture_t *subpic;
>>> @@ -60,8 +57,10 @@ typedef struct {
>>>        bool is_late;
>>>    } spu_render_entry_t;
>>>    
>>> +typedef struct VLC_VECTOR(spu_render_entry_t) spu_render_vector;
>>> +
>>>    struct spu_channel {
>>> -    spu_render_entry_t entries[VOUT_MAX_SUBPICTURES];
>>> +    spu_render_vector entries;
>>>        size_t id;
>>>        vlc_clock_t *clock;
>>>        vlc_tick_t delay;
>>> @@ -110,52 +109,49 @@ static void spu_channel_Init(struct spu_channel *channel, size_t id,
>>>    {
>>>        channel->id = id;
>>>        channel->clock = clock;
>>> -
>>> -    for (size_t i = 0; i < VOUT_MAX_SUBPICTURES; i++)
>>> -        channel->entries[i].subpic = NULL;
>>> +    vlc_vector_init(&channel->entries);
>>>    }
>>>    
>>>    static int spu_channel_Push(struct spu_channel *channel, subpicture_t *subpic)
>>>    {
>>> -    for (size_t i = 0; i < VOUT_MAX_SUBPICTURES; i++)
>>> -    {
>>> -        if (channel->entries[i].subpic)
>>> -            continue;
>>> -
>>> -        channel->entries[i].subpic = subpic;
>>> -        return VLC_SUCCESS;
>>> -    }
>>> -    return VLC_EGENERIC;
>>> +    const spu_render_entry_t entry = {
>>> +        .subpic = subpic,
>>> +    };
>>> +    return vlc_vector_push(&channel->entries, entry) ? VLC_SUCCESS : VLC_EGENERIC;
>>>    }
>>>    
>>>    static void spu_channel_DeleteAt(struct spu_channel *channel, size_t index)
>>>    {
>>> -    if (channel->entries[index].subpic)
>>> -    {
>>> -        subpicture_Delete(channel->entries[index].subpic);
>>> -        channel->entries[index].subpic = NULL;
>>> -    }
>>> +    assert(index < channel->entries.size);
>>> +    assert(channel->entries.data[index].subpic);
>>> +
>>> +    subpicture_Delete(channel->entries.data[index].subpic);
>>> +    vlc_vector_remove(&channel->entries, index);
>>>    }
>>>    
>>> -static int spu_channel_DeleteSubpicture(struct spu_channel *channel,
>>> -                                        subpicture_t *subpic)
>>> +static void spu_channel_DeleteSubpicture(struct spu_channel *channel,
>>> +                                         subpicture_t *subpic)
>>>    {
>>> -    for (size_t i = 0; i < VOUT_MAX_SUBPICTURES; i++)
>>> +    for (size_t i = 0; i < channel->entries.size; i++)
>>>        {
>>> -        if (channel->entries[i].subpic != subpic)
>>> -            continue;
>>> -
>>> -        spu_channel_DeleteAt(channel, i);
>>> -        return VLC_SUCCESS;
>>> +        if (channel->entries.data[i].subpic == subpic)
>>> +        {
>>> +            subpicture_Delete(subpic);
>>> +            vlc_vector_remove(&channel->entries, i);
>>> +            return;
>>> +        }
>>>        }
>>> -    return VLC_EGENERIC;
>>> +    vlc_assert_unreachable();
>>>    }
>>>    
>>>    static void spu_channel_Clean(struct spu_channel *channel)
>>>    {
>>> -    for (size_t i = 0; i < VOUT_MAX_SUBPICTURES; i++)
>>> -        if (channel->entries[i].subpic)
>>> -            subpicture_Delete(channel->entries[i].subpic);
>>> +    for (size_t i = 0; i < channel->entries.size; i++)
>>> +    {
>>> +        assert(channel->entries.data[i].subpic);
>>> +        subpicture_Delete(channel->entries.data[i].subpic);
>>> +    }
>>> +    vlc_vector_destroy(&channel->entries);
>>>    }
>>>    
>>>    static struct spu_channel *spu_GetChannel(spu_t *spu, size_t channel_id)
>>> @@ -572,11 +568,15 @@ static size_t spu_channel_ConvertDates(struct spu_channel *channel,
>>>    {
>>>        /* Put every spu start and stop ts into the same array to convert them in
>>>         * one shot */
>>> -    vlc_tick_t date_array[VOUT_MAX_SUBPICTURES *2];
>>> +    vlc_tick_t *date_array = vlc_alloc(channel->entries.size,
>>> +                                       2 * sizeof(vlc_tick_t));
>>> +    if (!date_array)
>>> +        return 0;
>>> +
>>>        size_t entry_count = 0;
>>> -    for (size_t index = 0; index < VOUT_MAX_SUBPICTURES; index++)
>>> +    for (size_t index = 0; index < channel->entries.size; index++)
>>>        {
>>> -        subpicture_t *current = channel->entries[index].subpic;
>>> +        subpicture_t *current = channel->entries.data[index].subpic;
>>>    
>>>            if (!current)
>>>                continue;
>>> @@ -586,7 +586,10 @@ static size_t spu_channel_ConvertDates(struct spu_channel *channel,
>>>            entry_count++;
>>>        }
>>>        if (entry_count == 0)
>>> +    {
>>> +        free(date_array);
>>>            return 0;
>>> +    }
>>>    
>>>        /* Convert all spu ts */
>>>        if (channel->clock)
>>> @@ -595,9 +598,9 @@ static size_t spu_channel_ConvertDates(struct spu_channel *channel,
>>>    
>>>        /* Put back the converted ts into the output spu_render_entry_t struct */
>>>        entry_count = 0;
>>> -    for (size_t index = 0; index < VOUT_MAX_SUBPICTURES; index++)
>>> +    for (size_t index = 0; index < channel->entries.size; index++)
>>>        {
>>> -        spu_render_entry_t *render_entry = &channel->entries[index];
>>> +        spu_render_entry_t *render_entry = &channel->entries.data[index];
>>>            subpicture_t *current = render_entry->subpic;
>>>    
>>>            if (!current)
>>> @@ -610,6 +613,8 @@ static size_t spu_channel_ConvertDates(struct spu_channel *channel,
>>>                entry_count++;
>>>            }
>>>        }
>>> +
>>> +    free(date_array);
>>>        return entry_count;
>>>    }
>>>    
>>> @@ -619,9 +624,7 @@ spu_render_entry_IsSelected(spu_render_entry_t *render_entry, size_t channel_id,
>>>                                vlc_tick_t render_subtitle_date, bool ignore_osd)
>>>    {
>>>        subpicture_t *subpic = render_entry->subpic;
>>> -
>>> -    if (!subpic)
>>> -        return false;
>>> +    assert(subpic);
>>>    
>>>        if ((subpic->i_channel >= 0 && (size_t) subpic->i_channel != channel_id) ||
>>>           (ignore_osd && !subpic->b_subtitle))
>>> @@ -656,9 +659,14 @@ spu_SelectSubpictures(spu_t *spu, vlc_tick_t system_now,
>>>    
>>>        /* */
>>>        *subpicture_count = 0;
>>> +    size_t total_size = 0;
>>> +    for (size_t i = 0; i < sys->channels.size; ++i)
>>> +        total_size += sys->channels.data[i].entries.size;
>>> +    if (total_size == 0)
>>> +        return NULL;
>>> +
>>>        spu_render_entry_t *subpicture_array =
>>> -        vlc_alloc(sys->channels.size,
>>> -                  sizeof(spu_render_entry_t) * VOUT_MAX_SUBPICTURES);
>>> +        vlc_alloc(total_size, sizeof(spu_render_entry_t));
>>>        if (!subpicture_array)
>>>            return NULL;
>>>    
>>> @@ -666,7 +674,7 @@ spu_SelectSubpictures(spu_t *spu, vlc_tick_t system_now,
>>>        for (size_t i = 0; i < sys->channels.size; i++)
>>>        {
>>>            struct spu_channel *channel = &sys->channels.data[i];
>>> -        spu_render_entry_t *render_entries = channel->entries;
>>> +        spu_render_entry_t *render_entries = channel->entries.data;
>>>    
>>>            vlc_tick_t   start_date = render_subtitle_date;
>>>            vlc_tick_t   ephemer_subtitle_date = 0;
>>> @@ -678,7 +686,7 @@ spu_SelectSubpictures(spu_t *spu, vlc_tick_t system_now,
>>>                continue;
>>>    
>>>            /* Select available pictures */
>>> -        for (size_t index = 0; index < VOUT_MAX_SUBPICTURES; index++) {
>>> +        for (size_t index = 0; index < channel->entries.size; index++) {
>>>                spu_render_entry_t *render_entry = &render_entries[index];
>>>                subpicture_t *current = render_entry->subpic;
>>>                bool is_stop_valid;
>>> @@ -720,7 +728,7 @@ spu_SelectSubpictures(spu_t *spu, vlc_tick_t system_now,
>>>                start_date = INT64_MAX;
>>>    
>>>            /* Select pictures to be displayed */
>>> -        for (size_t index = 0; index < VOUT_MAX_SUBPICTURES; index++) {
>>> +        for (size_t index = 0; index < channel->entries.size; index++) {
>>>                spu_render_entry_t *render_entry = &render_entries[index];
>>>                subpicture_t *current = render_entry->subpic;
>>>                bool is_late = render_entry->is_late;
>>> @@ -1147,7 +1155,7 @@ static subpicture_t *SpuRenderSubpictures(spu_t *spu,
>>>        subpicture_region_t **output_last_ptr = &output->p_region;
>>>    
>>>        /* Allocate area array for subtitle overlap */
>>> -    spu_area_t subtitle_area_buffer[VOUT_MAX_SUBPICTURES];
>>> +    spu_area_t subtitle_area_buffer[100];
>> I would like to see 100 overlapping subtitles :-)
>>
>> Anyway, maybe better to have a constant?
> cf. code just after: it's allocated from the heap if the count is > 100.

Ah, I see.

>
>>>        spu_area_t *subtitle_area;
>>>        size_t subtitle_area_count = 0;
>>>    
>>> @@ -1797,7 +1805,7 @@ ssize_t spu_RegisterChannel(spu_t *spu)
>>>    
>>>    static void spu_channel_Clear(struct spu_channel *channel)
>>>    {
>>> -    for (size_t i = 0; i < VOUT_MAX_SUBPICTURES; i++)
>>> +    for (size_t i = 0; i < channel->entries.size; i++)
>>>            spu_channel_DeleteAt(channel, i);
>>>    }
>>>    
>>> @@ -1821,7 +1829,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_Clear(channel);
>>> +    spu_channel_Clean(channel);
>>>        vlc_vector_remove(&sys->channels, channel_id);
>>>        vlc_mutex_unlock(&sys->lock);
>>>    }
> By the way, I will work on rebasing your branch on top of my new branch soon, probably start of next week.

Great :-)

If you need anything, feel free to email me.

I will keep an eye open for new patches.

>
>> _______________________________________________
>> 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