-
Notifications
You must be signed in to change notification settings - Fork 405
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 RTSP VP8 #47
Conversation
Added VP8 RTP packet reader and added support for VP8 playback through RTSP. Change-Id: Ie22ab79a234f61681cf95886a6ed8104a742f056
Change-Id: I2c8eb8d6ee28d8c044d71db042f3b186ea5762f3
@@ -56,6 +56,10 @@ | |||
|
|||
private static final String GENERIC_CONTROL_ATTR = "*"; | |||
|
|||
/** Default width and height for VP8. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comment on where these defaults are defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RFC doesn't mandate any codec specific data(like width, height) to be present in fmtp attributes for VP8.
RFC also doesn't mentioned any default resolution. So, i use 320X240 as default since it is the default resolution used by Android Software decoder.
Adding the link for your reference:
https://cs.android.com/android/platform/superproject/+/master:frameworks/av/media/codec2/components/vpx/C2SoftVpxDec.cpp;drc=749a74cc3e081c16ea0e8c530953d0a247177867;l=70
Format trackFormat = payloadFormat.format; | ||
Format.Builder formatBuilder = trackFormat.buildUpon(); | ||
formatBuilder.setWidth(width).setHeight(height); | ||
trackOutput.format(formatBuilder.build()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double checking: If playing back a 1080p VP8 video, the player is able to pick up the 1080p format information? Because no other RTP readers reconfigures track format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have checked that the player is able to pick format information for 1080p.
firstReceivedTimestamp = C.TIME_UNSET; | ||
previousSequenceNumber = C.INDEX_UNSET; | ||
fragmentedSampleSizeBytes = 0; | ||
gotFirstPacketOfVP8Frame = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
receivedFirstVp8FramePacket
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In VP8 a frame can be divided into partitions, these partitions can be sent as a packet. This variable is used to check that the packet received is the first packet of a frame.
|
||
/** Creates an instance. */ | ||
public RtpVP8Reader(RtpPayloadFormat payloadFormat) { | ||
this.payloadFormat = payloadFormat; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please order these to the declaration order. Also initialize startTimeOffsetUs
to C.TIME_UNSET
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on the order you want to keep because as for my knowledge variables are in the same order of declaration.
I can't initialize "startTimeOffsetUs" to C.TIME_UNSET because value of "C.TIME_UNSET" is min of Long. For any clip startTimeOffsetUs need to zero except if we are seeking.
libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVP8Reader.java
Outdated
Show resolved
Hide resolved
// Parsing frame data to get width and height, RFC6386 Section 9.1 | ||
int currPosition = data.getPosition(); | ||
data.setPosition(currPosition + 6); | ||
int width = data.readLittleEndianUnsignedShort() & 0x3fff; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need littleEndian here? I assume the data here is in "network order". Also this library could be used on big-endian devices, will using littleEndian method here limit the usecases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to RFC6386 Section 19.1 width and height are save in littleEndianess.
https://datatracker.ietf.org/doc/html/rfc6386#section-19.1
libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVP8Reader.java
Outdated
Show resolved
Hide resolved
libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVP8Reader.java
Outdated
Show resolved
Hide resolved
libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVP8Reader.java
Outdated
Show resolved
Hide resolved
libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpVP8Reader.java
Outdated
Show resolved
Hide resolved
Change-Id: Id47c746b199831d0bb51dc736c43fd20c2e79c08
Merged internally. The issue will be marked as merged automatically when we do a push. Thanks for the contribution! |
Added VP8 RTP packet reader and added support for VP8 playback
through RTSP.
Change-Id: Ie22ab79a234f61681cf95886a6ed8104a742f056