RTSP H265 Aggregation packet support #2413

ArtsemKurantsou · 2025-05-12T00:05:57Z

Aggregation packet support for H265 RTSP added according to the specification.
The implementation is based on the assumption that DONL won't be present in the packet since sprop-max-don-diff != 0 is not supported

Fixes issue: #1008

ArtsemKurantsou · 2025-05-15T05:44:04Z

Hi @tonihei, could you please help with review and moving forward with the change?

microkatz · 2025-05-20T10:04:50Z

libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpH265Reader.java

+    int endOfData = data.bytesLeft();
+    int currentPosition = 2; // skipping payload header (2 bytes)
+    do {
+      int nalUnitSize = ((data.getData()[currentPosition] & 0xFF) << 8)


Is this equivalent to data.readUnsignedShort()?

ArtsemKurantsou · 2025-05-20T16:22:27Z

Hi @microkatz thank you for review and comment, could you please review again?

microkatz · 2025-05-22T10:46:29Z

I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!

microkatz · 2025-05-22T11:57:50Z

@ArtsemKurantsou

Would you be able to add a test for this? I believe that you would create a file and test similar to the one in RtpH263ReaderTest.

Thank you!

libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpH265Reader.java

ArtsemKurantsou · 2025-05-22T17:03:02Z

@ArtsemKurantsou

Would you be able to add a test for this? I believe that you would create a file and test similar to the one in RtpH263ReaderTest.

Thank you!

Yes, sure, I can add tests for that

ArtsemKurantsou · 2025-06-23T02:40:22Z

Hi @microkatz I added unit tests, could you please take a look?

microkatz · 2025-06-23T11:56:49Z

...es/exoplayer_rtsp/src/test/java/androidx/media3/exoplayer/rtsp/reader/RtpH265ReaderTest.java

+        .build();
+  }
+
+  private static void consumeFirstPacket(


All your tests only consume one packet? Is this static method needed?

If you can please just move these lines to each test.

Side note: Maybe it would be beneficial to create a test with multiple packets similar to RtpH263ReaderTest::consume_outOfOrderPackets.

I think it make sense to keep this method to not repeat h265Reader.onReceivingFirstPacket in every test as it seems to be an expected call before first RTP packet is consumed.
I also added test with multiple packets

microkatz · 2025-06-23T12:29:54Z

...es/exoplayer_rtsp/src/test/java/androidx/media3/exoplayer/rtsp/reader/RtpH265ReaderTest.java

+              .build(),
+          /* rtpPayloadType= */ 98,
+          /* clockRate= */ (int) MEDIA_CLOCK_FREQUENCY,
+          /* fmtpParameters= */ ImmutableMap.of(),


Currently the RtpH265Reader does not handle reading the decoder initialization data from the stream. It requires the addition of the fmtp parameters in the rtp transport layer.

If you copied these example packets from a stream, would you be so kind as to provide the fmtp parameter data from that stream into this format data as well?

The payloads are just dummy data, but I've tested on the real stream and added parameters and format from the real stream.

microkatz · 2025-06-23T12:31:47Z

...es/exoplayer_rtsp/src/test/java/androidx/media3/exoplayer/rtsp/reader/RtpH265ReaderTest.java

+    consumeFirstPacket(h265Reader, INVALID_AP_PACKET_EXTRA_BYTE);
+  }
+
+  @Test(expected = ParserException.class)


Rather than utilize the (expected = thrownexception), the style is to use assertThrows with the specific line intending to provide the error. Would you update these unit tests using the following as an example?

assertThrows(ParserException.class, () -> consumeFirstPacket(h265Reader, INVALID_AP_PACKET_EXTRA_BYTE));

microkatz · 2025-06-23T12:36:08Z

...es/exoplayer_rtsp/src/test/java/androidx/media3/exoplayer/rtsp/reader/RtpH265ReaderTest.java

+    );
+  }
+
+  private static RtpPacket copyPacket(RtpPacket packet) {


Currently your tests are not re-using RtpPackets across multiple unit tests so I'm not sure you need to run copyPacket? Your tests don't seem to fail if copyPacket is not executed.

yeah, removed this, just added based on the the H263 implementation, but doesn't seems to be required since packet is not being modified anywhere.

ParsableByteArray operations utilized instead of manual bytes manipulation

Got read of endOfData variable in favor of check based on bytesLeft

…acket-support PiperOrigin-RevId: 776088856 (cherry picked from commit 5dc4d94)

ArtsemKurantsou marked this pull request as ready for review May 12, 2025 00:09

microkatz self-requested a review May 16, 2025 12:23

microkatz self-assigned this May 16, 2025

microkatz reviewed May 20, 2025

View reviewed changes

ArtsemKurantsou requested a review from microkatz May 20, 2025 16:21

microkatz force-pushed the rtap-h265-aggregation-packet-support branch from 68d2c66 to af67e05 Compare May 22, 2025 10:46

microkatz added the should merge label May 22, 2025

microkatz reviewed May 22, 2025

View reviewed changes

libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpH265Reader.java Outdated Show resolved Hide resolved

libraries/exoplayer_rtsp/src/main/java/androidx/media3/exoplayer/rtsp/reader/RtpH265Reader.java Outdated Show resolved Hide resolved

ArtsemKurantsou requested a review from microkatz June 23, 2025 02:39

microkatz reviewed Jun 23, 2025

View reviewed changes

ArtsemKurantsou requested a review from microkatz June 24, 2025 02:49

ArtsemKurantsou and others added 8 commits June 26, 2025 08:56

RTP H265 aggregation packet support implemented

b8216cf

Minor adjustment based on the review

20a74b8

ParsableByteArray operations utilized instead of manual bytes manipulation

Minor adjustments

d84fd51

Got read of endOfData variable in favor of check based on bytesLeft

Format with google-java-format

c367d71

Added release note

3350dc2

Additional validations and unit tests added

6e63a1b

Unit tests adjustments based on the review

2ce5276

NAL unit header type calculation updated to use NalUnitUtil

1225847

microkatz force-pushed the rtap-h265-aggregation-packet-support branch 4 times, most recently from 645cd0a to d3d32f1 Compare June 26, 2025 09:52

Formatted code and added additional unit test

e04c3af

microkatz force-pushed the rtap-h265-aggregation-packet-support branch from d3d32f1 to e04c3af Compare June 26, 2025 10:08

copybara-service bot merged commit 5dc4d94 into androidx:main Jun 26, 2025
1 check passed

shahdDaghash pushed a commit that referenced this pull request Jul 7, 2025

Merge pull request #2413 from ArtsemKurantsou:rtap-h265-aggregation-p…

653f227

…acket-support PiperOrigin-RevId: 776088856 (cherry picked from commit 5dc4d94)

RTSP H265 Aggregation packet support #2413

RTSP H265 Aggregation packet support #2413

Uh oh!

Conversation

ArtsemKurantsou commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArtsemKurantsou commented May 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArtsemKurantsou commented May 20, 2025

Uh oh!

microkatz commented May 22, 2025

Uh oh!

microkatz commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ArtsemKurantsou commented May 22, 2025

Uh oh!

ArtsemKurantsou commented Jun 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ArtsemKurantsou commented May 12, 2025 •

edited

Loading

microkatz commented May 22, 2025 •

edited

Loading