Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extend _parse_extinf to allow for #EXIINF tags like #EXTINF:-1 tvg-i… #145

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cbaurtx
Copy link

@cbaurtx cbaurtx commented Jul 14, 2019

…d=....., Title

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 97.313% when pulling 3078f81 on cbaurtx:cbaurtx-additions into a84b2ac on globocom:master.

5 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 97.313% when pulling 3078f81 on cbaurtx:cbaurtx-additions into a84b2ac on globocom:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 97.313% when pulling 3078f81 on cbaurtx:cbaurtx-additions into a84b2ac on globocom:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 97.313% when pulling 3078f81 on cbaurtx:cbaurtx-additions into a84b2ac on globocom:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 97.313% when pulling 3078f81 on cbaurtx:cbaurtx-additions into a84b2ac on globocom:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 97.313% when pulling 3078f81 on cbaurtx:cbaurtx-additions into a84b2ac on globocom:master.

@mauricioabreu
Copy link
Member

Thank you for contributing to this project @cbaurtx
Can you provide a test to ensure it is working?

@leandromoreira
Copy link
Contributor

leandromoreira commented Aug 10, 2019

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

The correct format for the EXTINF is:

4.3.2.1.  EXTINF

   The EXTINF tag specifies the duration of a Media Segment.  It applies
   only to the next Media Segment.  This tag is REQUIRED for each Media
   Segment.  Its format is:

   #EXTINF:<duration>,[<title>]

   where duration is a decimal-floating-point or decimal-integer number
   (as described in Section 4.2) that specifies the duration of the
   Media Segment in seconds.  Generally, durations SHOULD be decimal-
   floating-point, with enough accuracy to avoid perceptible error when
   segment durations are accumulated.  If the compatibility version
   number is less than 3, durations MUST be integers.  Durations that
   are reported as integers SHOULD be rounded to the nearest integer.
   The remainder of the line following the comma is an optional human-
   readable informative title of the Media Segment expressed as raw
   UTF-8 text.

It seems that there is no space for #EXTINF:-1 tvg-i format.

@leandromoreira
Copy link
Contributor

And our parser already takes the title from the official standard https://github.com/globocom/m3u8/blob/master/m3u8/parser.py#L186

@mauricioabreu
Copy link
Member

@leandromoreira thank you!
Let's wait for @cbaurtx feedback and closes this if he agrees our understanding.

@cbaurtx
Copy link
Author

cbaurtx commented Aug 11, 2019

Thank you for contributing to this project @cbaurtx
Can you provide a test to ensure it is working?

@leandromoreira thank you!
Let's wait for @cbaurtx feedback and closes this if he agrees our understanding.

What I did was just a quick hack. So I am all for improving this. I reckon I will run into similar requirements as you.
Right now I have two open questions:

  1. How do we come to a mutual understanding on the exact requirements?
    Did Leandro study this: https://tools.ietf.org/html/rfc8216 ?
    Should we base the next code changes on the write-up from Leandro or do we need to carefully
    study https://tools.ietf.org/html/rfc8216 ?
  2. There is (in this project) already a large body of example and test playlists. Do we need additional playlists, and if so how do we create these? What malformed playlists should be used for testing?

@mauricioabreu
Thank you so much for all your work!

@cbaurtx
Copy link
Author

cbaurtx commented Aug 11, 2019

Addendum:
There are playlists around, which do not adhere to the Standard you cited. Please check:
iheartradio/open-m3u8#34 and http://ss-iptv.com/en/users/documents/m3u.

Should we generate a dictionary with the attributes of the #EXTINF tag in case the non-strict option is selected?

@leandromoreira
Copy link
Contributor

leandromoreira commented Aug 11, 2019

For the question about where should we be looking to implement new features, yes it's https://tools.ietf.org/html/rfc8216

About the playlists around, which do not adhere to the Standard I think this lib should leave it open for custom parsers but not implement them inside this lib. For this matter we do have the custom_tags_parser which is demostrated by the test SIMPLE_PLAYLIST_WITH_CUSTOM_TAGS

https://github.com/globocom/m3u8/blob/master/tests/test_parser.py#L324-L335

