globocom / m3u8

Python m3u8 Parser for HTTP Live Streaming (HLS) Transmissions
Other
2.06k stars 474 forks source link

if EXT-X-KEY:METHOD=NONE, get TypeError: __init__() takes at least 4 arguments (3 given) #58

Open sloh opened 9 years ago

sloh commented 9 years ago

in model.py > Class Segment() > init() for the following line:

self.key = Key(base_uri=base_uri,**key) if key else None

if a segment has "#EXT-X-KEY:METHOD=NONE", it is missing the uri parameter needed for class Key(), its init function:

def __init__(self, method, uri, base_uri, iv=None, keyformat=None, keyformatversions=None):

I believe uri should also be an optional parameter according to this documentation:

URI

   The value is a quoted-string containing a URI that specifies how to
   obtain the key.  This attribute is REQUIRED unless the METHOD is
   NONE.

http://tools.ietf.org/html/draft-pantos-http-live-streaming-16#section-4.3.2.4

The fix would be the following code for Key().init() where uri=None is the main different making it optional:

def __init__(self, method, base_uri, uri=None, iv=None, keyformat=None, keyformatversions=None):
bertothunder commented 8 years ago

I can confirm URI should be optional, if METHOD=NONE then most of encoders or packagers won't add URI onto it.

davidjameshowell commented 8 years ago

I can confirm the same. Encountered this most recently with content that I had been using that has METHOD=None. I had to get the script working, so I just went through and edited it to return none for the key for the time being until a fix is rolled.

leandromoreira commented 8 years ago

@davidjameshowell try now with @birme fixes (you need to checkout the master)

bertothunder commented 8 years ago

@leandromoreira fix from @birme is wrong: the URI is optional, but his fix forces URI to be empty string (not None), so the fix drops the URI even if provided.

Check my commit, it fixes the issue and brings a new test for this.

bertothunder commented 8 years ago

So my fix is wrong as well as the original fix for this issue. As the uri must NOT be present (as pointed out by Bradley).

So, as pointed out by the author, uri must be none in the constructor to avoid the original error.

leandromoreira commented 8 years ago

Now, I'm confused, should I revert some commits?

birme commented 8 years ago

I understand what you mean and will fix. If method is NONE uri is optional but if provided it should remain intact

birme commented 8 years ago

Disregard this comment. Saw the other thread. According to spec if method is none no following attributes must be present. So go ahead and revert my fix as the original problem it solves was not a problem...

bertothunder commented 8 years ago

Well, thinking about it, I did actually close my pull request too soon.

The important thing here is, what are we trying to achieve? sticking to the standard, or broad compatibility? I've seen output from encoders and packagers that will add EXT-X-KEY tags with method=NONE, and an optional URI. That's why I was wrong about the standard itself.

If the parser has to stick to the RFC (strict mode?) then the URI can't be present, so that should either be ignored or raise an error. If we are looking for broad compatibility, then the URI must be checked and added.

Anyway, it won't hurt if the Key class init method adds a default =None to uri parameter. I'll wait to see how this discussion goes, and then I can reopen the pull request with my change and the init method with uri=None on it.

bradleyfalzon commented 8 years ago

Note, my comment in PR #88 was only to clarify the RFC, however, the adage Be conservative in what you send, be liberal in what you accept is appropriate, unless there's real world evidence of requiring the non-standard support, I think accepting the malformed RFC and silently discarding the URI is likely best.

Note, I do not use this library, but I do help maintain a similar Go based m3u8 parser.

davidjameshowell commented 8 years ago

https://tools.ietf.org/html/draft-pantos-http-live-streaming-06#section-3.3.3

This seems to imply that it should either be a key or none if included, unless I'm reading the RFC wrong?

leandromoreira commented 8 years ago

@davidjameshowell I think we should rely on the latest version of it https://tools.ietf.org/html/draft-pantos-http-live-streaming-20#section-4.3.2.4

4.3.2.4.  EXT-X-KEY

   Media Segments MAY be encrypted.  The EXT-X-KEY tag specifies how to
   decrypt them.  It applies to every Media Segment that appears between
   it and the next EXT-X-KEY tag in the Playlist file with the same
   KEYFORMAT attribute (or the end of the Playlist file).  Two or more
   EXT-X-KEY tags with different KEYFORMAT attributes MAY apply to the
   same Media Segment if they ultimately produce the same decryption
   key.  The format is:

   #EXT-X-KEY:<attribute-list>

   The following attributes are defined:

      METHOD

      The value is an enumerated-string that specifies the encryption
      method.  This attribute is REQUIRED.

      The methods defined are: NONE, AES-128, and SAMPLE-AES.

      An encryption method of NONE means that Media Segments are not
      encrypted.  If the encryption method is NONE, other attributes
      MUST NOT be present.

      An encryption method of AES-128 signals that Media Segments are
      completely encrypted using the Advanced Encryption Standard
      [AES_128] with a 128-bit key, Cipher Block Chaining, and PKCS7
      padding [RFC5652].  CBC is restarted on each segment boundary,
      using either the IV attribute value or the Media Sequence Number
      as the IV; see Section 5.2.

      An encryption method of SAMPLE-AES means that the Media Segments
      contain media samples, such as audio or video, that are encrypted
      using the Advanced Encryption Standard [AES_128].  How these media
      streams are encrypted and encapsulated in a segment depends on the
      media encoding and the media format of the segment.  fMP4 Media
      Segments are encrypted using the 'cbcs' scheme of Common
      Encryption [COMMON_ENC].  Encryption of other Media Segment
      formats containing H.264 [H_264], AAC [ISO_14496], AC-3 [AC_3],
      and Enhanced AC-3 [AC_3] media streams is described in the HLS
      Sample Encryption [SampleEnc] specification.  The IV attribute MAY
      be present; 

      The value is a quoted-string containing a URI that specifies how
      to obtain the key.  This attribute is REQUIRED unless the METHOD
      is NONE.

      IV

      The value is a hexadecimal-sequence that specifies a 128-bit
      unsigned integer Initialization Vector to be used with the key.
      Use of the IV attribute REQUIRES a compatibility version number of
      2 or greater.  See Section 5.2 for when the IV attribute is used.

      KEYFORMAT

      The value is a quoted-string that specifies how the key is
      represented in the resource identified by the URI; see Section 5
      for more detail.  This attribute is OPTIONAL; its absence
      indicates an implicit value of "identity".  Use of the KEYFORMAT
      attribute REQUIRES a compatibility version number of 5 or greater.

      KEYFORMATVERSIONS

      The value is a quoted-string containing one or more positive
      integers separated by the "/" character (for example, "1", "1/2",
      or "1/2/5").  If more than one version of a particular KEYFORMAT
      is defined, this attribute can be used to indicate which
      version(s) this instance complies with.  This attribute is
      OPTIONAL; if it is not present, its value is considered to be "1".
      Use of the KEYFORMATVERSIONS attribute REQUIRES a compatibility
      version number of 5 or greater.

   If the Media Playlist file does not contain an EXT-X-KEY tag then
   Media Segments are not encrypted.

   See Section 5 for the format of the key file, and Section 5.2,
   Section 6.2.3 and Section 6.3.6 for additional information on Media
   Segment encryption.