Skip to content

Conversation

@drift8797
Copy link

Now that the app is using CameraX 1.5, it is possible to set an initial muted state when recording a video. This PR makes it so that the microphone mute icon displays at all times in video mode, unless the Include Audio toggle is disabled.

Unlike Include Audio, this setting does not persist. This makes it useful as a quicker way to disable audio for a one-off recording or to start a recording muted and unmute audio only at a later time.

One downside compared to disabling Include Audio is that an audio track is always included regardless of whether the microphone was ever unmuted (it will be completely silent if muted for the entire recording). For videos meant to not have any audio, this uses additional storage and might display differently in some players, so I don't think this should replace the Include Audio toggle.

@MHShetty
Copy link
Member

Hi @drift8797,

As you have rightly mentioned, choosing not to include in the video is different from muting the audio while recording. Always showing the microphone when include audio option is enabled would be a good way to support both options, although I'm unsure if we would want to have too many options at the bottom or if there could be a better way to show the mute toggle.

I'll review and test your branch and provide feedback soon.

Thanks a lot for showing your interest towards contributing to our repositories, value and appreciate all the time and efforts you have put in!

@MHShetty
Copy link
Member

Hi @drift8797,

I have tested the code across multiple modes and it seems to work fine everywhere. There's some minor re-factoring that can be optionally done that I have suggested above, but others it seems all good.

@drift8797
Copy link
Author

@MHShetty Thanks for taking a look! I don't think I can see your feedback here, could you try resubmitting it? I'd be happy to make the changes.

I wasn't completely sure about the UI decisions here. I was thinking since this button already exists once the recording starts it makes sense to keep it in one place, but I would be open to implementing alternative options.

@MHShetty
Copy link
Member

Hi @drift8797,

Here's a screenshot of the code changes suggested that could be made: image

@MHShetty
Copy link
Member

I wasn't completely sure about the UI decisions here. I was thinking since this button already exists once the recording starts it makes sense to keep it in one place, but I would be open to implementing alternative options.

Yes that's completely understandable, I think the mute toggle feels fine in its place can't think of a better place, we could possibly keep it as it is

@drift8797
Copy link
Author

@MHShetty Thanks for the feedback! I agree it's cleaner like that and have updated it. I also moved the micOffIcon to follow the same pattern if that's alright, since it's also later set to visible in startCamera.

Also I realized I hadn't tested the video capture intent and I found that the mute toggle was still showing on the preview screen so I added a line to hide the button on that screen and confirmed it still shows again when pressing cancel.

@MHShetty
Copy link
Member

MHShetty commented Jun 26, 2025

No worries it's my pleasure @drift8797!

Both of the mentioned changes related to micOff indicator icon are unrelated to this PR, so it might be better to separate them into a different PR. Also there's PR #554 already filed previously to fix the UI issue you have mentioned, so undoing the micOff indicator changes within this PR should be fine to avoid conflicts.

Thanks for making these changes and contributing to our camera app!

@drift8797
Copy link
Author

@MHShetty I saw your PR #554. The change I made in VideoCaptureActivity here is for muteToggle, not micOffIcon. I think it would cause conflicts but it's necessary to avoid a regression. Previously the muteToggle would've been hidden on the preview screen by the call I had to remove in afterRecordingStops.

The other change was just in switchMode to match the same refactoring you suggested for consistency. That shouldn't conflict with your PR or affect functionality, though I can revert that.

@MHShetty
Copy link
Member

Hi @drift8797,

Ah apologizes for the inconvenience. I misunderstood what you were trying to convey earlier. The changes shouldn't conflict in that case. We can keep the changes as it is.

Accepting the PR might take a while as we're a bit occupied with our Android 16 porting process which might take a while, but will surely look forward to accepting this PR once everything settles there.

Thanks a lot for contributing to our camera app, appreciate and value all the time and efforts you have put in!

@drift8797
Copy link
Author

@MHShetty No worries at all, thank you for all you and the team do for GrapheneOS!

} else {
View.VISIBLE
}
mActivity.muteToggle.visibility =
Copy link

Choose a reason for hiding this comment

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

consider one liner
e.g.
mActivity.muteToggle.visibility = if (includeAudio) View.VISIBLE else View.GONE

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.

2 participants