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

Convert to MP4 doesn't support resolution changes #328

Open
obilodeau opened this issue Jul 29, 2021 · 4 comments
Open

Convert to MP4 doesn't support resolution changes #328

obilodeau opened this issue Jul 29, 2021 · 4 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed
Milestone

Comments

@obilodeau
Copy link
Member

Preconditions during the MITM session:

  • client connects to a low resolution
  • client updates to a higher resolution (tested: via client dynamic resolution update, not tested: via manual resolution change)

Converting this .pyrdp file to an MP4 will result in a video that uses the first low resolution throughout the duration of the video meaning you will miss out on some screen elements.

Attaching a capture to repro the issue.

@obilodeau obilodeau added the bug Something isn't working label Jul 29, 2021
@obilodeau
Copy link
Member Author

reproducer.zip

@alxbl
Copy link
Collaborator

alxbl commented Oct 24, 2021

This is tricky, AFAIK H264 doesn't support dynamic resolution changes within the same stream. I can think of two approaches:

  1. Do a first passs of the replay to look for resolution changes and use the maximum resolution. Fit everything else (resize or pad with borders?) into the frame
  2. Finalize the mp4 and start a new render. Maybe with extra info in the title

Approach 2 seems like the easiest to implement, but not the most user-friendly.

I'll take a look at the dynamic resolution handling code and see if I can handle it in the Mp4Handler to implement approach 2

@alxbl alxbl self-assigned this Oct 24, 2021
@obilodeau
Copy link
Member Author

I think option 1) is the only option. Especially if one would create a conversion pipeline. Creating a new file would be surprising and hard to track.

Then again, how fast is this first pass going to be? I hope it'll be fast.

An interim fix would be to emit a warning and exit with a non-zero status if there's a resolution change in a conversion. That would send a clear message to users.

@alxbl
Copy link
Collaborator

alxbl commented Oct 25, 2021

I do not know how heavy it will be to parse the whole session without actually rendering it. If you have a repro capture, you can send it to me off-band, otherwise I'll try to prepare one myself. I see you attached the files above... D:

client updates to a higher resolution (tested: via client dynamic resolution update, not tested: via manual resolution change)

This isn't allowed by MSTSC as far as I know. Maybe some Linux clients allow it, though?

@obilodeau obilodeau added this to the v2.1.0 milestone Dec 22, 2023
@obilodeau obilodeau added the help wanted Extra attention is needed label Dec 22, 2023
@obilodeau obilodeau modified the milestones: v2.1.0, v2.2.0 Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants