Skip to content

obs-ffmpeg: Add end action UI property for local file source #6239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mvji wants to merge 1 commit into
base: master
Choose a base branch
from
Open

obs-ffmpeg: Add end action UI property for local file source #6239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions plugins/obs-ffmpeg/data/locale/en-US.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ NVENC.TooManyBFrames="Max B-frames setting (%d) is more than encoder supports (%
FFmpegSource="Media Source"
LocalFile="Local File"
Looping="Loop"
EndPlayback="End of Playback"
EndPlayback.ShowNothing="Show Nothing"
EndPlayback.Freeze="Freeze on last frame"
EndPlayback.Loop="Loop"
Input="Input"
InputFormat="Input Format"
BufferingMB="Network Buffering"
Expand Down
84 changes: 79 additions & 5 deletions plugins/obs-ffmpeg/obs-ffmpeg-source.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,20 +87,73 @@ static bool is_local_file_modified(obs_properties_t *props,
obs_property_t *input_format =
obs_properties_get(props, "input_format");
obs_property_t *local_file = obs_properties_get(props, "local_file");
obs_property_t *looping = obs_properties_get(props, "looping");
obs_property_t *end_playback =
obs_properties_get(props, "end_playback");
obs_property_t *buffering = obs_properties_get(props, "buffering_mb");
obs_property_t *seekable = obs_properties_get(props, "seekable");
obs_property_t *speed = obs_properties_get(props, "speed_percent");
obs_property_t *reconnect_delay_sec =
obs_properties_get(props, "reconnect_delay_sec");
obs_property_t *clear_on_media_end =
obs_properties_get(props, "clear_on_media_end");
obs_property_set_visible(input, !enabled);
obs_property_set_visible(input_format, !enabled);
obs_property_set_visible(buffering, !enabled);
obs_property_set_visible(local_file, enabled);
obs_property_set_visible(looping, enabled);
obs_property_set_visible(end_playback, enabled);
obs_property_set_visible(speed, enabled);
obs_property_set_visible(seekable, !enabled);
obs_property_set_visible(reconnect_delay_sec, !enabled);
obs_property_set_visible(clear_on_media_end, !enabled);

return true;
}

static bool is_end_playback_modified(obs_properties_t *props,
obs_property_t *prop, obs_data_t *settings)
{
UNUSED_PARAMETER(prop);
UNUSED_PARAMETER(props);
int end_playback = obs_data_get_int(settings, "end_playback");
bool looping = false;
bool clear_on_media_end = true;

if (end_playback == 1) { // End action : Hide
clear_on_media_end = false;
} else if (end_playback == 2) { // End action : Loop
clear_on_media_end = false;
looping = true;
}
obs_data_set_bool(settings, "looping", looping);
obs_data_set_bool(settings, "clear_on_media_end", clear_on_media_end);

return true;
}

static void update_end_playback_property_list(obs_data_t *settings)
{
bool clear_on_media_end =
obs_data_get_bool(settings, "clear_on_media_end");
bool looping = obs_data_get_bool(settings, "looping");
int end_playback = 0;

if (looping == true) {
end_playback = 2;
} else if (clear_on_media_end == false) {
end_playback = 1;
}

obs_data_set_int(settings, "end_playback", end_playback);
Copy link
Member

@PatTheMav PatTheMav Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably also want to use obs_data_erase to remove the old setting when upgrading it to the new variant. Otherwise the old setting will persist in a user's configuration and you will always go and update it.

Probably worthwhile to check if the old setting exists first and only go through the effort to upgrade when it's necessary in the first place.

Copy link
Member

@RytoEX RytoEX Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A "safer" method would be to only upgrade the setting if the old setting exists and the new setting does not. Erasing old setting values completely breaks configs for older versions.

Copy link
Member

@PatTheMav PatTheMav Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that expected? If I were to create the source anew after the change, I could not go back to an older version of OBS and expect the setting to be "downgraded" either.

I'm aware of programs commonly upgrading settings but not that these upgraded settings are still expected to work with older versions (new settings would just be ignored).

Copy link
Member

@RytoEX RytoEX Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not saying that the upgraded settings should work with old versions. I'm saying that if you open a newer OBS one time that deletes settings entirely, and then you open an older OBS because you only wanted to run the newer OBS once to check it out, your settings are gone. I'm aware that we don't want to encourage or support downgrading, but purposefully breaking stuff seems a bit risky for users who just want to test stuff (and don't know that we encourage using portable mode or backing up your settings beforehand).

