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

Flac audio codec #4410

Closed
wants to merge 4 commits into from
Closed

Flac audio codec #4410

wants to merge 4 commits into from

Conversation

megapro17
Copy link
Contributor

I didn't tested it, compiling is complicated
It would also be nice to have an 8 compression ratio

@rom1v
Copy link
Collaborator

rom1v commented Nov 7, 2023

Refs #3757 (comment)

Small fix to compile:

diff --git a/app/src/demuxer.c b/app/src/demuxer.c
index ca27c0bc1..69d0c42b7 100644
--- a/app/src/demuxer.c
+++ b/app/src/demuxer.c
@@ -26,7 +26,7 @@ sc_demuxer_to_avcodec_id(uint32_t codec_id) {
 #define SC_CODEC_ID_AV1 UINT32_C(0x00617631) // "av1" in ASCII
 #define SC_CODEC_ID_OPUS UINT32_C(0x6f707573) // "opus" in ASCII
 #define SC_CODEC_ID_AAC UINT32_C(0x00616163) // "aac in ASCII"
-#define SC_CODEC_ID_AAC UINT32_C(0x664c6143) // "fLaC in ASCII"
+#define SC_CODEC_ID_FLAC UINT32_C(0x664c6143) // "fLaC in ASCII"
 #define SC_CODEC_ID_RAW UINT32_C(0x00726177) // "raw" in ASCII
     switch (codec_id) {
         case SC_CODEC_ID_H264:
diff --git a/app/src/options.h b/app/src/options.h
index 5a37b1311..914338946 100644
--- a/app/src/options.h
+++ b/app/src/options.h
@@ -33,7 +33,7 @@ sc_record_format_is_audio_only(enum sc_record_format fmt) {
     return fmt == SC_RECORD_FORMAT_M4A
         || fmt == SC_RECORD_FORMAT_MKA
         || fmt == SC_RECORD_FORMAT_OPUS
-        || fmt == SC_RECORD_FORMAT_AAC;
+        || fmt == SC_RECORD_FORMAT_AAC
         || fmt == SC_RECORD_FORMAT_FLAC;
 }
 

But at runtime, I get:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==190749==ERROR: AddressSanitizer: FPE on unknown address 0x7fe68841b808 (pc 0x7fe68841b808 bp 0x000000001000 sp 0x7fe619ea9520 T25)
    #0 0x7fe68841b808  (/lib/x86_64-linux-gnu/libswresample.so.4+0x10808) (BuildId: ddc2f6bd89b08150e9399afd21a7bd46803246da)
    #1 0x7fe68841c4a5  (/lib/x86_64-linux-gnu/libswresample.so.4+0x114a5) (BuildId: ddc2f6bd89b08150e9399afd21a7bd46803246da)
    #2 0x7fe68841d519 in swr_convert (/lib/x86_64-linux-gnu/libswresample.so.4+0x12519) (BuildId: ddc2f6bd89b08150e9399afd21a7bd46803246da)
    #3 0x563d190dee3c in sc_audio_player_frame_sink_push ../app/src/audio_player.c:156
    #4 0x563d1911817c in sc_frame_source_sinks_push ../app/src/trait/frame_source.c:53
    #5 0x563d190ec708 in sc_decoder_push ../app/src/decoder.c:66
    #6 0x563d190ec7d8 in sc_decoder_packet_sink_push ../app/src/decoder.c:94
    #7 0x563d1911881b in sc_packet_source_sinks_push ../app/src/trait/packet_source.c:53
    #8 0x563d190ef615 in run_demuxer ../app/src/demuxer.c:257
    #9 0x7fe6860c3408  (/lib/x86_64-linux-gnu/libSDL2-2.0.so.0+0xa4408) (BuildId: 212fce418ad3777e67ef93d7a228ba10d4e8fb51)
    #10 0x7fe686168fec  (/lib/x86_64-linux-gnu/libSDL2-2.0.so.0+0x149fec) (BuildId: 212fce418ad3777e67ef93d7a228ba10d4e8fb51)
    #11 0x7fe685ca63eb in start_thread nptl/pthread_create.c:444
    #12 0x7fe685d26a4b in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: FPE (/lib/x86_64-linux-gnu/libswresample.so.4+0x10808) (BuildId: ddc2f6bd89b08150e9399afd21a7bd46803246da) 
Thread T25 created by T0 here:
    #0 0x7fe687c47c26 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:208
    #1 0x7fe68616905e  (/lib/x86_64-linux-gnu/libSDL2-2.0.so.0+0x14a05e) (BuildId: 212fce418ad3777e67ef93d7a228ba10d4e8fb51)

==190749==ABORTING

To be investigated.

EDIT: ctx->sample_fmt is unexpectedly -1.

(Also, an argument to set the compression compression should maybe be added?)

@megapro17
Copy link
Contributor Author

maybe this?

@@ -25,7 +25,7 @@ enum sc_record_format {
     SC_RECORD_FORMAT_MKA,
     SC_RECORD_FORMAT_OPUS,
     SC_RECORD_FORMAT_AAC,
-    SC_RECORD_FORMAT_FLAC,
+    SC_RECORD_FORMAT_FLAC;
 };

@megapro17
Copy link
Contributor Author

(Also, an argument to set the compression compression should maybe be added?)

would --audio-codec-options=KEY_FLAC_COMPRESSION_LEVEL=8 work?

@rom1v
Copy link
Collaborator

rom1v commented Nov 7, 2023

==190749==ERROR: AddressSanitizer: FPE on unknown address 0x7fe68841b808 (pc 0x7fe68841b808 bp 0x000000001000 sp 0x7fe619ea9520 T25)

If I hardcode this value, it "works":

diff --git a/app/src/demuxer.c b/app/src/demuxer.c
index 69d0c42b7..520ac609e 100644
--- a/app/src/demuxer.c
+++ b/app/src/demuxer.c
@@ -210,6 +210,7 @@ run_demuxer(void *data) {
         codec_ctx->channels = 2;
 #endif
         codec_ctx->sample_rate = 48000;
+        codec_ctx->sample_fmt = AV_SAMPLE_FMT_S16;
     }
 
     if (avcodec_open2(codec_ctx, codec, NULL) < 0) {

But another problem is that the flac encoder is not low latency at all: it produces blocks of 4096 samples, which represent ~85.333ms.

So to make it work, the audio latency must be increased:

scrcpy --audio-codec=flac --audio-buffer=200

And recording to mp4 does not work (mp4 does not support flac), only mkv works.

😕

@megapro17
Copy link
Contributor Author

megapro17 commented Nov 7, 2023

can you share binary for windows? even without ffmpeg dlls

mp4 does not support flac

it does, opus too as well as vp9. by the official mpeg specification. most players can play these files fine. ffmpeg can create it without problems

flac encoder is not low latency at all

seems like there's no any other lossless audio encoder built in android... i guess you don't need low latency when recording to a file

@rom1v
Copy link
Collaborator

rom1v commented Nov 7, 2023

can you share binary for windows? even without ffmpeg dlls

  • scrcpy-flac.zip SHA-256: d740ce526d2ed66939d531a2447306e67a25e6a3f231ff0a0651b820298e1e43

(only the .exe and the server, not tested)

mp4 does not support flac

it does, opus too. by the official mpeg specification. most players can play these files fine. ffmpeg can create it without problems

For some reason, the call to av_write_trailer() returns AVERROR_INVALIDDATA:

int ret = av_write_trailer(recorder->ctx);

@megapro17
Copy link
Contributor Author

It does play audio, but it doesn't record it. After recording, file is broken. It seems like it has two fLaC signatures. If i remove the first one, it doesn't show errors, but still can't play, because length is 0 seconds
image

[flac @ 0000023fcb0b1980] extradata too small.
[matroska,webm @ 0000023fcb0ae780] Failed to open codec in avformat_find_stream_info
[flac @ 0000023fcb0b1980] extradata too small.
[matroska,webm @ 0000023fcb0ae780] Could not find codec parameters for stream 1 (Audio: flac, 48000 Hz, 0 channels): unspecified number of channels
Consider increasing the value for the 'analyzeduration' (0) and 'probesize' (5000000) options
[aist#0:1/flac @ 0000023fcb0fcb00] Guessed Channel Layout: stereo
Input #0, matroska,webm, from '1.mkv':
  Metadata:
    COMMENT         : Recorded by scrcpy v2.2
    ENCODER         : Lavf60.17.100
  Duration: 00:00:07.84, start: 0.000000, bitrate: 3906 kb/s
  Stream #0:0: Video: h264 (High), yuv420p(tv, bt470bg/bt470bg/smpte170m, progressive), 1440x3088, 1k tbr, 1k tbn
    Metadata:
      DURATION        : 00:00:07.836000000
  Stream #0:1: Audio: flac, 48000 Hz, 2 channels
    Metadata:
      DURATION        : 00:00:07.686000000
At least one output file must be specified

i removed first fLaC from mkv and this file can be converted to wav...

@rom1v
Copy link
Collaborator

rom1v commented Nov 8, 2023

OK, so I guess the extradata produced by MediaCodec does not have the format expected by FFmpeg, so it has to be transformed, like we do for opus (but it looks like it's yet another format):

// Here is an example of the config packet received for an OPUS stream:
//
// 00000000 41 4f 50 55 53 48 44 52 13 00 00 00 00 00 00 00 |AOPUSHDR........|
// -------------- BELOW IS THE PART WE MUST PUT AS EXTRADATA -------------------
// 00000010 4f 70 75 73 48 65 61 64 01 01 38 01 80 bb 00 00 |OpusHead..8.....|
// 00000020 00 00 00 |... |
// ------------------------------------------------------------------------------
// 00000020 41 4f 50 55 53 44 4c 59 08 00 00 00 00 | AOPUSDLY.....|
// 00000030 00 00 00 a0 2e 63 00 00 00 00 00 41 4f 50 55 53 |.....c.....AOPUS|
// 00000040 50 52 4c 08 00 00 00 00 00 00 00 00 b4 c4 04 00 |PRL.............|
// 00000050 00 00 00 |...|
//
// Each "section" is prefixed by a 64-bit ID and a 64-bit length.
//
// <https://developer.android.com/reference/android/media/MediaCodec#CSD>

EDIT: for reference, the config packet produced by MediaCodec on my test device:

00000000  66 4c 61 43 00 00 00 22  10 00 10 00 00 00 00 00  |fLaC..."........|
00000010  00 00 0b b8 02 f0 00 00  00 00 00 00 00 00 00 00  |................|
00000020  00 00 00 00 00 00 00 00  00 00 84 00 00 28 20 00  |.............( .|
00000030  00 00 72 65 66 65 72 65  6e 63 65 20 6c 69 62 46  |..reference libF|
00000040  4c 41 43 20 31 2e 33 2e  32 20 32 30 32 32 31 30  |LAC 1.3.2 202210|
00000050  32 32 00 00 00 00                                 |22....|
00000056

EDIT: I confirm that if we cut only this part in the config packet, it works (I can record to mp4 properly now):

00000000  66 4c 61 43 00 00 00 22                           |fLaC..."        |
--- 8< -----------------------------------------------------------------------
00000000                           10 00 10 00 00 00 00 00  |        ........|
00000010  00 00 0b b8 02 f0 00 00  00 00 00 00 00 00 00 00  |................|
00000020  00 00 00 00 00 00 00 00  00 00                    |..........      |
--- 8< -----------------------------------------------------------------------
00000020                                 84 00 00 28 20 00  |          ...( .|
00000030  00 00 72 65 66 65 72 65  6e 63 65 20 6c 69 62 46  |..reference libF|
00000040  4c 41 43 20 31 2e 33 2e  32 20 32 30 32 32 31 30  |LAC 1.3.2 202210|
00000050  32 32 00 00 00 00                                 |22....|
00000056

(I have not done it properly, I just hardcoded to test)

rom1v added a commit that referenced this pull request Nov 12, 2023
PR #4410 <##4410>

Co-authored-by: Romain Vimont <rom@rom1v.com>
Signed-off-by: Romain Vimont <rom@rom1v.com>
@rom1v
Copy link
Collaborator

rom1v commented Nov 12, 2023

I made the following changes:

  • rebased on dev
  • generated FFmpeg build with FLAC support for Windows (and upgraded to FFmpeg 6.1 released yesterday)
  • fixed flac config packet for ffmpeg
  • automatically increased the default audio buffer when flac is used
  • updated documentation

Branch: flac.3 flac.4

Here is a full release of branch flac.3:

  • scrcpy-flac.zip SHA-256: 0305560795e4d4ebf465d3d8b8a5a0ad3c112ba9131736a723bc7fee91aeae25

Please review and test.

rom1v added a commit that referenced this pull request Nov 12, 2023
PR #4410 <##4410>

Co-authored-by: Romain Vimont <rom@rom1v.com>
Signed-off-by: Romain Vimont <rom@rom1v.com>
@megapro17
Copy link
Contributor Author

Wow, nice work! Everything works now, audio is recorded and played without any problems. But the file has no duration, to fix it I have to convert the file. But isn't that fixConfigPacket for each codec is looks weird? Maybe there is a more elegant solution.....

@rom1v
Copy link
Collaborator

rom1v commented Nov 14, 2023

But the file has no duration, to fix it I have to convert the file.

Indeed, when a FLAC audio stream is muxed to flac (.flac file), there is no global duration.

However, for an OPUS audio stream muxed to opus (.opus file), or an AAC audio stream muxed to mp4 (.aac), it works correctly, the global duration is correctly written.

Similarly, when a FLAC audio stream is muxed to MP4 (.mp4) or Matroska (.mkv), it also works.

So I guess this is related to the flac muxer, it does not write the duration when writing the trailer. I don't know if we can pass some more configuration so that it correctly writes the duration.

Related code: https://github.com/FFmpeg/FFmpeg/blob/6d60cc7baf662b64e3c50f95826dead58cf8e839/libavformat/flacenc.c#L320-L346

But isn't that fixConfigPacket for each codec is looks weird?

It's just that MediaCodec wraps the extradata into its own structure (different per codec), so we have to extract the relevant extradata to pass to FFmpeg.

rom1v added a commit that referenced this pull request Nov 14, 2023
PR #4410 <##4410>

Co-authored-by: Romain Vimont <rom@rom1v.com>
Signed-off-by: Romain Vimont <rom@rom1v.com>
@rom1v
Copy link
Collaborator

rom1v commented Nov 14, 2023

On top of these changes, I added support for recording raw audio to .wav or .mkv/.mka (not mp4 since it's not supported).

Branch: wav.6

  • scrcpy-wav.6.zip SHA-256: fbaf241c10a2d640c4100c39bff46cce4ebdc2e70d44e81adbfd119437f8a958
scrcpy --audio-codec=raw --record=file.mkv
scrcpy --audio-codec=raw --no-video --record=file.wav

Please review and test 😉

rom1v added a commit that referenced this pull request Nov 15, 2023
PR #4410 <##4410>

Co-authored-by: Romain Vimont <rom@rom1v.com>
Signed-off-by: Romain Vimont <rom@rom1v.com>
@rom1v
Copy link
Collaborator

rom1v commented Nov 15, 2023

I merged flac and wav support into dev.

If we find a way to fix the missing flac duration, that would be great, but this can still be merged as is.

@rom1v rom1v closed this Nov 15, 2023
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