mirror of https://github.com/mpv-player/mpv
e265c07547
Dear diary, today I fixed a shitty bug that was all my fault because I made a horrible mess. (Except it was a horrible mess before I even touched this shit, but let's not blame others.) Sometimes, updates to VO option that control video sizing (like panscan) didn't update the screen correctly. They were delayed until the next option change or so. It turns out that if the option update happens at the "same" time as a VOCTRL, update_opts() doesn't actually notify the vo_driver of the change. This in turn happened because run_control() called m_config_cache_update(). The latter function returns true if the options changed since the last call, and update_opts() also calls it (on the same config cache) for the same purpose. The update_opts() call, which is triggered by a third mechanism, comes later, but the cache update call will return false (as it should). Basically, given the config API, you can't act differently on multiple update calls and expect it to work. The skipped handling in update_opts() meant that the notification required to apply the changed option wasn't run. Fix this by simply calling update_opts() directly instead. Now there's only 1 m_config_cache_update() call on this specific instance. Fix the call in run_reconfig() too, so the previous sentence isn't a lie (but it probably doesn't make a difference in practice due to certain details). I'm not sure how I even ran into this sort-of race condition. The VOCTRL that messed up the option update was VOCTRL_UPDATE_PLAYBACK_STATE, which happens semi-regularly. Why this config cache shit and all the other shit? Rediscovering this crap wasn't pleasant. It's a bunch of hacks that became necessary when the ancient MPlayer architecture made it hard to move the VO to a separate thread. All the VO code typically accesses vo->opts (whose fields all used to be global variables in MPlayer). The frontend changes these on user input. Putting locking around all the options would be a nightmare, and keeping a copy of the options in the thread was much simpler. You need a way to propagate option changes, notify the thread, and update the local copy too. And the result of these thoughts was the config cache mechanism. In this specific case, the relevant cache update call in update_opts() triggers a VOCTRL_SET_PANSCAN to the VO driver, which isn't related to its former function anymore. Instead, it causes the VO driver to update the video sizing/placing options, which the generic VO code can't do. (Mostly because the VO driver includes the windowing stuff and is responsible for resizing etc. itself.) VOCTRLs sent by the frontend are even worse. MPlayer had no real runtime option change mechanism. Some options were vaguely duplicated by properties, so you could effectively change those options at runtime. Each of these options had its own VOCTRL, which still exist today, e.g. VOCTRL_FULLSCREEN, or VOCTRL_ONTOP. I tried to make all options runtime changeable, and to unify properties with options. But I couldn't be bothered with updating all VO drivers to listen to option changes directly, because that would be pretty tedious. So the property code is still all there and sends the old VOCTRLs. But of course you need to sync up the options, which is why the run_control() code did that. (Unrelated: VO_EVENT_FULLSCREEN_STATE is the worst shithack of them all. Currently, only the frontend can actually write to options (for awful reasons), so if the fullscreen state changes due to outside interaction, the VO driver can't update the corresponding option fields. So the VO notifies the frontend with said VO_EVENT_, and the frontend then sends VOCTRL_GET_FULLSCREEN, and updates the global copy of the option with the value returned by that. I still like to think the situation is not that bad considering the monstrous effort of converting single-threaded code that had hundreds of options in global variables to multi-threaded code with no global variables at all.) |
||
|---|---|---|
| .. | ||
| decode | ||
| filter | ||
| out | ||
| csputils.c | ||
| csputils.h | ||
| d3d.c | ||
| d3d.h | ||
| fmt-conversion.c | ||
| fmt-conversion.h | ||
| hwdec.c | ||
| hwdec.h | ||
| image_loader.c | ||
| image_loader.h | ||
| image_writer.c | ||
| image_writer.h | ||
| img_format.c | ||
| img_format.h | ||
| mp_image.c | ||
| mp_image.h | ||
| mp_image_pool.c | ||
| mp_image_pool.h | ||
| sws_utils.c | ||
| sws_utils.h | ||
| vaapi.c | ||
| vaapi.h | ||
| vdpau.c | ||
| vdpau.h | ||
| vdpau_functions.inc | ||
| vdpau_mixer.c | ||
| vdpau_mixer.h | ||