KallDrexx / rust-media-libs

Rust based libraries for misc media functionality
Apache License 2.0
229 stars 58 forks source link

type 3 chunk may have extended timestamp #15

Closed HuHeng closed 3 years ago

HuHeng commented 3 years ago

A type 3 chunk followed a type 0 chunk which has extended timestamp may has the extended timestamp too.

some rtmp clients send that kind of type3 chunk, for examples, FMLE, ffmpeg, wechat miniprogram(live-pusher), but some not, like rtmpdump.

nignx-rtmp and lal(q191201771/lal, a mediaserver written by golang) always read extended timestamp of a type 3 chunk in this situation. srs(simple rtmp server) compatible with this situation too.

So I created the pull request, it's logic:

if previous chunk of a type 3 chunk was type 0 and it has extended timestamp, then we read 4 bytes as extended timestamp of the type 3 chunk, compare with the previous chunk timestamp, if they are same, then we assume it has extended timestamp, otherwise it does not have.

KallDrexx commented 3 years ago

Good catch! I totally missed this in the RTMP spec

This field is present in Type 3 chunks when the most recent Type 0, 1, or 2 chunk for the same chunk stream ID indicated the presence of an extended timestamp field.

Looking at the code you wrote, am I correct in my understanding that the intention isn't to apply the extended timestamp, but instead to make sure if an extended timestamp exists in a type 3 header that we push the cursor past it (thus not causing parse errors in the actual RTMP chunk body?

HuHeng commented 3 years ago

I miss this in rtmp spec too, so it's no need to compare timestamp of type 3 with the previous extended timestamp.I should delete the compare logic,just reading the extended timestamp,apply it and goto next step。

HuHeng commented 3 years ago

As the spec says,the if statement should add a condition " || timestampdelta>=0xffffff"

HuHeng commented 3 years ago

I change my commit according to the rtmp spec including deserializer, serializer and test.

KallDrexx commented 3 years ago

Thanks a lot for this! I'll try and get this uploaded to crates.io later today.

KallDrexx commented 3 years ago

0.3.6 pushed to crates with this change. Thanks again