H264: Ignore if reserved bit is set FU-A payload

[nemosupremo]

trafficstars

We found in certain cameras with the HeroSpeed firmware were setting this bit.

The spec says:

The Reserved bit MUST be equal to 0 and MUST be ignored by the receiver.

This patch just ignores the reserved bit instead of throwing an error.

[scottlamb]

I just found the following in RFC 6184 section 5.8:

A receiver in an endpoint or in a MANE MAY aggregate the first n-1 fragments of a NAL unit to an (incomplete) NAL unit, even if fragment n of that NAL unit is not received. In this case, the forbidden_zero_bit of the NAL unit MUST be set to one to indicate a syntax violation.

I wonder if that's what you're encountering. If so, we should probably treat it similar to loss, and discard the whole frame.

Could you add a test?

[nemosupremo]

1. gortsplib does not drop the entire frame https://github.com/bluenviron/gortsplib/blob/07039eb7d9fc8f54ef7f63f90b27d44641e485bd/pkg/format/rtph264/decoder.go#L87 (it seems to ignore the header entirely).
2. From the quoted section, I'm not sure what the spec is implying - it doesn't seem to imply to drop the frame. It seems to say that the reserved bit must be set while the receiver is aggregating the frame. I'm not sure how this is relevant because retina does not expose partially aggregated NAL units. Furthermore, I don't think part of the spec concerns what happens when the sender sends a NAL with the forbidden zero bit.

[nemosupremo]

The Reserved bit in the FU-A header is not the same as the forbidden_zero_bit in the NAL header.

I think RFC 6184 section 5.8 is stating that if you have an incomplete NAL unit then you must set the forbidden_zero_bit.

1. I don't think that's relevant here
2. If this is something that should be done, then I think that should be a separate PR.
3. As I understand it today, if retina encounters an incomplete fragmented NAL, retina will just error with Non-fragmented NAL while FU-A fragment in progress

[scottlamb]

The Reserved bit in the FU-A header is not the same as the forbidden_zero_bit in the NAL header.

Sorry, I've been neglecting this repo, but you're totally right about this. Just fixed CI at head and will squash/merge now.