m3u8-parser: fill missing rendition report attrs

[erankor]

This PR will...

Fill out any missing LAST-MSN/LAST-PART attributes from rendition reports, by copying the respective values from the current level being parsed.

Why is this Pull Request needed?

According to the hls spec -
" A server MAY omit adding an attribute to an EXT-X-RENDITION-REPORT
tag - even a mandatory attribute - if its value is the same as that
of the Rendition Report of the Media Playlist to which the EXT-X-
RENDITION-REPORT tag is being added. Doing so reduces the size of
the Rendition Report."

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

Normally LAST-MSN / LAST-PART are strings, here they are set to an int, from my tests it works just fine, but just a point to keep in mind.

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

[robwalch]

Have you looked at switchParams in base-stream-controller?

Parsed attributes should reflect what was parsed from the playlist. If using the current level details is allowed when these attributes are missing then this can be handled in switchParams using previous details when present:

https://github.com/video-dev/hls.js/blob/v1.2.9/src/controller/base-playlist-controller.ts#L69

- const msn = parseInt(attr['LAST-MSN']);
- let part = parseInt(attr['LAST-PART']);
+ const msn = parseInt(attr['LAST-MSN']) || previous?.lastPartSn;
+ let part = parseInt(attr['LAST-PART']) || previous?.lastPartIndex;

[erankor]

Thanks @robwalch, I drafted something that extracts this info in switchParams from existing fields, but it's a bit clumsy... and I have some problem there with the endSN + 1 - I need to add 1 there only when the last part is after the last segment, and not sure I currently have a way to know if that's the case...

Did you mean that I'll add new fields to LevelDetails to hold this stuff?

[robwalch]

Thanks @robwalch, I drafted something that extracts this info in switchParams from existing fields, but it's a bit clumsy... and I have some problem there with the endSN + 1 - I need to add 1 there only when the last part is after the last segment, and not sure I currently have a way to know if that's the case...

Did you mean that I'll add new fields to LevelDetails to hold this stuff?

It is doable with the fields that are there. The LL-HLS playlist reload logic already does this around calls to getDeliveryDirectives. When details.lastPartSn === details.endSN then the last part belongs to the last segment - this is when we increment msn and set part to 0.

[erankor]

Thanks Rob! I updated the PR.
I figured I can just use lastPartSn, since it already has a fallback to endSN when there are no parts.

Btw, in terms of style, my personal preference is always to have flat code, and avoid high levels of indentation. So, in this specific function, I would do something like: if (!renditionReports) { return } at the beginning, and then inside the for loop if (uri !== playlistUri.slice(-uri.length)) { continue } etc. I tried to change only the minimum required in this PR, but if other people here share the same opinion, I will happily apply those changes as well.

[robwalch]

Btw, in terms of style

prettier decides. Do npm run prettier and commit the indentation changes.

Why not use the optional operator on previous? it's flatter.

const msn = parseInt(attr['LAST-MSN']) || previous?.lastPartSn;
let part = parseInt(attr['LAST-PART']) || previous?.lastPartIndex;

endSN is not optional and always defined.

[erankor]

You're right, when I read your first comment, I thought this was just pseudocode :) sorry about that.

Anyway, I removed the isFinite check on msn, following what you wrote about endSN always being defined.
And, changed the checks on part to use >= 0 since lastPartIndex returns -1 when there are no parts.

[erankor]

@robwalch, does this look ok? do you think we can merge it?