@leandromoreira
Copy link
Contributor

leandromoreira commented Aug 11, 2019

by the way maybe we need to document this feature at the home page with the title "Custom parsers for non-standard tags" #130 and use the very test example to show it.

@cbaurtx
Copy link
Author

cbaurtx commented Aug 11, 2019

Thank you for the quick reply. I am good with sticking to https://tools.ietf.org/html/rfc8216.
Stricktly speaking just skipping attributes (that is what I did) is already outside the scope of https://tools.ietf.org/html/rfc8216 and you might elect not to use what I did so you can stick to https://tools.ietf.org/html/rfc8216.

I might have a wrong picture of custom tags. I thought these are new tags and not variations / extensions of existing tags. Or would you overload existing tag parsing with new functions?

@leandromoreira
Copy link
Contributor

Here you can see the implementation https://github.com/globocom/m3u8/pull/130/files#diff-f843380fa3bcd9ed49ae136974fa3aecR154

And here some usage:

SIMPLE_PLAYLIST_WITH_CUSTOM_TAGS = '''#EXTM3U
#EXT-X-MOVIE: million dollar baby
#EXT-X-TARGETDURATION:5220
#EXTINF:5220,
http://media.example.com/entire.ts
#EXT-X-ENDLIST
'''

def test_simple_playlist_with_custom_tags():
    def get_movie(line, data, lineno):
        custom_tag = line.split(':')
        if len(custom_tag) == 2:
            data['movie'] = custom_tag[1].strip()

    data = m3u8.parse(playlists.SIMPLE_PLAYLIST_WITH_CUSTOM_TAGS, strict=False, custom_tags_parser=get_movie)
    assert data['movie'] == 'million dollar baby'
    assert 5220 == data['targetduration']
    assert 0 == data['media_sequence']
    assert ['http://media.example.com/entire.ts'] == [c['uri'] for c in data['segments']]
    assert [5220] == [c['duration'] for c in data['segments']]

@SerhiyRomanov
Copy link
Contributor

@leandromoreira "Custom tag parser" is a very powerful functionality, but the problem is in the order of calling it.
In current realisation, the passed function has been called after parsing #EXTINF tag

elif line.startswith('#'):

It would better to change the priority of calling custom_tags_parser in such a way that it would be called first before calling any other parse* functions. Then we would be able to customize even parsing #EXTINF tag, so it would help to parse other non-standard attributes (tvg-id, group-title, etc.) instead of forking and patching the current repo (what I actually do)

What do you think about it? I'll try to prepare a pull request with these changes and examples of use

@leandromoreira
Copy link
Contributor

@SerhiyRomanov thanks I'm okay with this approach, what do you think? @mauricioabreu

@rokam
Copy link

rokam commented Mar 25, 2021

I think that we must be aware that the main parser should also be called. Or we always call the custom parser or adding some kind of TAG check to call it.
I think that always calling is safe although it'll be less optimal.

@mauricioabreu
Copy link
Member

@leandromoreira I am ok with that

@SerhiyRomanov tell me if you need any help with your pull request

@SerhiyRomanov
Copy link
Contributor

SerhiyRomanov commented Apr 5, 2021

@mauricioabreu I've made it #247
Only have to fix/write the tests and docs with examples.

Would be happy to get your comments.

@rokam
Copy link

rokam commented Apr 5, 2021

Nice job @SerhiyRomanov . It would be great to have a custom dumper as well.

@SerhiyRomanov
Copy link
Contributor

@rokam Yes, I'm thinking about it. But it seems slightly complicated to implement such functionality with current implementations of dumps(). I will create an issue for it after my PR become merged

@link89
Copy link

link89 commented Jan 8, 2022

I cannot find a spec for supporting attributes in EXTINF tag, but it is already an informal agreement among many iptv providers and clients, for example
https://ss-iptv.com/en/users/documents/m3u
https://en.f-player.ru/extm3u-iptv-format

#EXTINF: <duration> [attribute,...], [title]

I don't think to support the iptv dialect will do any harm. It's more like a superset of RFC8216 that won't break the compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants