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.

Sign up for GitHub

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

Add video track and attachment extraction #45

Open
wants to merge 2 commits into
base: release/1.1
Choose a base branch
from
Open

Add video track and attachment extraction #45

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e30e32b
Add ability to extract video tracks and attachments
Feb 11, 2020
f7c64c5
Fix document style
Feb 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion pymkv/MKVFile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import json
from os import devnull
from os.path import expanduser, isfile
from os.path import expanduser, isfile, join, basename, splitext
import subprocess as sp

import bitmath
Expand Down Expand Up @@ -38,6 +38,7 @@ def __init__(self, file_path=None, title=None):
be used if it exists.
"""
self.mkvmerge_path = 'mkvmerge'
self.mkvextract_path = 'mkvextract'
self.title = title
self._chapters_file = None
self._chapter_language = None
Expand Down Expand Up @@ -69,6 +70,19 @@ def __init__(self, file_path=None, title=None):
new_track.forced_track = track['properties']['forced_track']
self.add_track(new_track)

# add attachment with info
for attachment in info_json['attachments']:
new_attachment = MKVAttachment(file_path,
name=attachment['file_name'],
description=attachment['description'])
if 'id' in attachment:
new_attachment.id = attachment['id']
if 'size' in attachment:
new_attachment.size = attachment['size']
if 'properties' in attachment and 'uid' in attachment['properties']:
new_attachment.uid = attachment['properties']['uid']
Comment on lines +75 to +83
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the mkvmerge output schema, it seems that "id", "size", and "properties" will always be present in an attachment's section of the info_json. Therefore, these should be added in the MKVAttachment constructor so they are documented and not just dynamically assigned.

Note that "uid" is not required within the "properties" section of the mkvmerge output schema. I recommend including "properties" as a parameter to the constructor, then within the constructor, check if "uid" exists and assign it to an attribute within the MKVAttachment object.

Perhaps an implementation that allows something like this:

new_attachment = MKVAttachment(
    file_path,
    name=attachment['file_name'],
    description=attachment['description'],
    id=attachment['id'],
    size=attachment['size'],
    properties=attachment['properties']
)

self.add_attachment(new_attachment)

# split options
self._split_options = []

Expand Down Expand Up @@ -663,3 +677,34 @@ def flatten(item):
return flat_list
else:
return [item]

def extract_video_tracks(self, out_folder=''):
"""Extract video tracks."""
names = []
name_args = []
for track in self.tracks:
if track._track_type == 'video':
bname = splitext(basename(track.file_path))[0]
name = '{}.mp4'.format(join(out_folder, bname + "_" + track.track_name))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three comments for this line:

  1. Before using the out_folder parameter, you should run it through os.path.expanduser to ensure output directories like ~/data/ are compatible.

  2. track_name is an optional attribute in MKVTrack and will default to None. According to the mkvmerge output schema, "codec", "id", and "type" are the only required properties in a track. I would suggest combining these properties along with the filename to produce unique names for each video track.

  3. Video tracks are not guaranteed to be compatible with mp4 containers. The mkvextract docs list the different types of video tracks here (the video tracks are prepended with a "V_"). I think the best solution for the time being is to create a dictionary that maps each video track type to a compatible container type and use this to decide on an extension.

name_args.append('{}:{}'.format(track._track_id, name))
names.append(name)
file_path = track._file_path

sp.run([self.mkvextract_path, 'tracks', file_path] + name_args,
stdout=open(devnull, 'wb'))
return names

def extract_attachments(self, out_folder=''):
"""Extract attachments."""
names = []
name_args = []
for attachment in self.attachments:
bname = splitext(basename(attachment.file_path))[0]
name = join(out_folder, bname + "_" + attachment.name)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concerns as part 1 of the comment on line 688. As for the naming of attachment files, I think it would be more appropriate to keep the attachment name specified and not prepend the basename.

name_args.append('{}:{}'.format(attachment.id, name))
names.append(name)
file_path = attachment._file_path

sp.run([self.mkvextract_path, 'attachments', file_path] + name_args,
stdout=open(devnull, 'wb'))
return names