mp4-tools: fix hevc 608 captions

[erankor]

hevc nal header is different than avc.

avc nal header has the format -
forbidden_zero_bit All f(1)
nal_ref_idc All u(2)
nal_unit_type All u(5)

while hevc nal header has the format -
forbidden_zero_bit f(1)
nal_unit_type u(6)
nuh_layer_id u(6)
nuh_temporal_id_plus1 u(3)

therefore -

1. need to skip 2 bytes instead of 1 when parsing the SEI body
2. to get the nal unit type, need to do (x >> 1) & 0x3f, instead of x & 0x1f

This PR will...

Fix 608 caption extraction for HEVC

Why is this Pull Request needed?

608 captions for HEVC are broken, assumes AVC NAL header format.

Are there any points in the code the reviewer needs to double check?

Resolves issues:

Checklist

• changes have been done against master branch, and PR does not conflict
• new unit / functional tests have been added (whenever applicable)
• API or design changes are documented in API.md

[erankor]

Btw, on a different subject... looking at the code in tsdemuxer.ts, the call to parseSEIMessageFromNALu does 'discardEPB', this seems off to me, as it's already performed inside the function. Am I missing something?

[erankor]

Sample stream: https://cfvod.kaltura.com/hls/p/2035982/sp/203598200/serveFlavor/entryId/1_bfk9vnxa/v/1/pv/7/ev/1/flavorId/1_7pxn4lno/name/a.mp4/master.m3u8

[OrenMe]

@erankor seems the discard call is redundant and probably it doesn't cause any issues doing it twice cause 2nd time will not find Emulation Prevention Bytes so it will just return the data as is, so seems like it can be removed

[OrenMe]

Also a side note, I think MAGIC NUMBERS should start being normalized across the project and set as constants with meaningful names
Barrier for entering demuxing pipe code in the project is high enough for people with little/no background and having meaningful spec vale names will make it easier to follow

[erankor]

@erankor seems the discard call is redundant and probably it doesn't cause any issues doing it twice cause 2nd time will not find Emulation Prevention Bytes so it will just return the data as is, so seems like it can be removed

Yeah, I think that due to the parity bits in 608, having a sequence of zeroes is unlikely, if at all possible... Anyway, I think it's wrong and should be removed.

[robwalch]

the call to parseSEIMessageFromNALu does 'discardEPB', this seems off to me, as it's already performed inside the function. Am I missing something?

'discardEPB' is also defined twice. Once in mp4-tools and once in tsdemuxer.

[erankor]

Opened #4983 for removing the redundant discardEPB.
Since the these two PRs touch adjacent lines, once one of them is merged, the other would probably need conflict resolution...

[robwalch]

Sample stream: https://cfvod.kaltura.com/hls/p/2035982/sp/203598200/serveFlavor/entryId/1_bfk9vnxa/v/1/pv/7/ev/1/flavorId/1_7pxn4lno/name/a.mp4/master.m3u8

English channel looks good. What's going on in the Spanish channel / Unknown CC?

[erankor]

This stream has -

1. CC1 - English (608)
2. Service2 - Japanese (708)
3. Service3 - Chinese (708)

The player does not support 708 (it tries to parse it as 608 or something like that) so this is expected, we're getting the same behavior with AVC. We've recently implemented a module that does '608/708 -> webvtt' on the server side to work around the missing support for 708.
(Btw, this is the reason we needed the support for CLOSED-CAPTIONS=NONE, thanks for fixing it! :))