Support parsing for m3u and m3u8 files #187
Conversation
|
You can also add this new format in |
lib/parsers/m3u_parser.rb
Outdated
|
|
||
| FormatParser::Text.new( | ||
| format: :m3u, | ||
| size: io.size |
There was a problem hiding this comment.
Just want to be sure: will this read the whole file in order to get the size? Because if it does, it might be wiser not to include it (or implement it in a different way).
There was a problem hiding this comment.
size is available once a single read has been performed and will not incur any extra reads
There was a problem hiding this comment.
Everything looks fine - I would be careful with using a property called size for a Text, the reason is that it is unclear what the unit of size is. Is it a byte? (in this case it is). Is it a codepoint? (if the data read was a String in UTF8 and we would call size on it it would be). Is it a grapheme cluster? Number of lines? Number of paragraphs?
Also I wonder - do we need to pass the size of the IO to the caller? How is this information going to be used? Maybe we could surface the computed IO size in bytes in all Result types if that information is useful and can avoid an extra HTTP request?
lib/parsers/m3u_parser.rb
Outdated
|
|
||
| FormatParser::Text.new( | ||
| format: :m3u, | ||
| size: io.size |
There was a problem hiding this comment.
size is available once a single read has been performed and will not incur any extra reads
Thanks for your inputs @julik! I just wanted to add some more information about the file along with format. It would not be used by the previewing service but I thought it might be meaningful information for other services using FormatParser outside of our org. If you think this can be removed for now, I will skip it. We can always look into it again if there are any requests to include size. |
|
@nitika080289 Let's remove it and replace it later with, say, a |
Closes #186
While investigating about the m3u files, I found that there can be three types supported. A plain text file, a file with extended directives and file containing hls m3u extensions.
https://en.wikipedia.org/wiki/M3U
Files which are extracted playlists from media players fall into second and third category. I think that recognising a plain text file as m3u just on the basis of file extension might not be the right thing to do and there is no other way of recognising it as per my understanding.
The second and third category files contain a header #EXTM3U which can be used to recognise them. This is what I am currently doing here. So, this logic will not recognise a plain text m3u file but will be able to recognise an extended m3u or hls m3u extension.
Also, the directives vary for the second and third category and seems complicated to extract the information from these files. I will create another ticket for extracting more information from the files and work on it separately