Copy link
Member

@PatTheMav PatTheMav Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me, there's no issue with keeping it. The property should be removed nevertheless though.

Copy link
Member

@RytoEX RytoEX Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me, there's no issue with keeping it. The property should be removed nevertheless though.

Agree with that.

}

static bool is_clear_on_media_end_modified(obs_properties_t *props,
obs_property_t *prop,
obs_data_t *settings)
{
UNUSED_PARAMETER(prop);
UNUSED_PARAMETER(props);

update_end_playback_property_list(settings);

return true;
}
Expand All @@ -109,6 +162,7 @@ static void ffmpeg_source_defaults(obs_data_t *settings)
{
obs_data_set_default_bool(settings, "is_local_file", true);
obs_data_set_default_bool(settings, "looping", false);
obs_data_set_default_int(settings, "end_playback", 0);
obs_data_set_default_bool(settings, "clear_on_media_end", true);
obs_data_set_default_bool(settings, "restart_on_activate", true);
obs_data_set_default_bool(settings, "linear_alpha", false);
Expand Down Expand Up @@ -166,7 +220,23 @@ static obs_properties_t *ffmpeg_source_getproperties(void *data)
dstr_free(&filter);
dstr_free(&path);

obs_properties_add_bool(props, "looping", obs_module_text("Looping"));
prop = obs_properties_add_bool(props, "looping",
Copy link
Member

@PatTheMav PatTheMav Jan 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the property is upgraded, it should not be retained and hidden. As I outlined above, the config value should be upgraded when it exists, but from then on this property will not have any use.

Properties can and will be queried by other code which will not take visibility into account, so for those consumers it will be confusing that there is a bespoke property for looping and another one that also enables looping.

obs_module_text("Looping"));
obs_property_set_visible(prop, false);

prop = obs_properties_add_list(props, "end_playback",
obs_module_text("EndPlayback"),
OBS_COMBO_TYPE_LIST,
OBS_COMBO_FORMAT_INT);
obs_property_list_add_int(
prop, obs_module_text("EndPlayback.ShowNothing"), 0);

obs_property_list_add_int(prop, obs_module_text("EndPlayback.Freeze"),
1);

obs_property_list_add_int(prop, obs_module_text("EndPlayback.Loop"), 2);

obs_property_set_modified_callback(prop, is_end_playback_modified);

obs_properties_add_bool(props, "restart_on_activate",
obs_module_text("RestartWhenActivated"));
Expand All @@ -191,8 +261,11 @@ static obs_properties_t *ffmpeg_source_getproperties(void *data)
obs_properties_add_bool(props, "hw_decode",
obs_module_text("HardwareDecode"));

obs_properties_add_bool(props, "clear_on_media_end",
obs_module_text("ClearOnMediaEnd"));
prop = obs_properties_add_bool(props, "clear_on_media_end",
obs_module_text("ClearOnMediaEnd"));

obs_property_set_modified_callback(prop,
is_clear_on_media_end_modified);

prop = obs_properties_add_bool(
props, "close_when_inactive",
Expand Down Expand Up @@ -422,6 +495,7 @@ static void ffmpeg_source_update(void *data, obs_data_t *settings)
input = obs_data_get_string(settings, "local_file");
input_format = NULL;
s->is_looping = obs_data_get_bool(settings, "looping");
update_end_playback_property_list(settings);
} else {
input = obs_data_get_string(settings, "input");
input_format = obs_data_get_string(settings, "input_format");
Expand Down