Skip to content
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.

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

Make the retrieval of audio tracks consistent and implement trimming to AnimationTrackEditor shortcut and clean-up #86661

Merged
merged 1 commit into from
Feb 17, 2024

Conversation

TokageItLab
Copy link
Member

#85088 occurred in 4.2.0 and should be temporarily fixed in 4.2.1. However, it is causing a more serious regression as #86147. Therefore, #86227 must be merged and cause problem #85088 again.

Solution:

Allow the audio track to get the key from the middle of the track by forcing NEAREST to get the key. This may cause double playback, but unlike a method track, the same stream is never played twice at the same time, so this should not be a problem.

However, keys placed in negative time can only be retrieved during seek. This means that the key will not be retrieved at loop time.

The reason for that is the difference in the search range between track_find_key() and track_get_key_indices_in_range(). To begin with, it is not expected to access keys outside the animation range in the playback process, so I add an argument p_limit to track_find_key() to limit the range.

As a result, audio keys placed at negative time will not be played, although playback of audio keys from the middle will be permitted. Finally, add an option to Clean-up to trim them, then close #85088.

image

@TokageItLab TokageItLab requested review from a team as code owners December 31, 2023 11:35
@TokageItLab TokageItLab force-pushed the fix-audio-playback branch 3 times, most recently from e4c3d7c to 565e034 Compare January 3, 2024 14:50
@TokageItLab TokageItLab force-pushed the fix-audio-playback branch 2 times, most recently from fcb34ac to 851dad6 Compare January 14, 2024 11:39
editor/animation_track_editor.cpp Outdated Show resolved Hide resolved
editor/animation_track_editor.cpp Outdated Show resolved Hide resolved
@fire
Copy link
Member

fire commented Feb 12, 2024

@TokageItLab This needs a rebase so we can review and merge into Godot Engine master.

@TokageItLab TokageItLab requested review from a team as code owners February 13, 2024 06:06
@TokageItLab TokageItLab requested review from a team as code owners February 13, 2024 08:30
@TokageItLab TokageItLab requested review from fire and removed request for a team February 13, 2024 10:45
@TokageItLab TokageItLab force-pushed the fix-audio-playback branch 2 times, most recently from 02402c5 to aaf3775 Compare February 17, 2024 07:21
@TokageItLab
Copy link
Member Author

Due to concerns from @lyuma , options that are only valid for Audio now clearly indicate it and it will gray out when Audio Key is not selected.

image

@akien-mga
Copy link
Member

Needs rebase after #86629.

@akien-mga akien-mga merged commit 9283d0d into godotengine:master Feb 17, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips
Copy link
Member

AThousandShips commented Feb 17, 2024

This crashes for me:

================================================================
CrashHandlerException: Program crashed
Engine version: Godot Engine v4.3.dev.custom_build (9283d0d65c42663326f3f6f3d621207b550225ae)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] AnimationTrackEditor::_edit_menu_pressed (editor\animation_track_editor.cpp:6250)
[1] call_with_variant_args_helper<AnimationTrackEditor,int,0> (core\variant\binder_common.h:309)
[2] call_with_variant_args<AnimationTrackEditor,int> (core\variant\binder_common.h:419)
[3] CallableCustomMethodPointer<AnimationTrackEditor,int>::call (core\object\callable_method_pointer.h:99)
[4] Callable::callp (core\variant\callable.cpp:57)
[5] Object::emit_signalp (core\object\object.cpp:1128)
[6] Node::emit_signalp (scene\main\node.cpp:3846)
[7] Object::emit_signal<int> (core\object\object.h:922)
[8] PopupMenu::activate_item (scene\gui\popup_menu.cpp:2414)
[9] PopupMenu::_input_from_window_internal (scene\gui\popup_menu.cpp:642)
[10] PopupMenu::_input_from_window (scene\gui\popup_menu.cpp:452)
[11] Window::_window_input (scene\main\window.cpp:1606)
[12] call_with_variant_args_helper<Window,Ref<InputEvent> const &,0> (core\variant\binder_common.h:304)
[13] call_with_variant_args<Window,Ref<InputEvent> const &> (core\variant\binder_common.h:419)
[14] CallableCustomMethodPointer<Window,Ref<InputEvent> const &>::call (core\object\callable_method_pointer.h:99)
[15] Callable::callp (core\variant\callable.cpp:57)
[16] Callable::call<Ref<InputEvent> > (core\variant\variant.h:864)
[17] DisplayServerWindows::_dispatch_input_event (platform\windows\display_server_windows.cpp:3171)
[18] DisplayServerWindows::_dispatch_input_events (platform\windows\display_server_windows.cpp:3141)
[19] Input::_parse_input_event_impl (core\input\input.cpp:771)
[20] Input::flush_buffered_events (core\input\input.cpp:1042)
[21] DisplayServerWindows::process_events (platform\windows\display_server_windows.cpp:2690)
[22] OS_Windows::run (platform\windows\os_windows.cpp:1476)
[23] widechar_main (platform\windows\godot_windows.cpp:182)
[24] _main (platform\windows\godot_windows.cpp:204)
[25] main (platform\windows\godot_windows.cpp:218)
[26] WinMain (platform\windows\godot_windows.cpp:232)
[27] __scrt_common_main_seh
[28] <couldn't map PC to fn name>
-- END OF BACKTRACE --
================================================================

When using the start/end offset options

Seems like stream is null here:

double len = stream->get_length() - animation->audio_track_get_key_end_offset(E.key.track, E.key.key);

Will test further soon

@AThousandShips
Copy link
Member

AThousandShips commented Feb 17, 2024

Is indeed when the key does not have an associated stream, will write a fix

Edit: Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

AudioStreamPlayer keyframe is silent if the AudioClip starts at an negative time position
5 participants