[vlc-devel] [PATCH 1/4] video_output: move the window lock outside of the window enabling function
Thomas Guillem
thomas at gllm.fr
Mon Jan 13 14:51:48 CET 2020
Both way are fine with me. I think the most important is to respect the style used by the file/struct/API.
On Mon, Jan 13, 2020, at 14:03, Steve Lhomme wrote:
> 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
> >
> _______________________________________________
> 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