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

Add support for exporting replays as MP4 #199

Merged
merged 20 commits into from May 25, 2020
Merged

Add support for exporting replays as MP4 #199

merged 20 commits into from May 25, 2020

Conversation

alxbl
Copy link
Collaborator

@alxbl alxbl commented Mar 25, 2020

This is the foundational work to support mp4 export.

There will be several changes that must be done once #187 lands, since I've refactored the player event handler hierarchy to make implementation easier.

Right now the code will live in the player, but my hope is to have this code be part of the PCAP extractor script or as another standalone tool. #188.

image


This Pull Request refactors pyrdp-replay-extractor to pyrdp-convert and adds support for:

  • Decrypt TLS PCAPs with scapy when master secrets are available
  • Convert a PCAP to an MP4
  • Convert a replay to an MP4

It also preserves the OSI Layer 7 (plaintext exported PDU) export capabilities from the previous tool.

Closes #170

@alxbl
Copy link
Collaborator Author

alxbl commented Apr 2, 2020

I rebased this on the pyrdp-replay-rebase branch and master. All that's missing before we can land this:

  • Document how to use PyRDP-convert
  • Modify Mp4EventHandler to support the GDI extension

@alxbl alxbl marked this pull request as ready for review April 2, 2020 17:07
@alxbl alxbl mentioned this pull request Apr 2, 2020
4 tasks
@obilodeau
Copy link
Member

You want me to proceed with the review or you want to fix conflicts first? I don't mind either.

@alxbl
Copy link
Collaborator Author

alxbl commented Apr 15, 2020

Some changes will be required to make GDI work, so reviewing now would be pointless. I'll probably make the changes sometime this week and ping you for review again.

@alxbl
Copy link
Collaborator Author

alxbl commented Apr 15, 2020

Rebased and ready for review.

@obilodeau
Copy link
Member

Handled the conflicts, starting to review $now.

@obilodeau
Copy link
Member

Build failures are due to PyAV not being able to build on Ubuntu 18.04 because its ffmpeg is too old for latest PyAV.

Ubuntu 18.04 carries:

libavcodec-dev/bionic-updates,bionic-security 7:3.4.6-0ubuntu0.18.04.1 amd64
  FFmpeg library with de/encoders for audio/video codecs - development files

PyAV dropped support for ffmpeg < 4.x in 7.0.

Since Ubuntu 18.04 is Ubuntu's latest LTS, I suggest we pin PyAV to latest 6.x+ for the time being. Note that Ubuntu 20.04 is unreleased but even if it would, it would be too new for a last a few months. Objections?

Side note: Coverage tests didn't fail because we don't pull the full package there. I'll adjust it in another PR.

@alxbl
Copy link
Collaborator Author

alxbl commented Apr 16, 2020

I'm fine with pinning PyAV version, although I will need to retest before doing that.

Copy link
Member

@obilodeau obilodeau left a comment

Choose a reason for hiding this comment

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

Very cool work! Nicely integrated. One general comment: should we have a smoke test conversion with pre-built results? Video might not make it but pcap to replay file should do. What do you think?

Otherwise, just a handful of comments.

README.md Outdated Show resolved Hide resolved
bin/pyrdp-convert.py Show resolved Hide resolved
def __iter__(self):
return self

def __next__(self):
Copy link
Member

Choose a reason for hiding this comment

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

This is a large method that makes it hard to approach. Can't we refactor it in logical subparts like _performPduReassembly(), and so on?

bin/pyrdp-convert.py Outdated Show resolved Hide resolved
bin/pyrdp-convert.py Show resolved Hide resolved
pyrdp/player/Mp4EventHandler.py Outdated Show resolved Hide resolved
@obilodeau
Copy link
Member

Fixed latest conflicts and the new pinned dependencies were studied a little bit before deciding which branch of versions to pin.

Oh I realize now I forgot to update the requirements files. I got to go, I'll do it later.

@alxbl
Copy link
Collaborator Author

alxbl commented May 21, 2020

I looked into why the CI is not working for this PR:

Basically we need to install avcodec as a requirement. For Linux this is going to be trivial, but the Windows test run will make this complicated. I'll investigate what can be done.

To make things even more fun, the docker build fails because of invalid certificates:

Couldn't find index page for 'progressbar2' (maybe misspelled?)
Scanning index of all packages (this may take a while)
Reading https://pypi.python.org/simple/
Download error on https://pypi.python.org/simple/: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:852) -- Some packages may not be found!
No local packages or working download links found for progressbar2<4,>=3.20
error: Could not find suitable distribution for Requirement.parse('progressbar2<4,>=3.20')```

Copy link
Member

@obilodeau obilodeau left a comment

Choose a reason for hiding this comment

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

Good to go

@obilodeau obilodeau merged commit aa46ae4 into master May 25, 2020
@obilodeau
Copy link
Member

obilodeau commented Oct 7, 2020

In the rebase process / force-push some of the code has been removed. Let's be more careful to avoid that in the future. To be crystal clear: I'm also to blame since I approved the merge.

image

Regression tracked by #250.

@obilodeau
Copy link
Member

Another note to say that the PyAV version pin to deploy to Ubuntu 18.04 (#199 (comment)) has been removed too and it's causing issues on our Docker builds. Still related to #250. I'll see if I can update to 20.04.

@obilodeau obilodeau deleted the mp4-conversion branch October 19, 2020 19:34
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.

Convert pyrdp recordings to mp4/mkv
4 participants