Fix #933 - Correct Chapter Durations If They Extend Past the Episode Length

[maarut]

Fixes #933

When reading chapter information, sanitise it so that no chapter duration is past the episodes end time.

To test

Playing ATP episode 538 shows the correct chapter duration.

Checklist

• I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
• I have considered adding unit tests for my changes.
• I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

[leandroalonso]

@maarut thanks for opening this issue and working on a fix. I think the logic might not work if a wrong duration is returned for a chapter that is not the last one. Keen to hear your thoughts about it.

[leandroalonso]

Am I right in assuming that this logic works only if the last chapter has a wrong duration?

If, by any chance, the penultimate chapter has a similar issue, the operation episodeDuration - chapterStart.seconds will return the incorrect chapter duration (being the remaining time counting from the penultimate chapter, including the last one).

I think we can either add a logic to perform this only on the last chapter or adapt the logic so it grabs the duration between this chapter and the next one (if any).

[maarut]

Thanks for your comments.

Am I right in assuming that this logic works only if the last chapter has a wrong duration?

No. The logic applies to all chapters.

If, by any chance, the penultimate chapter has a similar issue, the operation episodeDuration - chapterStart.seconds will return the incorrect chapter duration (being the remaining time counting from the penultimate chapter, including the last one).

The logic checks the duration of the chapter. If the chapter is longer that the time remaining to the end of the episode, then the chapter duration is capped.

I think we can either add a logic to perform this only on the last chapter or adapt the logic so it grabs the duration between this chapter and the next one (if any).

I think this logic should apply to all chapters, whether hidden or not. We don't know what the episode author intended, so it's not "safe" to cap the duration to the beginning of the next chapter (IMO).

[leandroalonso]

@maarut maybe I'm overthinking it, but take this example:

episodeDuration is 60 and we have 3 chapters:

1. duration is 20
2. duration is 70
3. duration is 30

For the second chapter, the expression: let sanitisedDuration = chapterStart.seconds + chapterDuration > episodeDuration ? episodeDuration - chapterStart.seconds : chapterDuration will be:

let sanitisedDuration = 20 + 70 > 60 ? 60 - 20 : 70 which will return 40 — while the correct duration of the chapter should be 10.

I believe that the episodeDuration - chapterStart.seconds is only correct when the chapter with an issue is the last one, otherwise, this won't return the correct duration.

I agree that the logic should fix all the chapters as you mentioned, but it seems that it won't if the chapter is not the last one.

[maarut]

@leandroalonso I think I was unclear in my previous comment and that there may be some misunderstanding as to how chapters are stored in an episode.

Each chapter contains a start time and a duration (amongst other things). Because chapters can overlap, it's not necessarily true that the end of one chapter is the beginning of the next. For example, you could have a file with chapters like so:

1. startTime is 0, duration is 10
2. startTime is 5, duration is 5
3. startTime is 10, duration is 10
4. startTime is 15, duration is 5

In this example, if episodeDuration is 17, we would want to cap chapters 3 and 4 to have a duration of 7 and 2 respectively. We can calculate the duration by taking episodeDuration and subtracting the startTime of the chapter.

I believe my code does exactly this (with the extra check to determine if we even need to make the calculation).

[leandroalonso]

@maarut got it! Thanks for explaining.

I think I was unclear in my previous comment and that there may be some misunderstanding as to how chapters are stored in an episode.

What do you think about adding some unit tests to cover this? This way another person in the future won't have to guess (and/or search) for this context as it would be available in the tests. :)

Another question, do you have any real episode examples that contain more than one chapter with the wrong duration?