[RFC] trace: Add android screenrecord collector

[valschneider]

I have a few issues with the screenrecord utility, so I'm posting this here to collect a bit of feedback.

The main problem is that the max video duration is enforced to be 3 minutes. This means that if we want to record more than 3 minutes of screen footage, we'd need to chain screenrecord commands and then assemble the video files. AFAIK, we'd add a package dependency if we want to assemble the files - and if we want to return all of the recordings when get_trace() is called, we'd kinda break the rules as we'd be returning more than one file (or maybe we could return an archive and call it fair ?)

I think there's 3 ways to handle this:

• Create a tiny bash program (i.e. a loop that calls screenrecord enough times to cover the duration) that is fed to busybox
• Have a threading private class that does the above
• Ignore that issue and simply say "sorry can't do more than 3 minutes"

[setrofim]

It's valid to call reset prior to the first invocation of start, so do not assume that self._remote_file will exist on target.

[setrofim]

I think there's 3 ways to handle this:

In the past, we've handled this the second way -- i.e. a separate utility collector running in a thread (e.g. see the logcat collector implementation). That is probably the easiest, and offers the most flexibility; though I would not be opposed to doing this via a bash function in shutils instead. Simply limiting this to 3 minutes is definitely not acceptable.

This means that if we want to record more than 3 minutes of screen footage, we'd need to chain screenrecord commands and then assemble the video files. AFAIK, we'd add a package dependency if we want to assemble the files

I think we should do this opportunistically -- look for the dependency, and if it present, concatenate the parts; but do not require it, and fall back to returning the parts separately in its absence. This is what I'm doing in the pending FPS pull -- using pandas to speed up FPS processing if it is present, but fall back to the slower method if it isn't.

and if we want to return all of the recordings when get_trace() is called, we'd kinda break the rules as we'd be returning more than one file (or maybe we could return an archive and call it fair ?)

OK, so I can think of the couple of ways of handling this off the top of my head.

• Use archive the individual parts, as you suggest. The is probably the simplest, but will make this awkward to use, as you'd almost always want to extract the archive immediately afterwards, and the archiving will add an unnecessary delay.
• Return an index file rather than the video. The index will then contain the paths to the actual video files. This is also going to be awkward to use, but will not introduce the delay of creating the archive.
• We relax the TraceCollector API (which hasn't been officially documented yet anyway) to define that get_trace() returns the path to the trace, but that it is not necessarily a file. The individual video parts can then be accumulated in a subdirectory and it is the path to that subdirectory that is returned.
• Extend the TraceCollector API to handle this some other way..?

[valschneider]

Thanks for the review & comments.

We relax the TraceCollector API (which hasn't been officially documented yet anyway) to define that get_trace() returns the path to the trace, but that it is not necessarily a file. The individual video parts can then be accumulated in a subdirectory and it is the path to that subdirectory that is returned.

I like this solution but it makes get_trace() awkward to use - depending on the TraceCollector implementation, you're expected to give either a file or a directory path.

In the end, I'm not sure if the archive would be that bad - if I take a look at ftrace, when we use Trappy it "extracts" the trace.dat to a trace.txt.

Maybe we could change the TraceCollector API to the following:

get_trace(self, outdir, outfile=None)

Where the trace product would be outputted to the outdir directory with its default (unspecified) name, e.g. 'trace.dat' for ftrace. If the user expects to collect several traces in a run, he can specify the name to not overwrite previous collections - but in that case I think it's fair to say he knows what is being outputted.

The archive thing here is because of screenrecord's 3m limitation, but are there types of traces that would generate several independent files ? In this case that would be problematic.