Record screen to file #292

igorinov · 2018-10-09T02:25:14Z

This change enables writing AV packets received from the target to a video file without re-encoding.

npes87184

FYI, I have some advice about insignificant little problems.

npes87184 · 2018-10-09T03:05:38Z

app/src/main.c

@@ -207,14 +211,15 @@ static SDL_bool parse_args(struct args *args, int argc, char *argv[]) {
        {"fullscreen",   no_argument,       NULL, 'f'},
        {"help",         no_argument,       NULL, 'h'},
        {"max-size",     required_argument, NULL, 'm'},
+        {"output",       required_argument, NULL, 'o'},


In my opinion, the output is too general. In the future, we may add some feature like output format. It make a bit confusing.

--output-file?

npes87184 · 2018-10-09T03:07:09Z

app/src/main.c

@@ -235,6 +240,9 @@ static SDL_bool parse_args(struct args *args, int argc, char *argv[]) {
                    return SDL_FALSE;
                }
                break;
+            case 'o':
+                args->outfilename = optarg;


To make consistent, use underscore as delimiter.

npes87184 · 2018-10-09T03:08:25Z

app/src/scrcpy.c

@@ -228,7 +228,7 @@ SDL_bool scrcpy(const struct scrcpy_options *options) {
    }

    ret = event_loop();
-    LOGD("quit...");
+    LOGI("quit...");


In my opinion, LOGD is good enough.

rom1v

Thank you for your contribution.

I compiled it (after changing CODEC_FLAG_GLOBAL_HEADER, which does not exist in my FFmpeg, to AV_CODEC_FLAG_GLOBAL_HEADER) and run it with scrcpy -o test.h264.

A video file is correctly recorded, and can be played with VLC or MPV. That's great an interesting 👍

However, the framerate of the video stream received from the device is (very) variable, and is not respected in the resulting video file. For example, if I navigate to a screen which is "constant" (no pixel is changed) and stay on this screen for 10 seconds, then navigate to another application, in the resulting video file, the 10 seconds are not preserved (it immediately switches to the other application).

This is confirmed by the warning given by mpv:

[lavf] This format is marked by FFmpeg as having no timestamps!
[lavf] FFmpeg will likely make up its own broken timestamps. For
[lavf] video streams you can correct this with:
[lavf]     --no-correct-pts --fps=VALUE
[lavf] with VALUE being the real framerate of the stream. You can
[lavf] expect seeking and buffering estimation to be generally
[lavf] broken as well.

(and by the seekbar behavior both in VLC and mpv)

So to preserve timestamps, the H.264 raw stream should be muxed into a container like mkv or mp4 I guess.

Writing to a file is blocking, so ideally, it should be done in a thread separated from the decoder thread (otherwise it can block/slow down decoding, resulting in frame drop). It does not matter as much as if it were the UI thread though.

As a side note, recording on the client side will include the jitter from communication from the server to the client (see #21 (comment)). Maybe it would be "better" to mux on the server side (only when recording is enabled to avoid additional latency when nothing is recorded). What do you think?

Also, what could we do on device rotation?

igorinov · 2018-10-10T03:32:31Z

Thank you! I am adding muxing to the server, but it only supports Android 8.0+ because MediaMuxer(FileDescriptor fd, int format) is not available in earlier versions. While MPV correctly handles frame size changes in the same video file, creating a new file on rotation may be a better option.

rom1v · 2018-10-10T08:27:04Z

but it only supports Android 8.0+ because MediaMuxer(FileDescriptor fd, int format) is not available in earlier versions

In addition, it says that the "File descriptor must be seekable", which is not the case. It seems it cannot mux into a container that can be streamed in live.

Recording on the client side is still acceptable to me, it just makes me sad that people will use it for recording video of games or whatever, while adb screenrecord /sdcard/file.mp4 would give a better result (timestamps would be more accurate).

But this would still be a great feature to be able to record to mkv locally.

While MPV correctly handles frame size changes in the same video file

Frame size changes in the same video file will probably cause problems (mkv only defines the video size once; however VLC should still handle frame size change properly in theory). screenrecord keeps the original frame size, and resize the content inside this rectangle, which is not great but works.

creating a new file on rotation may be a better option

Yes, but it would require a pattern to generate new files (instead of just a filename).

Why do you think about just storing the video rotated? For example, if we start recording in 1200x720, then rotate to 720x1200, the video would be recorded "rotated" in 1200x720? The first frame would define the dimensions. (This is not that simple though, in theory, the frame size may change to arbitrary dimensions)

igorinov · 2018-10-11T05:24:16Z

In my latest experiment, the server sends headers with packet size and PTS before encoded packets:
https://github.com/igorinov/scrcpy/tree/server-timestamp
The output file looks good in MPV but libav prints too many warnings about incorrect PTS while scrcpy is running.

I like the idea about recording rotated video to keep constant frame size, but is it possible without re-encoding?

rom1v · 2018-10-12T19:34:25Z

(just acking your message, I'll try to review it next week, thank you)

rom1v · 2018-10-25T15:54:03Z

@igorinov I just tested your server-timestamp branch. I think it's a good idea to retrieve the PTS like you did if recording is requested.

It records a file, but unfortunately, the timestamps are still not respected in the resulting file (like #292 (review)). IMO we need to mux in a container like mp4 or mkv.

rom1v · 2018-10-25T16:14:29Z

It records a file, but unfortunately, the timestamps are still not respected in the resulting file

Ah, that's because I saved to test.h264. If I save to test.mp4, timestamps are respected 🎉
However, the mp4 takes a lot of time to open in VLC. And saving to test.mkv does not work.

(Is it possible to specify the target container explicitly?)

igorinov · 2018-11-01T18:27:12Z

I think it should be recognized by extension, if CONFIG_MATROSKA_MUXER is enabled in the build configuration:

AVOutputFormat ff_matroska_muxer = {
.name = "matroska",
.long_name = NULL_IF_CONFIG_SMALL("Matroska"),
.mime_type = "video/x-matroska",
.extensions = "mkv",

igorinov · 2018-11-01T18:31:11Z

I tried recording to .mkv on a Linux machine, and the file was played successfully with cvlc.

rom1v · 2018-11-03T18:05:51Z

I tried recording to .mkv on a Linux machine, and the file was played successfully with cvlc.

Arf, on Debian sid, I cannot record to mkv :(

I like your branch, and it would be great to get recording into scrcpy. But there are still some issues to solve.

If I record to mp4, the resulting mp4 file takes a very long time to load in vlc, so it is not "proper" (while a file recorded by adb screenrecord works fine). I don't know what the cause is. Any idea?

If I take your current branch, (after I changed CODEC_FLAG_GLOBAL_HEADER to AV_CODEC_FLAG_GLOBAL_HEADER), I get these warnings:

../app/src/decoder.c: In function ‘run_decoder’:
../app/src/decoder.c:145:13: warning: ‘codec’ is deprecated [-Wdeprecated-declarations]
             outstream->codec = avcodec_alloc_context3(codec);
             ^~~~~~~~~
In file included from ../app/src/decoder.c:3:
/usr/include/x86_64-linux-gnu/libavformat/avformat.h:877:21: note: declared here
     AVCodecContext *codec;
                     ^~~~~
../app/src/decoder.c:146:13: warning: ‘codec’ is deprecated [-Wdeprecated-declarations]
             outstream->codec->pix_fmt = AV_PIX_FMT_YUV420P;
             ^~~~~~~~~
In file included from ../app/src/decoder.c:3:
/usr/include/x86_64-linux-gnu/libavformat/avformat.h:877:21: note: declared here
     AVCodecContext *codec;
                     ^~~~~
../app/src/decoder.c:147:13: warning: ‘codec’ is deprecated [-Wdeprecated-declarations]
             outstream->codec->width = decoder->frame_size.width;
             ^~~~~~~~~
In file included from ../app/src/decoder.c:3:
/usr/include/x86_64-linux-gnu/libavformat/avformat.h:877:21: note: declared here
     AVCodecContext *codec;
                     ^~~~~
../app/src/decoder.c:148:13: warning: ‘codec’ is deprecated [-Wdeprecated-declarations]
             outstream->codec->height = decoder->frame_size.height;
             ^~~~~~~~~
In file included from ../app/src/decoder.c:3:
/usr/include/x86_64-linux-gnu/libavformat/avformat.h:877:21: note: declared here
     AVCodecContext *codec;
                     ^~~~~
../app/src/decoder.c:150:13: warning: ‘codec’ is deprecated [-Wdeprecated-declarations]
             outstream->codec->flags |= AV_CODEC_FLAG_GLOBAL_HEADER;
             ^~~~~~~~~
In file included from ../app/src/decoder.c:3:
/usr/include/x86_64-linux-gnu/libavformat/avformat.h:877:21: note: declared here
     AVCodecContext *codec;
                     ^~~~~

I have no time to investigate on this currently. I'll try to find some time later. If you have some time and find how to fix these issues, that would be awesome 😉

Thank you.

rom1v · 2018-11-04T12:00:21Z

If I record to mp4, the resulting mp4 file takes a very long time to load in vlc

I just found the problem: the PTS given by bufferInfo.presentationTimeUs on the server is the current timestamp (since epoch). To fix this issue, the timestamp of the first frame must be 0, and all others must be relative to this one (in other words, we must substract the first presentationTimeUs from all PTS).

There are still little problems, for example on quit:

Application provided invalid, non monotonically increasing dts to muxer in stream 0: 3347160 >= 3347160

rom1v · 2018-11-04T13:11:49Z

I tried recording to .mkv on a Linux machine, and the file was played successfully with cvlc.

For me, it fails at avformat_write_header(output_ctx, NULL) which returns AVERROR_INVALIDDATA.

rom1v · 2018-11-09T15:38:39Z

@igorinov I had some time today to work on it.

I started from your commit, and did these changes:

I created a new "class" recorder, so that the recording is not implemented directly by the decoder.
I use the non-deprecated API (AVCodecParameters) to pass codec params to the output AVStream.
I explicitly select the MP4 muxer (I don't let FFmpeg choose based on the filename, some muxers won't record the video properly, and for some reason I can't write header for matroska muxer, cf Record screen to file #292 (comment)).
I use 1/1000000 as time base (since the input provide timestamps in microseconds).
I fixed PTS (as explained in Record screen to file #292 (comment)).
I set both PTS/DTS to AVPacket before writing it (to avoid FFmpeg warnings).
I don't write "config" packets (when bufferInfo.flags has MEDIA_CODEC_FLAG_CONFIG) to output file since they contain no data and do not have any PTS/DTS.
I handle display (decode + push frame) before recording (to reduce latency).

Here is my branch: record.

scrcpy -o file.mp4

It correctly records rotation, and plays correctly in VLC 😄

Remaining things I will work on:

~~have absolutely no impact on perfs if recording is not enabled (e.g. provide the original read_packet() function if we do not record).~~
~~handle the case where we receive only part of the header in read_packet().~~
~~also use AVStream deprecated API (the one you used) in ifdefs to support older FFmpeg versions.~~
~~also use deprecated av_oformat_next() in ifdefs (in addition to the new av_muxer_iterate()) to support older FFmpeg versions.~~
~~fix the warning on exit: Application provided invalid, non monotonically increasing dts to muxer in stream 0: 17164020 >= 17164020.~~

rom1v · 2018-11-09T20:26:44Z

fix the warning on exit: Application provided invalid, non monotonically increasing dts to muxer in stream 0: 17164020 >= 17164020.

Oh, all PTS are not assigned to the right frame, we immediately use the last PTS read for the current decoded frame.

EDIT: fixed by 0cb16be
EDIT2: not fixed: several calls to read_packet() may occur even before the first av_read_frame() is called, which lead to assign wrong PTS to frames (to reproduce, record a small static region with something like --crop 200:200:0:0).

npes87184 · 2018-11-10T02:24:42Z

Hi, @rom1v

I cloned the branch record and tried to build.
I met the building error.

../app/src/recorder.c: In function ‘find_mp4_muxer’:
../app/src/recorder.c:13:19: warning: implicit declaration of function ‘av_muxer_iterate’; did you mean ‘av_buffer_c
reate’? [-Wimplicit-function-declaration]
         oformat = av_muxer_iterate(&opaque);
                   ^~~~~~~~~~~~~~~~
                   av_buffer_create
../app/src/recorder.c:13:17: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
         oformat = av_muxer_iterate(&opaque);
                 ^
[31/32] Linking target app/scrcpy.
FAILED: app/scrcpy
cc  -o app/scrcpy 'app/scrcpy@exe/src_main.c.o' 'app/scrcpy@exe/src_command.c.o' 'app/scrcpy@exe/src_control_event.c
.o' 'app/scrcpy@exe/src_controller.c.o' 'app/scrcpy@exe/src_convert.c.o' 'app/scrcpy@exe/src_decoder.c.o' 'app/scrcp
y@exe/src_device.c.o' 'app/scrcpy@exe/src_file_handler.c.o' 'app/scrcpy@exe/src_fps_counter.c.o' 'app/scrcpy@exe/src
_frames.c.o' 'app/scrcpy@exe/src_input_manager.c.o' 'app/scrcpy@exe/src_lock_util.c.o' 'app/scrcpy@exe/src_net.c.o'
'app/scrcpy@exe/src_recorder.c.o' 'app/scrcpy@exe/src_scrcpy.c.o' 'app/scrcpy@exe/src_screen.c.o' 'app/scrcpy@exe/sr
c_server.c.o' 'app/scrcpy@exe/src_str_util.c.o' 'app/scrcpy@exe/src_tiny_xpm.c.o' 'app/scrcpy@exe/src_sys_unix_comma
nd.c.o' 'app/scrcpy@exe/src_sys_unix_net.c.o' -flto -Wl,--no-undefined -Wl,--as-needed -Wl,-O1 -Wl,--start-group -la
vformat -lavcodec -lavutil -lSDL2 -Wl,--end-group
/tmp/ccxOcwQJ.ltrans0.ltrans.o: In function `run_decoder':
<artificial>:(.text+0x1020): undefined reference to `av_muxer_iterate'
collect2: error: ld returned 1 exit status

/usr/include/x86_64-linux-gnu/libavformat » dpkg -s libavformat-dev                            
Package: libavformat-dev                                                               
Status: install ok installed                                
Priority: optional                                                                                                 
Section: libdevel
Installed-Size: 4985
Maintainer: Ubuntu Developers <[email protected]>
Architecture: amd64
Multi-Arch: same
Source: ffmpeg
Version: 7:3.4.4-0ubuntu0.18.04.1
Depends: libavcodec-dev (= 7:3.4.4-0ubuntu0.18.04.1), libavformat57 (= 7:3.4.4-0ubuntu0.18.04.1), libavutil-dev (= 7
:3.4.4-0ubuntu0.18.04.1), libswresample-dev (= 7:3.4.4-0ubuntu0.18.04.1)

Thanks,
yuchenlin

rom1v · 2018-11-10T08:27:18Z

@npes87184 Thank you for your test.

I cloned the branch record and tried to build.
I met the building error.

Yes, that's the reason I planned to:

also use deprecated av_oformat_next() in ifdefs (in addition to the new av_muxer_iterate()) to support older FFmpeg versions.

😉

npes87184 · 2018-11-10T11:05:47Z

Hi, @rom1v

Thank you for your explanation.
According to your explanation, to fix building error, I simply modify the find_mp4_muxer by this commit recorder: use av_oformat_next to support older FFmpeg and it works.

At the first glance, it works like a charm!
However, when scrcpy receive SIGTERM the mp4 file cannot be played.

Thanks,
yuchenlin

rom1v · 2018-11-10T11:45:24Z

However, when scrcpy receive SIGTERM the mp4 file cannot be played.

Ah yes, this need to be fixed too. Thank you.

Basically, reverting f00c6c5 works, but we need to fix what this commit prevented.

npes87184 · 2018-11-10T12:11:24Z

Or set a flag to call recorder_close when we receive SIGTERM?

rom1v · 2018-11-10T13:23:15Z

@npes87184 c2cb069 😉

rom1v · 2018-11-11T14:54:09Z

I think we have a version ready to be merged: branch record.

I also changed the option name:

scrcpy --record file.mp4
scrcpy -r file.mp4

If you want to test before the merge/release, feedbacks are welcome :)

Record screen to file

rom1v · 2018-11-11T22:40:26Z

I merged my record branch into master, and published v1.5 with screen recording 🎉

Thank you very much @igorinov for your initial work, this provided an awesome working base. Your commit is merged as d706c5d.

Enable video file output

ebc1f2d

npes87184 reviewed Oct 9, 2018

View reviewed changes

rom1v reviewed Oct 9, 2018

View reviewed changes

rom1v mentioned this pull request Oct 19, 2018

Screenshot/Record Screen #21

Closed

rom1v mentioned this pull request Nov 5, 2018

[Suggestion] Using a webclient instead sdl app. #313

Open

rom1v changed the title ~~Enable video file output~~ Record video to file Nov 9, 2018

rom1v changed the title ~~Record video to file~~ Record screen to file Nov 11, 2018

rom1v added a commit that referenced this pull request Nov 11, 2018

Merge branch 'record' into dev (#292)

77b620e

Record screen to file

rom1v closed this Nov 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Record screen to file #292

Record screen to file #292

igorinov commented Oct 9, 2018

npes87184 left a comment

npes87184 Oct 9, 2018

igorinov Oct 10, 2018

npes87184 Oct 9, 2018

igorinov Oct 10, 2018

npes87184 Oct 9, 2018

rom1v left a comment

igorinov commented Oct 10, 2018

rom1v commented Oct 10, 2018

igorinov commented Oct 11, 2018

rom1v commented Oct 12, 2018

rom1v commented Oct 25, 2018

rom1v commented Oct 25, 2018

igorinov commented Nov 1, 2018

igorinov commented Nov 1, 2018

rom1v commented Nov 3, 2018

rom1v commented Nov 4, 2018 •

edited

Loading

rom1v commented Nov 4, 2018

rom1v commented Nov 9, 2018 •

edited

Loading

rom1v commented Nov 9, 2018 •

edited

Loading

npes87184 commented Nov 10, 2018 •

edited

Loading

rom1v commented Nov 10, 2018

npes87184 commented Nov 10, 2018

rom1v commented Nov 10, 2018 •

edited

Loading

npes87184 commented Nov 10, 2018

rom1v commented Nov 10, 2018

rom1v commented Nov 11, 2018 •

edited

Loading

rom1v commented Nov 11, 2018 •

edited

Loading

Record screen to file #292

Record screen to file #292

Conversation

igorinov commented Oct 9, 2018

npes87184 left a comment

Choose a reason for hiding this comment

npes87184 Oct 9, 2018

Choose a reason for hiding this comment

igorinov Oct 10, 2018

Choose a reason for hiding this comment

npes87184 Oct 9, 2018

Choose a reason for hiding this comment

igorinov Oct 10, 2018

Choose a reason for hiding this comment

npes87184 Oct 9, 2018

Choose a reason for hiding this comment

rom1v left a comment

Choose a reason for hiding this comment

igorinov commented Oct 10, 2018

rom1v commented Oct 10, 2018

igorinov commented Oct 11, 2018

rom1v commented Oct 12, 2018

rom1v commented Oct 25, 2018

rom1v commented Oct 25, 2018

igorinov commented Nov 1, 2018

igorinov commented Nov 1, 2018

rom1v commented Nov 3, 2018

rom1v commented Nov 4, 2018 • edited Loading

rom1v commented Nov 4, 2018

rom1v commented Nov 9, 2018 • edited Loading

rom1v commented Nov 9, 2018 • edited Loading

npes87184 commented Nov 10, 2018 • edited Loading

rom1v commented Nov 10, 2018

npes87184 commented Nov 10, 2018

rom1v commented Nov 10, 2018 • edited Loading

npes87184 commented Nov 10, 2018

rom1v commented Nov 10, 2018

rom1v commented Nov 11, 2018 • edited Loading

rom1v commented Nov 11, 2018 • edited Loading

rom1v commented Nov 4, 2018 •

edited

Loading

rom1v commented Nov 9, 2018 •

edited

Loading

rom1v commented Nov 9, 2018 •

edited

Loading

npes87184 commented Nov 10, 2018 •

edited

Loading

rom1v commented Nov 10, 2018 •

edited

Loading

rom1v commented Nov 11, 2018 •

edited

Loading

rom1v commented Nov 11, 2018 •

edited

Loading