I believe there maybe an issue with the Set* methods when the media playlist capacity is not a power of 2. It appears that once the playlist is full, a Set* (such as SetDiscontinuity()
or SetKey()
) sets the correct value on the wrong segment.
The following should demonstrate this, ignore the fact I'm ignoring errors, even when checked the problem still occurs. The SetKey()
method best shows the issue as I can set the key URI (and IV) to be the same as the segment URI to better illustrate the problem, but the same occurs with SetDiscontinuity()
(and likely other methods that use the p.Segments[(p.tail-1)%p.capacity]
calculation).
package main
import (
"fmt"
"github.com/grafov/m3u8"
)
func main() {
// OK when size is 1,2,4,8,16....
// Not OK when size is 3,5,6,7,9,10...
size := uint(5)
pls, _ := m3u8.NewMediaPlaylist(size, size)
for i := uint(0); i < size; i++ {
uri := fmt.Sprintf("uri-%d", i)
_ = pls.Append(uri+".ts", 4, "")
_ = pls.SetKey("AES-128", uri+".key", fmt.Sprintf("%d", i), "", "")
}
fmt.Print(pls)
}
Output
#EXTM3U
#EXT-X-VERSION:3
#EXT-X-MEDIA-SEQUENCE:1
#EXT-X-TARGETDURATION:4
#EXT-X-KEY:METHOD=AES-128,URI="uri-4.key",IV=4 <- the 5th iteration overwrote the 1st's
#EXTINF:4.000,
uri-0.ts
#EXT-X-KEY:METHOD=AES-128,URI="uri-1.key",IV=1 <- 2nd iteration correct
#EXTINF:4.000,
uri-1.ts
#EXT-X-KEY:METHOD=AES-128,URI="uri-2.key",IV=2 <- 3rd iteration correct
#EXTINF:4.000,
uri-2.ts
#EXT-X-KEY:METHOD=AES-128,URI="uri-3.key",IV=3 <- 4th iteration correct
#EXTINF:4.000,
uri-3.ts
#EXTINF:4.000,
uri-4.ts <- 5th iteration is missing the EXT-X-KEY
Generally, it looks like the problem is that the .Append()
method sets playlist tail property (p.tail
) to be modulo capacity and stores it back in p.tail
(p.tail
is always between 0 and p.capacity
). Whereas the Set* methods assume this is a incrementing integer (based on they're calculation of (p.tail-1)%p.capacity
). Essentially, because p.tail
wraps to zero and because it is a uint, when p.tail
is 0 the result of p.tail-1
is 18446744073709551615. Changing this to an int doesn't quite fix things either, at least not without still requiring further changes within the Set* methods (but this may also be the preferred solution) and more casting.
I believe a simple change maybe:
@@ -275,8 +276,8 @@ func (p *MediaPlaylist) Append(uri string, duration float64, title string) error
seg.URI = uri
seg.Duration = duration
seg.Title = title
- p.Segments[p.tail] = seg
- p.tail = (p.tail + 1) % p.capacity
+ p.Segments[p.tail%p.capacity] = seg
+ p.tail++
p.count++
if p.TargetDuration < duration {
p.TargetDuration = math.Ceil(duration)
There's other methods to fix it in each Set* method, via (something similar to, but perhaps using a private tail()
method to obtain the correct tail):
+ tail := p.tail - 1
+ if p.tail == 0 {
+ tail = p.capacity - 1
+ }
+ // OR: tail := int(math.Min(float64(p.tail-1), float64(p.capacity-1)))
- p.Segments[(p.tail-1)%p.capacity].Key = &Key{method, uri, iv, keyformat, keyformatversions}
+ p.Segments[tail].Key = &Key{method, uri, iv, keyformat, keyformatversions}
I wanted to get hear others' thoughts on this before I go too much further, as this maybe a misunderstanding, maybe another solution, or this proposed change may cause other unintended consequences. This isn't a straight forward issue as there's multiple causes (depending on your opinion) and multiple solutions.
I'll be happy to send in a PR + tests and also address the Remove()
methods (as well as the corresponding head variable), just focusing on the Append()
and Set* methods first.
Note, I can't reproduce the issue when there capacity is a power of 2, e.g.: 1,2,4,8,16 - this may explain why the issue hasn't been discussed before?
I think there's two ways to solve this, move tail/head to ints and handle negatives in Set* methods, or use the above method (tail to always be incrementing). I'll continue to have a bit of a think about this, but I want to hear others' opinions.