Conversation
Added VP8 RTP packet reader and added support for VP8 playback through RTSP. Change-Id: Ie22ab79a234f61681cf95886a6ed8104a742f056
Change-Id: I2c8eb8d6ee28d8c044d71db042f3b186ea5762f3
|
|
||
| private static final String GENERIC_CONTROL_ATTR = "*"; | ||
|
|
||
| /** Default width and height for VP8. */ |
There was a problem hiding this comment.
Please add comment on where these defaults are defined.
There was a problem hiding this comment.
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.
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.
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.
receivedFirstVp8FramePacket?
There was a problem hiding this comment.
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.
please order these to the declaration order. Also initialize startTimeOffsetUs to C.TIME_UNSET
There was a problem hiding this comment.
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.
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.
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