Implement Camera2 support as a MediaSource #232

IanDBird · 2022-12-26T16:15:31Z

This PR introduces two new MediaSources:

ExternalMediaSource: This provides the owner direct access to the Surface that will be used as the input into any configured filters, followed by the encoder. The reason for this is to allow non-file based sources of video.
Camera2MediaSource: This utilises the ExternalMediaSource and uses Android's Camera2 APIs to render from the camera to the input Surface. This is being combined with the previously landed AudioRecordMediaSource that is adding the audio stream. This demonstrates a Camera to File example.

Also, one thing to note is that the ExternalMediaSource does not support cropping. It expects the source aspect ratio to match that of the file/target aspect ratio. We could either keep this as a hard constraint, or we could introduce a CroppingFilter to handle this case.

IanDBird · 2022-12-26T16:16:08Z

litr/src/main/java/com/linkedin/android/litr/io/Camera2MediaSource.kt

+     * Since we don't support cropping of the sensor or input surface, we will record to a Surface
+     * that has the same aspect ratio as the camera's sensor.
+     */
+    private fun getRecordSize(sensorRect: Rect?, sensorOrientation: Int?): Size {


For now, we will require the source aspect ratio to match the target aspect ratio. For the camera specific cases, we could follow up with a couple of options:

Have people crop at the sensor

Implement some filter support for cropping

While the former is arguably simpler, it introduces the limitation that if the application is using a Preview Surface, then it will also be cropped. A crop based filter may be a better / more versatile solution.

I have used DefaultVideoFrameRenderFilter for cropping in the past, we could probably experiment with that here.

Sounds good.

I thought for v1, requiring the same aspect ratio of the sensor is a requirement some people could live with. Then separately, we can iterate on it and support things like cropping.

IanDBird · 2022-12-26T16:16:57Z

litr/src/main/java/com/linkedin/android/litr/io/AudioRecordMediaSource.kt

@@ -61,7 +61,7 @@ class AudioRecordMediaSource(
    private var isRecording = false

    @Synchronized
-    fun start() {
+    fun startRecording() {


I tweaked to be consistent with the new MediaSources, and also thought being more explicit (rather than start/stop) would be better.

IanDBird · 2022-12-26T16:24:36Z

litr-demo/src/main/java/com/linkedin/android/litr/demo/RecordCamera2Fragment.kt

+    private lateinit var mediaTransformer: MediaTransformer
+    private var targetMedia: TargetMedia = TargetMedia()
+
+    private val cameraManager: CameraManager by lazy {


This requires lazy because I am deferring it to when I need to use it, which will be after the context has been attached to the Fragment.

izzytwosheds · 2022-12-26T16:54:34Z

litr-demo/src/main/java/com/linkedin/android/litr/demo/RecordCamera2Fragment.kt

+
+private const val DEFAULT_CAMERA_FPS = 30
+private const val DEFAULT_TARGET_BITRATE = 5_000_000 // 5Mbps
+private const val DEFAULT_RECORD_WIDTH = 1280


this works (and you can keep this "as is" since this is in demo app), but I would use resolution as a parameter since it is a common terminology.

Yeah, makes sense. I'll have a think, and potentially push another update. Maybe, while we have the aspect ratio requirement, we could do something like a "maximum resolution" that we compute our target resolution within. For this example case, it would be like passing in a max resolution of "1280x1280" which is basically the max bounding box?

Commonly, video size is defined as resolution (HD, FHD, 4K, etc.) + aspect ratio (square, letterbox, wide screen, etc.) I usually use that convention.

Again, this is a fragment, not public LiTr API, so we can do whatever here.

izzytwosheds · 2022-12-26T16:55:18Z

litr-demo/src/main/java/com/linkedin/android/litr/demo/RecordCamera2Fragment.kt

+    private lateinit var mediaTransformer: MediaTransformer
+    private var targetMedia: TargetMedia = TargetMedia()
+
+    private val cameraManager: CameraManager by lazy {


you can also use lateinit var here

Sure, we switch to be consistent 👍

izzytwosheds · 2022-12-26T16:56:23Z

litr-demo/src/main/java/com/linkedin/android/litr/demo/RecordCamera2Fragment.kt

+        // this.
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
+            AlertDialog.Builder(requireContext())
+                    .setMessage("Android Marshmallow or newer required")


You will probably get a lint error on hard coding strings.

Removed hard coded strings, and also fixed the same on in the RecordAudioFragment

izzytwosheds · 2022-12-26T16:59:19Z

litr-demo/src/main/java/com/linkedin/android/litr/demo/data/TransformationPresenter.java

+                    .setTargetTrack(1)
+                    .setTargetFormat(createAudioMediaFormat(audioTrackFormat))
+                    .setEncoder(new MediaCodecEncoder())
+                    .setDecoder(new MediaCodecDecoder());


JC: wasn't this supposed to be a PassthroughDecoder for audio recorder?

I can't remember exactly why I used the MediaCodecDecoder but I tried to change as you suggested, but it results in the following error:

2022-12-27 10:29:05.019 17230-17298/com.linkedin.android.litr.demo E/TransformationJob: Transformation job error java.nio.BufferOverflowException at java.nio.ByteBuffer.put(ByteBuffer.java:613) at java.nio.DirectByteBuffer.put(DirectByteBuffer.java:268) at com.linkedin.android.litr.render.PassthroughAudioProcessor.processFrame(PassthroughAudioProcessor.kt:21) at com.linkedin.android.litr.render.AudioRenderer.renderFrame(AudioRenderer.kt:103) at com.linkedin.android.litr.transcoder.AudioTrackTranscoder.queueDecodedInputFrame(AudioTrackTranscoder.java:182) at com.linkedin.android.litr.transcoder.AudioTrackTranscoder.processNextFrame(AudioTrackTranscoder.java:90) at com.linkedin.android.litr.TransformationJob.processNextFrame(TransformationJob.java:211) at com.linkedin.android.litr.TransformationJob.transform(TransformationJob.java:108) at com.linkedin.android.litr.TransformationJob.run(TransformationJob.java:77) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:463) at java.util.concurrent.FutureTask.run(FutureTask.java:264) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1137) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:637) at java.lang.Thread.run(Thread.java:1012)

That seems to happen even if I increase the inputBufferCapacity. I'd suggest we keep as is, but can look into this error separately, if that's okay?

That is totally fine. It was more of a question than change suggestion.

izzytwosheds · 2022-12-26T17:00:07Z

litr-demo/src/main/java/com/linkedin/android/litr/demo/view/AutoFitSurfaceView.kt

+    }
+
+    companion object {
+        private val TAG = AutoFitSurfaceView::class.java.simpleName


I don't think we need companion object here, this can be a const val

izzytwosheds · 2022-12-26T17:01:50Z

litr/src/main/java/com/linkedin/android/litr/io/Camera2MediaSource.kt

+            previewSurfaceHolder = holder
+        }
+
+        override fun surfaceChanged(surfaceHolder: SurfaceHolder, p1: Int, p2: Int, p3: Int) {}


p1, etc are not good parameter names, let's make them more descriptive

Nice catch, thanks

izzytwosheds · 2022-12-26T17:04:41Z

litr/src/main/java/com/linkedin/android/litr/io/Camera2MediaSource.kt

+     * configured Callback will be notified via onDeviceReady. Once this has done, the consumer is
+     * able to start the Transcode session.
+     */
+    @SuppressLint("MissingPermission")


Can we use @RequiresPermission (https://developer.android.com/reference/androidx/annotation/RequiresPermission) here?

I think we'd need to use both, right?

The RequiresPermission reports that the caller needs the permission

The SuppressLint avoids Android Studio trying to force that we add the permission to the litr manifest (instead requiring people who want/need to use the source to make sure they add it)

Actually, scrap that, you're right (and I made a typo which is why it failed locally)

izzytwosheds · 2022-12-26T17:06:22Z

litr/src/main/java/com/linkedin/android/litr/io/Camera2MediaSource.kt

+     * Since we don't support cropping of the sensor or input surface, we will record to a Surface
+     * that has the same aspect ratio as the camera's sensor.
+     */
+    private fun getRecordSize(sensorRect: Rect?, sensorOrientation: Int?): Size {


I have used DefaultVideoFrameRenderFilter for cropping in the past, we could probably experiment with that here.

izzytwosheds · 2022-12-26T17:07:07Z

litr/src/main/java/com/linkedin/android/litr/io/Camera2MediaSource.kt

+     * that has the same aspect ratio as the camera's sensor.
+     */
+    private fun getRecordSize(sensorRect: Rect?, sensorOrientation: Int?): Size {
+        val aspectRatio = sensorRect?.let { it.height().toFloat() / it.width().toFloat() } ?: (3f / 4f)


note to self: we should add an enum (or a collection of constants) for typical aspect ratios. Don't have to do this in this PR.

izzytwosheds · 2022-12-26T17:10:26Z

litr/src/main/java/com/linkedin/android/litr/io/ExternalMediaSource.kt

+ * Callback. An instance of this class is expected to be both the MediaSource, and Decoder for which
+ * the pipeline is configured. This allows these components to be bypassed.
+ */
+open class ExternalMediaSource: MediaSource, Decoder {


This is a question, not a change suggestion. Would it make sense to name this CaptureMediaSource or something similar?

Yeah, I like it. I think "External" made more sense when we expected most consumers would need it directly. However, with the addition of Camera2MediaSource, most people may use this instead. Will rename

izzytwosheds · 2022-12-27T18:57:54Z

litr-demo/src/main/java/com/linkedin/android/litr/demo/RecordCamera2Fragment.kt

+
+private const val DEFAULT_CAMERA_FPS = 30
+private const val DEFAULT_TARGET_BITRATE = 5_000_000 // 5Mbps
+private const val DEFAULT_RECORD_WIDTH = 1280


Commonly, video size is defined as resolution (HD, FHD, 4K, etc.) + aspect ratio (square, letterbox, wide screen, etc.) I usually use that convention.

izzytwosheds · 2022-12-27T18:59:18Z

litr-demo/src/main/java/com/linkedin/android/litr/demo/RecordCamera2Fragment.kt

+
+private const val DEFAULT_CAMERA_FPS = 30
+private const val DEFAULT_TARGET_BITRATE = 5_000_000 // 5Mbps
+private const val DEFAULT_RECORD_WIDTH = 1280


Again, this is a fragment, not public LiTr API, so we can do whatever here.

izzytwosheds · 2022-12-27T19:01:10Z

litr/src/main/java/com/linkedin/android/litr/io/Camera2MediaSource.kt

+     * Since we don't support cropping of the sensor or input surface, we will record to a Surface
+     * that has the same aspect ratio as the camera's sensor.
+     */
+    private fun getRecordSize(sensorRect: Rect?, sensorOrientation: Int?): Size {


izzytwosheds · 2022-12-27T19:01:50Z

litr/src/main/java/com/linkedin/android/litr/io/CaptureMediaSource.kt

+private const val TAG = "CaptureMediaSource"
+private const val DECODER_NAME = "CaptureMediaSource.Decoder"
+
+private const val MIME_TYPE_RAW = "video/raw"


You can add this to MimeType class

Will do. I originally resisted because it's not really a "standard" mime type, but happy to move.

IanDBird · 2022-12-28T11:44:39Z

@izzytwosheds - That's me pushed a change to move the MimeType. If you're happy with the current PR, maybe we could land and look at any tweaks in some follow up PRs?

IanDBird commented Dec 26, 2022

View reviewed changes

izzytwosheds reviewed Dec 26, 2022

View reviewed changes

IanDBird force-pushed the external-record branch 5 times, most recently from ca4d1e1 to 79caab9 Compare December 27, 2022 10:52

izzytwosheds approved these changes Dec 27, 2022

View reviewed changes

Implement Camera2 support as a MediaSource

d199abd

IanDBird force-pushed the external-record branch from 79caab9 to d199abd Compare December 28, 2022 11:43

izzytwosheds approved these changes Dec 28, 2022

View reviewed changes

izzytwosheds merged commit 3304f72 into linkedin:main Dec 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Camera2 support as a MediaSource #232

Implement Camera2 support as a MediaSource #232

IanDBird commented Dec 26, 2022

IanDBird Dec 26, 2022

izzytwosheds Dec 26, 2022

IanDBird Dec 27, 2022

izzytwosheds Dec 27, 2022

IanDBird Dec 26, 2022

izzytwosheds Dec 26, 2022

IanDBird Dec 26, 2022

izzytwosheds Dec 26, 2022

IanDBird Dec 27, 2022

izzytwosheds Dec 27, 2022

izzytwosheds Dec 27, 2022

izzytwosheds Dec 26, 2022

IanDBird Dec 27, 2022

izzytwosheds Dec 26, 2022

IanDBird Dec 27, 2022

izzytwosheds Dec 26, 2022

IanDBird Dec 27, 2022

izzytwosheds Dec 27, 2022

izzytwosheds Dec 26, 2022

izzytwosheds Dec 26, 2022

IanDBird Dec 27, 2022

izzytwosheds Dec 26, 2022

IanDBird Dec 27, 2022

IanDBird Dec 27, 2022

izzytwosheds Dec 26, 2022

izzytwosheds Dec 26, 2022

izzytwosheds Dec 26, 2022

IanDBird Dec 27, 2022

izzytwosheds Dec 27, 2022

izzytwosheds Dec 27, 2022

izzytwosheds Dec 27, 2022

izzytwosheds Dec 27, 2022

IanDBird Dec 28, 2022

IanDBird commented Dec 28, 2022

Implement Camera2 support as a MediaSource #232

Implement Camera2 support as a MediaSource #232

Conversation

IanDBird commented Dec 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IanDBird commented Dec 28, 2022