-
Notifications
You must be signed in to change notification settings - Fork 417
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
Rtp mpeg4 #35
Rtp mpeg4 #35
Conversation
Added MPEG4 RTP packet reader and added support for MPEG4 playback through RTSP Change-Id: I57c9a61b18471dbd2c368177ebfb89ee662f995b
@ManishaJajoo We can only accept PRs if you sign the CLA. Could you please do that so we can take a look? Thanks! @claincly Assigning to you to further handle the PR once the CLA is signed. |
177517e
to
dfef2d1
Compare
libraries/common/src/main/java/androidx/media3/common/util/CodecSpecificDataUtil.java
Outdated
Show resolved
Hide resolved
libraries/common/src/main/java/androidx/media3/common/util/CodecSpecificDataUtil.java
Outdated
Show resolved
Hide resolved
libraries/common/src/main/java/androidx/media3/common/util/CodecSpecificDataUtil.java
Outdated
Show resolved
Hide resolved
libraries/common/src/main/java/androidx/media3/common/util/CodecSpecificDataUtil.java
Outdated
Show resolved
Hide resolved
libraries/common/src/main/java/androidx/media3/common/util/CodecSpecificDataUtil.java
Outdated
Show resolved
Hide resolved
...aries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpMPEG4Reader.java
Outdated
Show resolved
Hide resolved
...aries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpMPEG4Reader.java
Outdated
Show resolved
Hide resolved
...aries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpMPEG4Reader.java
Outdated
Show resolved
Hide resolved
...aries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpMPEG4Reader.java
Outdated
Show resolved
Hide resolved
...aries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpMPEG4Reader.java
Outdated
Show resolved
Hide resolved
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.
Mostly nits
libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtspMediaTrack.java
Outdated
Show resolved
Hide resolved
libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/RtspMediaTrack.java
Outdated
Show resolved
Hide resolved
...aries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpMPEG4Reader.java
Outdated
Show resolved
Hide resolved
...aries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpMPEG4Reader.java
Outdated
Show resolved
Hide resolved
Please also add test to RtspMediaTrackTest, targeting processing of the codec specific parsing logic, like the ones added in
Also please consider adding the test for RtpMPEG4Reader, for the logics like You can submit the test as a separate PR, thanks! |
resolved the changes requested |
CodecSpecificDataUtil.getVideoResolutionFromMpeg4VideoConfig(configBuffer); | ||
formatBuilder.setWidth(resolution.first).setHeight(resolution.second); | ||
} else { | ||
// set the default width and height |
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.
Why is the default width and height 352x288? Is it in an RFC? If so please add a link
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.
The default resolution is not mentioned in RFC.
For non-standard fmtp where configinput is missing, we use 352X288 as default since CIF 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/mpeg4_h263/C2SoftMpeg4Dec.cpp;l=130
As mentioned in #35 (comment), please add a simple test. |
Sure, we will submit the test as a separate PR. |
Merged internally. The issue will be marked as merged automatically when we do a push. Thanks for the contribution! |
Added support for RTP Mpeg4Reader in Exoplayer