Skip to content

Announce the rotated resolution so portrait streams are not pillarboxed - #2182

Merged
pedroSG94 merged 1 commit into
pedroSG94:masterfrom
x270880x:fix/announce-rotated-resolution
Aug 12, 2026
Merged

Announce the rotated resolution so portrait streams are not pillarboxed#2182
pedroSG94 merged 1 commit into
pedroSG94:masterfrom
x270880x:fix/announce-rotated-resolution

Conversation

@x270880x

Copy link
Copy Markdown
Contributor

A portrait broadcast tells the RTMP server it is landscape.

VideoEncoder stores width/height exactly as prepareVideoEncoder received them and keeps rotation separately — at 90 or 270 it configures the codec as height x width (VideoEncoder.java:136-141). So a stream prepared with (1920, 1080, rotation = 90) really produces 1080x1920 frames, while getVideoResolution() returns 1920x1080.

That pair goes straight into onMetaData:

  • GenericStream.kt:111
  • RtmpStream.kt:73
  • MultiStream.kt:188

Players trust onMetaData. YouTube opens a 16:9 window and pillarboxes the vertical picture — black bars down both sides — which is exactly what our users reported for vertical streams.

MultiCamera1/2 already handle this at the call site:

if (videoEncoder.rotation == 90 || videoEncoder.rotation == 270) {
    rtmpClients[index].setVideoResolution(videoEncoder.height, videoEncoder.width)
} else {
    rtmpClients[index].setVideoResolution(videoEncoder.width, videoEncoder.height)
}

so only the newer classes are affected. I fixed it inside getVideoResolution() instead of repeating the swap three times, so anything else reading it also gets the shape that is actually on the wire.

Measured on a Samsung SM-A065F, camera in portrait: prepared 1920x1080 with rotation 90, captured stream carries 1080x1920 frames (read from the SPS), and the announced resolution now matches instead of claiming landscape.

@pedroSG94

Copy link
Copy Markdown
Owner

Hello,

Thank you for the PR, but please, remove the comment in the code.
The code is self explanatory since it is the same than in other classes and you already have the explanation in the commit message.
That type of messages, but in a short way if possible, are fine only with public methods (to let the user how to use them) or in a internal complex code that you will need feedback about the reason in the future. I think it is not the case since it is done in all RTMP classes like RtmpCamera1.

VideoEncoder stores width/height exactly as prepareVideoEncoder received them and
keeps rotation separately; at 90 or 270 it configures the codec as height x width.
A portrait broadcast prepared with (1920, 1080, rotation = 90) therefore produces
1080x1920 frames, while getVideoResolution() reported 1920x1080.

That pair goes straight into onMetaData:

  GenericStream.kt:111  val resolution = super.getVideoResolution()
  RtmpStream.kt:73      val resolution = super.getVideoResolution()
  MultiStream.kt:188    val resolution = super.getVideoResolution()

so every portrait broadcast through the newer API told the server it was landscape.
Players trust onMetaData: YouTube opens a 16:9 window and pillarboxes the vertical
picture, which is what our users reported. MultiCamera1/2 already swapped at their
call sites (MultiCamera2.kt:197-201), so only the newer classes were affected.

Fixed in getVideoResolution() rather than at the three call sites, so anything else
reading it gets the shape that is actually on the wire.

Verified on a Samsung SM-A065F: prepared 1920x1080 with rotation 90, the encoder
emits 1080x1920 (confirmed from the SPS of the captured stream), and the metadata
now matches instead of claiming landscape.
@x270880x
x270880x force-pushed the fix/announce-rotated-resolution branch from fa5aae5 to ddfac19 Compare August 12, 2026 13:15
@x270880x

Copy link
Copy Markdown
Contributor Author

Done — comment removed, the diff is now the code change only. The reasoning stays in the commit message, as you suggested.

Thanks for looking at it.

@pedroSG94
pedroSG94 merged commit 191d9f0 into pedroSG94:master Aug 12, 2026
1 check passed
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.

2 participants