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

DVR #68

Open
wants to merge 33 commits into
base: dev
Choose a base branch
from
Open

DVR #68

wants to merge 33 commits into from

Conversation

shambler123
Copy link
Collaborator

this is a first version of a dvr recording with button and ambient audio. this will be updated soon

@shambler123 shambler123 changed the base branch from master to dev May 22, 2021 00:58
@fichek fichek marked this pull request as draft May 22, 2021 01:38
@fichek fichek changed the title Feature/22 DVR May 22, 2021
jlucidar and others added 18 commits May 23, 2021 00:33
# Conflicts:
#	app/src/main/java/com/fpvout/digiview/MainActivity.java
#	app/src/main/java/com/fpvout/digiview/PerformancePreset.java
#	app/src/main/java/com/fpvout/digiview/VideoReaderExoplayer.java
#	app/src/main/java/usb/AndroidUSBOutputStream.java
# Conflicts:
#	app/build.gradle
#	app/src/main/java/com/fpvout/digiview/InputStreamDataSource.java
#	app/src/main/java/com/fpvout/digiview/MainActivity.java
#	app/src/main/java/com/fpvout/digiview/UsbMaskConnection.java
#	app/src/main/java/com/fpvout/digiview/VideoReaderExoplayer.java
#	app/src/main/java/usb/AndroidUSBInputStream.java
Copy link
Member

@jlucidar jlucidar left a comment

Choose a reason for hiding this comment

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

A few comments on how we could better integrate this :)
Great job !

app/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
<uses-permission android:name="android.permission.WAKE_LOCK" />

<application
android:allowBackup="false"
android:icon="@mipmap/ic_launcher"
android:label="@string/app_name"
android:roundIcon="@mipmap/ic_launcher_round"
android:largeHeap="true"
android:requestLegacyExternalStorage="true"
Copy link
Member

Choose a reason for hiding this comment

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

Would be best to use scoped storage and shared directories :
https://developer.android.com/training/data-storage/shared/media

Copy link
Collaborator Author

@shambler123 shambler123 May 25, 2021

Choose a reason for hiding this comment

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

mhh i know bad habit to use the old implementation ;-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for backwards compatibility I leave it as it is. we can change it later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Better change it before a release/merge: WRITE_EXTERNAL_STORAGE no longer provides write access when targeting Android 11 - even with 'android:requestLegacyExternalStorage'. Sdk level 11+ is required for updates starting November 2021. This could delay feature/bugfix rollouts until the dvr is reworked.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok good to know! i will update that

app/src/main/java/usb/AndroidUSBInputStream.java Outdated Show resolved Hide resolved
app/src/main/java/com/fpvout/digiview/dvr/DVR.java Outdated Show resolved Hide resolved
@jlucidar jlucidar added the enhancement New feature or request label May 24, 2021
@jlucidar jlucidar added this to the v1.0.0 milestone May 24, 2021
@shambler123 shambler123 marked this pull request as ready for review May 26, 2021 03:48
@shambler123 shambler123 linked an issue May 26, 2021 that may be closed by this pull request
@fichek fichek requested a review from fmatt May 26, 2021 11:39
<uses-permission android:name="android.permission.WAKE_LOCK" />

<application
android:allowBackup="false"
android:icon="@mipmap/ic_launcher"
android:label="@string/app_name"
android:roundIcon="@mipmap/ic_launcher_round"
android:largeHeap="true"
android:requestLegacyExternalStorage="true"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better change it before a release/merge: WRITE_EXTERNAL_STORAGE no longer provides write access when targeting Android 11 - even with 'android:requestLegacyExternalStorage'. Sdk level 11+ is required for updates starting November 2021. This could delay feature/bugfix rollouts until the dvr is reworked.

private boolean zoomedIn;
private final Context context;
private PerformancePreset performancePreset = PerformancePreset.getPreset(PerformancePreset.PresetType.DEFAULT);
static final String VideoZoomedIn = "VideoZoomedIn";
private final SharedPreferences sharedPreferences;
private boolean streaming = false;

private VideoPlayingListener videoPlayingListener = null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Listeners could be combined into one interface.

Copy link
Member

Choose a reason for hiding this comment

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

yeah, I kept them seperated so you can subscribe to an event in one place and to the other in another place (as you need to overide both methods when you implement it in a combined way )
Or maybe I just wasn't able to do it :p

