########################################################################################## HLS-22: IV will be wrong for some bitrates in ABR streams ########################################################################################## Issue Type: Task ----------------------------------------------------------------------------------------- Issue Information ==================== Priority: Major Status: Closed Resolution: Fixed (2017-04-29 12:07:25) Project: HLS Stream Creator (HLS) Reported By: btasker Assigned To: btasker Components: - Encryption Targeted for fix in version: - 1.0 Time Estimate: 60 minutes Time Logged: 60 minutes ----------------------------------------------------------------------------------------- Issue Description ================== In function encrypt, the for loop is written on the basis of there only being one bitrate, and so generates an IV based upon the segment number -- BEGIN SNIPPET -- count=0 for file in ${OUTPUT_DIRECTORY}/*.ts do ENC_FILENAME="$OUTPUT_DIRECTORY/${SEGMENT_PREFIX}_enc_${count}".ts INIT_VECTOR=$(printf '%032x' $count) -- END SNIPPET -- If we've got an ABR stream with 3 bitrates, containing 5 segments each, then when we move onto the second bitrate count is going to be at 6 (whilst we'll want it to be 0). So the function almost certainly needs adapting to take bitrates into account rather than simply iterating over files. Apple's technote ( https://developer.apple.com/library/content/technotes/tn2288/_index.html ) specifies that -- BEGIN QUOTE -- The default Initialization Vector for media encryption (if none is specified) is the sequence number of the media file. You should specify an Initialization Vector value, and not rely on sequence numbers. The main reason for this is portability. For example, if you change where the segment appears in the playlist (e.g. inserting an ad), that changes its sequence number, requiring a re-encrypt. -- END QUOTE -- So this is almost certainly going to be an issue. Longer term, it'd be better to follow their advice and set a specific IV (which will mean rewriting the manifest), but in the short term need to make the sequence numbers work as a minimum ----------------------------------------------------------------------------------------- Issue Relations ================ - Discovered In HLS-21: Review Encrypt function ----------------------------------------------------------------------------------------- Activity ========== ----------------------------------------------------------------------------------------- 2017-04-29 09:53:38 btasker ----------------------------------------------------------------------------------------- As the codebase currently stands, segment file names for a multi bitrate stream will always be -- BEGIN SNIPPET -- [arbitrary strings]_[bitrate]_[segment number].ts -- END SNIPPET -- For a single bitrate (where the -b argument hasn't been used at all) it'll be -- BEGIN SNIPPET -- [arbitrary strings]_[segment number].ts -- END SNIPPET -- When generating the IV, we're only interested in the segment number, so we can probably just iterate over the files and extract the segment number from the filename to then use as the IV. ----------------------------------------------------------------------------------------- 2017-04-29 10:09:23 btasker ----------------------------------------------------------------------------------------- Triggering a 'before' run for comparison while I make the changes -- BEGIN SNIPPET -- ben@milleniumfalcon:/tmp/hlstest$ ./HLS-Stream-Creator/HLS-Stream-Creator.sh -i big_buck_bunny_720p_stereo.avi -s 10 -e -o before -b 272,872,1372 -- END SNIPPET -- Doi -- BEGIN SNIPPET -- [aac @ 0xa40a60] The encoder 'aac' is experimental but experimental codecs are not enabled, add '-strict -2' if you want to use it. ben@milleniumfalcon:/tmp/hlstest$ FFMPEG_FLAGS='-strict -2' ben@milleniumfalcon:/tmp/hlstest$ export FFMPEG_FLAGS ben@milleniumfalcon:/tmp/hlstest$ ./HLS-Stream-Creator/HLS-Stream-Creator.sh -i big_buck_bunny_720p_stereo.avi -s 10 -e -o before -b 272,872,1372 -- END SNIPPET -- I'm gonna drop a check for that in (HLS-23) once I've got this sorted, it was introduced in https://github.com/bentasker/HLS-Stream-Creator/commit/0796febbc73d9b96c7ee58864e6ce79c9ff71b4d - I'm running a fairly recent install, so I suspect a few people may hit up against that ----------------------------------------------------------------------------------------- 2017-04-29 10:09:30 ----------------------------------------------------------------------------------------- btasker changed status from 'Open' to 'In Progress' ----------------------------------------------------------------------------------------- 2017-04-29 10:19:10 btasker ----------------------------------------------------------------------------------------- Changes are made, have just triggered a test run so can test. ----------------------------------------------------------------------------------------- 2017-04-29 11:11:00 btasker ----------------------------------------------------------------------------------------- For some reason, when running, the following can't be used to strip leading 0's off the segment number (needs doing so printf doesn't assume the number is octal) -- BEGIN SNIPPET -- SEG_NO=${SEG_NO##+(0)} -- END SNIPPET -- For now, I've done it with sed, but it's irritating. Want to get around to having a look at shopt and the like when running to see if I can figure out why. Running the same loop manually in a shell, it works fine. ----------------------------------------------------------------------------------------- 2017-04-29 11:12:51 git ----------------------------------------------------------------------------------------- -- BEGIN QUOTE -- Repo: HLS-Stream-Creator Commit: 7e3fdefffd16f9051dba6a41100f36dd7ca4cc0a Author: B Tasker