[vlc-devel] [PATCH 1/4] video_output: move the window lock outside of the window enabling function

Steve Lhomme robux4 at ycbcr.xyz
Mon Jan 13 14:03:09 CET 2020


This is not the first time we have this discussion. This is exactly how 
we name functions that are called with the locked done outside:

#find vlc -name "*.c" -exec grep -nH "Locked(" {} \;
vlc/modules/access/rist.c:154:static void 
rist_WriteTo_i11e_Locked(vlc_mutex_t lock, int fd, const void *buf, 
size_t len,
vlc/modules/access/rist.c:263:    rist_WriteTo_i11e_Locked(p_sys->lock, 
flow->fd_nack, buf, rtcp_feedback_size,
vlc/modules/access/rist.c:302: 
rist_WriteTo_i11e_Locked(p_sys->lock, fd_nack, buf, len,
vlc/modules/access/rist.c:342: 
rist_WriteTo_i11e_Locked(p_sys->lock, fd_nack, buf, len,
vlc/modules/audio_output/coreaudio_common.c:239:ca_GetLatencyLocked(audio_output_t 
*p_aout)
vlc/modules/audio_output/coreaudio_common.c:266:    *delay = 
ca_GetLatencyLocked(p_aout) + i_render_delay;
vlc/modules/audio_output/coreaudio_common.c:326:        const vlc_tick_t 
first_render_time = date - ca_GetLatencyLocked(p_aout);
vlc/modules/audio_output/mmdevice.c:175:static int 
VolumeSetLocked(audio_output_t *aout, float vol)
vlc/modules/audio_output/mmdevice.c:200:    int ret = 
VolumeSetLocked(aout, vol);
vlc/modules/audio_output/mmdevice.c:733:static int 
DeviceRequestLocked(audio_output_t *aout)
vlc/modules/audio_output/mmdevice.c:750:static int 
DeviceSelectLocked(audio_output_t *aout, const char *id)
vlc/modules/audio_output/mmdevice.c:764:    return 
DeviceRequestLocked(aout);
vlc/modules/audio_output/mmdevice.c:767:static int 
DeviceRestartLocked(audio_output_t *aout)
vlc/modules/audio_output/mmdevice.c:773:    return 
DeviceRequestLocked(aout);
vlc/modules/audio_output/mmdevice.c:780:    int ret = 
DeviceSelectLocked(aout, id);
vlc/modules/audio_output/mmdevice.c:1137:    if 
((sys->request_device_restart && DeviceRestartLocked(aout) != 0)
vlc/modules/audio_output/mmdevice.c:1172:            ret = 
DeviceRestartLocked(aout);
vlc/modules/audio_output/mmdevice.c:1179:            ret = 
DeviceSelectLocked(aout, NULL);
vlc/modules/audio_output/mmdevice.c:1264:        VolumeSetLocked(aout, 
var_InheritFloat(aout, "mmdevice-volume"));
vlc/modules/codec/omxil/mediacodec.c:168:static void 
DecodeFlushLocked(decoder_t *);
vlc/modules/codec/omxil/mediacodec.c:995:static void 
AbortDecoderLocked(decoder_t *p_dec)
vlc/modules/codec/omxil/mediacodec.c:1030:    DecodeFlushLocked(p_dec);
vlc/modules/codec/omxil/mediacodec.c:1032:    AbortDecoderLocked(p_dec);
vlc/modules/codec/omxil/mediacodec.c:1310:static void 
DecodeFlushLocked(decoder_t *p_dec)
vlc/modules/codec/omxil/mediacodec.c:1326:        AbortDecoderLocked(p_dec);
vlc/modules/codec/omxil/mediacodec.c:1341:    DecodeFlushLocked(p_dec);
vlc/modules/codec/omxil/mediacodec.c:1459:static int 
QueueBlockLocked(decoder_t *p_dec, block_t *p_in_block,
vlc/modules/codec/omxil/mediacodec.c:1579: 
AbortDecoderLocked(p_dec);
vlc/modules/codec/omxil/mediacodec.c:1587:    AbortDecoderLocked(p_dec);
vlc/modules/codec/omxil/mediacodec.c:1612: 
QueueBlockLocked(p_dec, NULL, true);
vlc/modules/codec/omxil/mediacodec.c:1619: 
QueueBlockLocked(p_dec, NULL, true);
vlc/modules/codec/omxil/mediacodec.c:1620:        DecodeFlushLocked(p_dec);
vlc/modules/codec/omxil/mediacodec.c:1643: 
AbortDecoderLocked(p_dec);
vlc/modules/codec/omxil/mediacodec.c:1656: 
QueueBlockLocked(p_dec, NULL, true);
vlc/modules/codec/omxil/mediacodec.c:1657:        DecodeFlushLocked(p_dec);
vlc/modules/codec/omxil/mediacodec.c:1675: 
AbortDecoderLocked(p_dec);
vlc/modules/codec/omxil/mediacodec.c:1683: 
QueueBlockLocked(p_dec, p_in_block, false);
vlc/src/input/decoder.c:267:static void DecoderUpdateFormatLocked( 
struct decoder_owner *p_owner )
vlc/src/input/decoder.c:383:        DecoderUpdateFormatLocked( p_owner );
vlc/src/input/decoder.c:581:    DecoderUpdateFormatLocked( p_owner );
vlc/src/input/decoder.c:643:        DecoderUpdateFormatLocked( p_owner );
vlc/src/input/decoder.c:897:            DecoderUpdateFormatLocked( 
p_owner );
vlc/src/input/es_out.c:224:static void         EsOutDelLocked( es_out_t 
*, es_out_id_t * );
vlc/src/input/es_out.c:246:static int EsOutControlLocked( es_out_t *out, 
int i_query, ... );
vlc/src/input/es_out.c:728:    EsOutControlLocked(out, 
ES_OUT_SET_JITTER, p_sys->i_pts_delay,
vlc/src/input/es_out.c:746:    EsOutControlLocked(out, 
ES_OUT_SET_JITTER, p_sys->i_pts_delay,
vlc/src/input/es_out.c:1871:static es_out_id_t *EsOutAddSlaveLocked( 
es_out_t *out, const es_format_t *fmt,
vlc/src/input/es_out.c:1989:    es_out_id_t *es = EsOutAddSlaveLocked( 
out, fmt, NULL );
vlc/src/input/es_out.c:2231:        EsOutDelLocked( out, 
parent->cc.pp_es[i] );
vlc/src/input/es_out.c:2480:        *pp_es = EsOutAddSlaveLocked( out, 
&fmt, parent );
vlc/src/input/es_out.c:2626:static void EsOutDelLocked( es_out_t *out, 
es_out_id_t *es )
vlc/src/input/es_out.c:2692:    EsOutDelLocked( out, es );
vlc/src/input/es_out.c:2696:static int EsOutVaControlLocked( es_out_t *, 
int, va_list );
vlc/src/input/es_out.c:2697:static int EsOutControlLocked( es_out_t 
*out, int i_query, ... )
vlc/src/input/es_out.c:2702:    int ret = EsOutVaControlLocked( out, 
i_query, args );
vlc/src/input/es_out.c:2745:static int EsOutVaControlLocked( es_out_t 
*out, int i_query, va_list args )
vlc/src/input/es_out.c:3109:                EsOutControlLocked( out, 
ES_OUT_RESET_PCR );
vlc/src/input/es_out.c:3111:                EsOutControlLocked( out, 
ES_OUT_SET_JITTER,
vlc/src/input/es_out.c:3267:        int i_ret = EsOutControlLocked( out, 
i_new_query, p_es );
vlc/src/input/es_out.c:3557:    i_ret = EsOutVaControlLocked( out, 
i_query, args );
vlc/src/input/es_out_timeshift.c:258:static int          TsPopCmdLocked( 
ts_thread_t *, ts_cmd_t *, bool b_flush );
vlc/src/input/es_out_timeshift.c:599:static int ControlLocked( es_out_t 
*p_out, int i_query, va_list args )
vlc/src/input/es_out_timeshift.c:765:    i_ret = ControlLocked( p_out, 
i_query, args );
vlc/src/input/es_out_timeshift.c:853:        if( TsPopCmdLocked( p_ts, 
&cmd, true ) )
vlc/src/input/es_out_timeshift.c:900:static int TsPopCmdLocked( 
ts_thread_t *p_ts, ts_cmd_t *p_cmd, bool b_flush )
vlc/src/input/es_out_timeshift.c:1013:            if( ( !p_ts->b_paused 
|| b_buffering ) && !TsPopCmdLocked( p_ts, &cmd, false ) )
vlc/src/input/input.c:1423:static size_t ControlGetReducedIndexLocked( 
input_thread_t *p_input,
vlc/src/input/input.c:1477:    size_t i_next_control_idx = 
ControlGetReducedIndexLocked( p_input, &c );
vlc/src/input/item.c:253:const char 
*input_item_GetMetaLocked(input_item_t *item,
vlc/src/input/item.c:267:    const char *value = 
input_item_GetMetaLocked( p_i, meta_type );
vlc/src/input/resource.c:327:static void 
input_resource_PutVoutLocked(input_resource_t *p_resource,
vlc/src/input/resource.c:363:    input_resource_PutVoutLocked( 
p_resource, vout );
vlc/src/input/resource.c:446: 
input_resource_PutVoutLocked(p_resource, cfg->vout);
vlc/src/media_source/media_tree.c:73:vlc_media_tree_AssertLocked(vlc_media_tree_t 
*tree)
vlc/src/media_source/media_tree.c:90: 
vlc_media_tree_AssertLocked(tree); \
vlc/src/media_source/media_tree.c:298:    vlc_media_tree_AssertLocked(tree);
vlc/src/media_source/media_tree.c:314:    vlc_media_tree_AssertLocked(tree);
vlc/src/media_source/media_tree.c:325:    vlc_media_tree_AssertLocked(tree);
vlc/src/misc/background_worker.c:311:static void 
BackgroundWorkerCancelLocked(struct background_worker *worker,
vlc/src/misc/background_worker.c:332: 
BackgroundWorkerCancelLocked(worker, id);
vlc/src/misc/background_worker.c:355: 
BackgroundWorkerCancelLocked(worker, NULL);
vlc/src/playlist/content.c:184:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/content.c:191:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/content.c:198:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/content.c:208:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/content.c:220:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/content.c:232:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/content.c:245:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/content.c:268:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/content.c:295:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/content.c:308:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/content.c:330:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/content.c:356:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:114:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:121:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:129:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:142:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:154:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:257:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:272:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:287:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:302:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:317:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:337:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:344:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:351:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:379:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:407:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:427:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/control.c:439:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/export.c:58:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/notify.c:53:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/notify.c:75:    vlc_playlist_AssertLocked(playlist); 
VLC_UNUSED(playlist);
vlc/src/playlist/notify.c:115:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/player.c:42:    vlc_playlist_AssertLocked(playlist);
//~ vlc/src/playlist/player.c:83:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/player.c:97:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/player.c:144:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/preparse.c:40:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/preparse.c:54:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/preparse.c:69:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/request.c:34:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/request.c:209:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/request.c:239:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/request.c:263:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/shuffle.c:35:    vlc_playlist_AssertLocked(playlist);
vlc/src/playlist/sort.c:79:            const char *value = 
input_item_GetMetaLocked(media, vlc_meta_Title);
vlc/src/playlist/sort.c:96:            const char *value = 
input_item_GetMetaLocked(media,
vlc/src/playlist/sort.c:102:            const char *value = 
input_item_GetMetaLocked(media, vlc_meta_Album);
vlc/src/playlist/sort.c:107:            const char *value = 
input_item_GetMetaLocked(media,
vlc/src/playlist/sort.c:114:            const char *value = 
input_item_GetMetaLocked(media, vlc_meta_Genre);
vlc/src/playlist/sort.c:119:            const char *str = 
input_item_GetMetaLocked(media, vlc_meta_Date);
vlc/src/playlist/sort.c:127:            const char *str = 
input_item_GetMetaLocked(media,
vlc/src/playlist/sort.c:136:            const char *str = 
input_item_GetMetaLocked(media,
vlc/src/playlist/sort.c:145:            const char *value = 
input_item_GetMetaLocked(media, vlc_meta_URL);
vlc/src/playlist/sort.c:150:            const char *str = 
input_item_GetMetaLocked(media, vlc_meta_Rating);
vlc/src/playlist/sort.c:368:    vlc_playlist_AssertLocked(playlist);
vlc/src/video_output/video_output.c:200:static void 
vout_UpdateWindowSizeLocked(vout_thread_t *vout)
vlc/src/video_output/video_output.c:407: 
vout_UpdateWindowSizeLocked(vout);
vlc/src/video_output/video_output.c:474: 
vout_UpdateWindowSizeLocked(vout);
vlc/src/video_output/video_output.c:494: 
vout_UpdateWindowSizeLocked(vout);
vlc/src/video_output/video_output.c:517: 
vout_UpdateWindowSizeLocked(vout);
vlc/src/video_output/video_output.c:549: 
vout_UpdateWindowSizeLocked(vout);
vlc/src/video_output/video_output.c:581: 
vout_UpdateWindowSizeLocked(vout);
vlc/src/video_output/video_output.c:2001:static int 
EnableWindowLocked(vout_thread_t *vout, const video_format_t *original)
vlc/src/video_output/video_output.c:2029: 
vout_UpdateWindowSizeLocked(vout);
vlc/src/video_output/video_output.c:2055:    if 
(EnableWindowLocked(vout, &original) != 0)
vlc/src/video_output/video_output.c:2109: 
EnableWindowLocked(cfg->vout, &original);

On 2020-01-13 13:58, Rémi Denis-Courmont wrote:
> A function that does not take necessary locks is Unlocked, e.g. fputc vs 
> fputc_unlocked. The names in this patch are misleading.
> 
> Le 13 janvier 2020 13:07:31 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a 
> écrit :
> 
>     ------------------------------------------------------------------------
>       src/video_output/video_output.c | 15 +++++++++------
>       1 file changed, 9 insertions(+), 6 deletions(-)
> 
>     diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
>     index 659ede546ba..53c6416a620 100644
>     --- a/src/video_output/video_output.c
>     +++ b/src/video_output/video_output.c
>     @@ -1998,7 +1998,7 @@ int vout_ChangeSource( vout_thread_t *vout, const video_format_t *original )
>           return -1;
>       }
>       
>     -static int vout_EnableWindow(vout_thread_t *vout, const video_format_t *original,
>     +static int EnableWindowLocked(vout_thread_t *vout, const video_format_t *original,
>                                    vlc_decoder_device **pp_dec_device)
>       {
>           vout_thread_sys_t *sys = vout->p;
>     @@ -2006,7 +2006,6 @@ static int vout_EnableWindow(vout_thread_t *vout, const video_format_t *original
>           assert(!sys->dummy);
>           assert(vout != NULL);
>       
>     -    vlc_mutex_lock(&sys->window_lock);
>           if (!sys->window_enabled) {
>               vout_window_cfg_t wcfg = {
>                   .is_fullscreen = var_GetBool(vout, "fullscreen"),
>     @@ -2022,7 +2021,6 @@ static int vout_EnableWindow(vout_thread_t *vout, const video_format_t *original
>               vout_SizeWindow(vout, original, &wcfg.width, &wcfg.height);
>       
>               if (vout_window_Enable(sys->display_cfg.window, &wcfg)) {
>     -            vlc_mutex_unlock(&sys->window_lock);
>                   msg_Err(vout, "failed to enable window");
>                   return -1;
>               }
>     @@ -2036,7 +2034,6 @@ static int vout_EnableWindow(vout_thread_t *vout, const video_format_t *original
>                   sys->dec_device = vlc_decoder_device_Create(&vout->obj, sys->display_cfg.window);
>               *pp_dec_device = sys->dec_device ? vlc_decoder_device_Hold( sys->dec_device ) : NULL;
>           }
>     -    vlc_mutex_unlock(&sys->window_lock);
>           return 0;
>       }
>       
>     @@ -2061,13 +2058,16 @@ int vout_Request(const vout_configuration_t *cfg, vlc_video_context *vctx, input
>               return 0;
>           }
>       
>     -    if (vout_EnableWindow(vout, &original, NULL) != 0)
>     +    vlc_mutex_lock(&sys->window_lock);
>     +    if (EnableWindowLocked(vout, &original, NULL) != 0)
>           {
>               /* the window was not enabled, nor the display started */
>               msg_Err(vout, "failed to enable window");
>               video_format_Clean(&original);
>     +        vlc_mutex_unlock(&sys->window_lock);
>               return -1;
>           }
>     +    vlc_mutex_unlock(&sys->window_lock);
>       
>           if (sys->display != NULL)
>               vout_StopDisplay(vout);
>     @@ -2105,6 +2105,7 @@ vlc_decoder_device *vout_GetDevice(const vout_device_configuration_t *cfg)
>           vlc_decoder_device *dec_device = NULL;
>       
>           assert(cfg->fmt != NULL);
>     +    vout_thread_sys_t *sys = cfg->vout->p;
>       
>           if (!VoutCheckFormat(cfg->fmt))
>               return NULL;
>     @@ -2112,7 +2113,9 @@ vlc_decoder_device *vout_GetDevice(const vout_device_configuration_t *cfg)
>           video_format_t original;
>           VoutFixFormat(&original, cfg->fmt);
>       
>     -    int res = vout_EnableWindow(cfg->vout, &original, &dec_device);
>     +    vlc_mutex_lock(&sys->window_lock);
>     +    int res = EnableWindowLocked(cfg->vout, &original, &dec_device);
>     +    vlc_mutex_unlock(&sys->window_lock);
>           video_format_Clean(&original);
>           if (res != 0)
>               return NULL;
> 
> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser 
> ma brièveté.
> 
> _______________________________________________
> 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