videoReaderEventMessage.obj = eventCode;
videoReaderEventListener.sendMessage(videoReaderEventMessage);
}
public interface VideoPlayingListener {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This interfaces should go into their own files and might be combined.

@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding this file will mess with developers with a different environment, please consider removing it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sry was a bit lazy and forget it. Normally android studio fixes itself ;-) We should add this file to the ignore list ;-)

@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding this file will mess with developers with a different environment, please consider removing it.
It is likely to be overwritten by a gradle build.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sry was a bit lazy and forget it. Normally android studio fixes itself ;-) We should add this file to the ignore list ;-)

streamDumper = new StreamDumper(activity, defaultFolder);
}

public static DVR getInstance(Activity context, boolean recordAmbientAudio, Handler updateAfterRecord, UsbMaskConnection connection){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Keeping the context in a singleton might lead to a memory leak.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i will change it . The thing is i wanted to make sure only one instance is used in the whole app.

File file = files[i];
if (file.getAbsolutePath().endsWith(".h264")) {
File ambientAudio = new File(file.getAbsolutePath().replace(".h264", ".aac"));
if (ambientAudio.exists()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider muxing even if the ambient audio is not existing, to at least have a useable video file.

Copy link
Member

Choose a reason for hiding this comment

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

great idea !

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes we can do that. No Problem 10 Mins of awesome sound ... you never know whats beeing recorded :D

markussta and others added 3 commits June 7, 2021 16:31
# Conflicts:
#	app/build.gradle
#	app/src/main/java/com/fpvout/digiview/H264Extractor.java
#	app/src/main/java/com/fpvout/digiview/InputStreamBufferedDataSource.java
#	app/src/main/java/com/fpvout/digiview/InputStreamDataSource.java
#	app/src/main/java/com/fpvout/digiview/MainActivity.java
#	app/src/main/java/com/fpvout/digiview/PerformancePreset.java
#	app/src/main/java/com/fpvout/digiview/VideoReaderExoplayer.java
#	app/src/main/java/usb/AndroidUSBInputStream.java
@jlucidar
Copy link
Member

jlucidar commented Dec 1, 2021

Hey, I'm working on merging this with dev branch.
It looks great ! I just need to make a few minor fixes before pushing it :

  • better handling of the "gallery button" (I'd like to see if opening the gallery instead of last video look better)
  • bugfix : record file of more than 2Gb make the app crash. (due to the h264 muxer processing that happen in memory... I will check if I can do it in an other way or if we have to "cut" h264 file before the 2Gb limit)

I will add more to the list if I encounter other things in the way.

@robertsmd
Copy link

robertsmd commented Oct 8, 2022

@jlucidar Any progress on merging this? I'm interested in using it. There are other apps similarly that have DVR, but this one has the best video latency.

Also, is there a way to not grant microphone privilege and still record? I did a quick pull and rebuild with commenting out the line recorder.setAudioSource(MediaRecorder.AudioSource.MIC);, but it still breaks.

I also am getting an issue where it crashes on stopping the recording. I'm unable to have adb and the goggles connected at the same time to help debug.

@jlucidar
Copy link
Member

jlucidar commented Oct 9, 2022

Hi @robertsmd, Haven't got the time to work on this project for a long time.
Most of the work done of the DVR was not done by myself so I might not be the best to answer your questions but I'll try to help :

  • The main issue I see before finishing the merge is that the video is only encoded when you stop recording. And it does it by loading the whole unencoded h264 file in memory. This is quite annoying on cheap devices with not a lot of memory, causing the app to crash and never start again (because we try to reencode the file if it was not done properly on the last ending). Maybe we can check how other people resolved this problem and find a similar way to achieve it.
  • According to the documentation, you should be able to just comment this line + recorder.setAudioEncoder(MediaRecorder.AudioEncoder.AAC);
    https://developer.android.com/reference/android/media/MediaRecorder#setAudioSource(int)
    (I would add this to the option page so it would be configurable)
  • If you want adb, you can use it over your network by opening a remote session. (I usually do this so I can see what happened when playing with usb stuff) :
    from your command line with usb connected to your device :
adb tcpip TCP_PORT
adb connect DEVICE_IP_ADDRESS:TCP_PORT

@shambler123
Copy link
Collaborator Author

I had not had the time to complete the implementation as @jlucidar said it there are some issues that needs to be resolved first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add IN-APP DVR
5 participants