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

Remove video filters when detaching audio. #1203

Merged
merged 1 commit into from Dec 22, 2021

Conversation

bmatherly
Copy link
Member

Remove video filters from the audio clip
Remove audio filters from the video clip

As reported here:
https://forum.shotcut.org/t/detach-audio-duplicates-video-filters-on-audio-and-leave-audio-filters-on-video/29795

@ddennedy
Copy link
Member

So they can't move the audio clip "back" since the original video clip is still there.

They can move it anywhere. My "back" simply meant moving from any audio track to any video track - not necessarily to the same place. I think this change is doing maybe a little more than necessary to fix the bug reported, which I did not respond to. When a video stream is still selected in a clip on an audio track, the video is not processed (or any of its video-only filters). There is no overhead there. However, there is a legitimate but minor problem that Detach Audio does not remove the audio filters because they still process the silent "test" audio, which is not necessary.

If you drag the clip on an audio track to video track, should the user be required to know that they will need to re-select the video stream? Or, should the tool automatically choose the first one when doing that? Or how about a previously selected one? But if we were to keep the selected video stream for the case of moving from audio to video track, why get rid of the video filters (besides the anomaly of the non-analyzed stabilize)? What should happen if I copy the detached clip on the audio track and paste it to a video track? Today, the video with its filters is kept without having to learn about the extra steps. After this change you would need to restore the video and video filters. Of course, because the audio is deselected on the video clip, this situation does not entirely hold up when dragging a video clip after a detach to an audio track. On the other hand, we have the lack of waveform as a strong hint that it is not going to do what you wanted.

So, I guess we should make the detach operation make truly video-only and audo-only clips. Except I just found a bug that when you drag an A/V clip whose video stream is None from an audio to video track, it still shows the thumbnails. Is that bad? It is if you think it suggests that the clip is generating video. OTOH thumbnails help you remember a clip and is not really a preview, but we hide the waveform when you turn off the audio stream, and the shape of a waveform can help one remember a clip too. It all gets a little messy and confusing when you load many expectations instead of just accepting the current behavior and learn to handle it. And once you have done that and the tool changes behavior it requires relearning.

@bmatherly
Copy link
Member Author

They can move it anywhere. My "back" simply meant moving from any audio track to any video track - not necessarily to the same place.

That is a helpful clarification. We are both on the same page.

So, I guess we should make the detach operation make truly video-only and audio-only clips.

That is the perspective that I was coming from - and this is what I think the PR currently implements. I really like the simplicity and consistency of this as a concept. But you are making good points about the possible future use of these clips.

And once you have done that and the tool changes behavior it requires relearning.

My expectation is that most people who use the "Detach" feature have learned to use it before applying any filters - and they don't have a plan to move the detached audio to a video track. But your point is fair since the feature is probably being used by people in ways that we are not even aware of.

If you drag the clip on an audio track to video track, should the user be required to know that they will need to re-select the video stream?

This is a good comment from a more general perspective, too. If a user has an MP3 or the tone producer on a video track, what should their expectation be for the video? The MP3 might have cover art, but the tone producer will never make video. Perhaps a clip should have some kind of indicator to show if it has audio, video, or both.

Maybe we want to sleep on this change. I do not have a strong opinion on this change, so let me know if you are inspired by a particular solution and I can change the code.

@ddennedy
Copy link
Member

I am going to merge it as-is after the 21.12 release because I want to let it get more attention since it is a behavior change.

@ddennedy ddennedy added this to the v22.x milestone Dec 19, 2021
@bmatherly
Copy link
Member Author

because I want to let it get more attention since it is a behavior change.

Yes. I agree with that. I do not think it is a critical bug.

@ddennedy ddennedy merged commit 5f830e6 into mltframework:master Dec 22, 2021
@bmatherly bmatherly deleted the detach_audio branch December 24, 2021 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants