• Catch ValueError when decrypting and unpadding segments, as well as
  other exceptions that can be raised when downloading the segment,
  similar to unencrypted segments, and log error messages accordingly.
  Don't close the stream on any of those errors.
• Remove garbage data truncation from encrypted segments:
  This was (most likely) implemented because of an old issue related to
  Content-Length HTTP headers not being handled correctly, leading to
  partial segment downloads. Cutting off data that doesn't fit into the
  AES block size doesn't make sense, as it implies that the segment data
  incomplete, and incomplete segment data misses its padding bytes at
  the end which must always be included.
• Update tests

updated commit message above, old initial comment below

ValueErrors can get raised by both unpad and decryptor.decrypt(). This change therefore also includes the garbage data cut-off when the segment length is not a multiple of the AES block size (before decrypting).

We've had a discussion before where we were not sure why the garbage data cut-off was ever implemented. And to be honest, it doesn't make any sense having it, because if the encrypted segment is not a multiple of the AES block size, then it's an invalid segment and cutting off any remaining bytes surely won't help with fully decrypting it. The result will most likely be junk or incomplete data.

I believe that this was added in c38a543 to avoid ValueErrors being raised when calling decryptor.decrypt() when the segment length is invalid. This seemed to be a better solution to the Content-Length issue which could occur that resulted in partial/incomplete segment downloads (fixed by #3768), and it meant that the stream could continue.

Any opinions on whether the garbage truncation should be removed? It doesn't hurt having it, but it's pretty much useless IMO. The test_hls_encrypted_aes128_incorrect_block_length test was added by me in b218259 just to have the code path covered.

As you can see in the diff, it now logs the Error while decrypting segment {sequence.num} error message instead of showing an exception call stack. This makes it much more clear that it's an issue with the stream, and not Streamlink.

However, if an invalid decryption key is used (eg. #4528), the error message is still not 100% clear. Invalid decryption keys will result in nonsense data after decrypting the segment, so the unpad function call will raise a ValueError in 99.9% of all cases. I don't think it's too much of a problem though because it can't be distinguished from corrupted/broken segment downloads anyway, so adding something like "Invalid key?" isn't always a good fit.