# Reading a proprietary file system and creating files

The goal of this software is to read the proprietary file system index of a digital video recorder and to carve all of the video chunks into files within a dated directory structure.

I'm not very experienced and working in isolation so would like some feedback on the code in general in terms of style/efficiency and whatever else you may deem relevant.

import datetime
import os

from bitstring import ConstBitStream

class HikvisionDrive:

def __init__(self, file_loc):
# create data stream of hard drive
self.file = file_loc
self.data = ConstBitStream(filename=self.file)

# get information from master sector
self.data.pos = 584 * 8

self.data.pos += 56 * 8

self.data.pos += 4 * 8

# get information from hikbtree
self.data.pos = (self.hikbtree_offset + 80) * 8

# initialise list for data blocks
self.data_blocks = []

def show_info(self):
print('-Hard Drive Information-')
print('File: ' + str(self.file))
print('Capacity: ' + str(self.capacity) + ' bytes')
print('Data Block Size: ' + str(self.data_block_size) + ' bytes')
print('Number of Data Blocks: ' + str(self.total_data_blocks))
print('HikBTree Offset: ' + str(self.hikbtree_offset))
print('HikBTree Size: ' + str(self.hikbtree_size) + ' bytes')
print('Page List Offset: ' + str(self.page_list_offset))

def save_blocks(self, save_directory):
# loop through data blocks and save the data into a structured directory
# setup progress counter
x = 1
with open(self.file, 'rb') as f1:
for data_block in self.data_blocks:
# update progress
print('saving data block #', x, 'of', self.total_data_blocks, '...')
# set directory
date = data_block['start_time']
save_dir = save_directory + '\\' + str(date.year) + '-' + date.strftime('%B') + '\\'\
+ date.strftime('%d')
if not os.path.exists(save_dir):
os.makedirs(save_dir)
# save the data to a file
file_name = 'CH' + str(data_block['channel']) + '_' + date.strftime('%H') + '-' + date.strftime(
'%M') + '-' + date.strftime('%S') + '_block' + str(x) + '.raw'
length = self.data_block_size
f1.seek(data_block['offset'])
with open(str(save_dir + '\\' + file_name), 'wb') as f2:
while length:
chunk = min(1024 * 1024, length)
f2.write(data)
length -= chunk
f2.close()
x += 1
f1.close()

def scan_pages(self):
# create list of page offsets
page_offsets = []
# get the offset to Page #1
self.data.pos = (self.page_list_offset + 24) * 8
# loop through the rest of the list
self.data.pos += 64 * 8
while page_offset != 0:
page_offsets.append(page_offset)
self.data.pos += 40 * 8
# create list of data blocks from the pages
for page in page_offsets:
self.data.pos = (page + 96) * 8
entry = True if self.data.read('hex:64') == 'ffffffffffffffff' else False
while entry:
video_exists = True if self.data.read('hex:64') == '0000000000000000' else False
self.data.pos += 1 * 8
self.data.pos += 6 * 8
self.data_blocks.append({'channel': channel, 'has_video': video_exists, 'start_time': start_time,
'end_time': end_time, 'offset': data_offset})
# move to next block entry
self.data.pos += 8 * 8
entry = True if self.data.read('hex:64') == 'ffffffffffffffff' else False

drive = HikvisionDrive(r'D:\213_HDD.001')
drive.show_info()
drive.scan_pages()
drive.save_blocks(r'D:\Carved Files')


There are a couple of obvious issues:

1. Get used to writing docstrings. When you declare a function and then write a comment describing what it does, you're wrong. Python uses docstrings for this, which have the advantage that they can be accessed and manipulated at runtime (for example, in the REPL command line):

def save_blocks(self, save_directory):
# loop through data blocks and save the data into a structured directory
# setup progress counter


def save_blocks(self, save_directory):
"""Loop through data blocks and save the data into a structured directory.

"""
# setup progress counter

2. You're not looping like a native. See this link to Ned Batchelder's "Loop like a Native" talk for slides and video.

In particular, you are writing "read and store" code, when you should be writing "generate when needed" code. There's no reason to process the blocks by first loading all of them into memory. And, considering you are talking about video files, there's plenty of reasons not to load all of them into memory! Instead, use a generator to yield one item at a time, as needed.

And that whole x += 1 thing. Ugh! enumerate!

3. Use the library! Say "Hello!" to my leetle friend: pathlib

date = data_block['start_time']
save_dir = save_directory + '\\' + str(date.year) + '-' + date.strftime('%B') + '\\'\
+ date.strftime('%d')
if not os.path.exists(save_dir):
os.makedirs(save_dir)


Becomes:

from pathlib import Path    # At Top of module

date = data_block['start_time']
save_path = Path(save_directory) / date.strftime('%Y-%B') / date.strftime('%d')

if not save_path.exists():
save_path.mkdir(parents=True)


Note the changes to strftime, but I kept the subdirectory %d separate to allow Path's operator-/ to do its magic on Windows/Unix paths.

• Thanks! I am with you on the docstrings, enumeration and pathlib library. All very helpful tips. I'm not sure I have understood the generate when needed though, is this referring to me saving the data_block information to a list and then having another method to carve the data_blocks? – DanWheeler Dec 15 '17 at 23:05
• Pretty much any time you find yourself writing = [] you should ask, "Am I about to store a list that I should just generate on the fly?" – Austin Hastings Dec 15 '17 at 23:12
• Ok, I've also just got to the generator part of the video you linked so hopefully i'll understand shortly haha. Thanks for your help. – DanWheeler Dec 15 '17 at 23:14

Okay I know I'm late to the party but I wanted to give my impressions on this code too.

• Functions/methods are too long and are doing too much. When a function is longer than a screenful, it becomes very hard to keep everything in mind. Functions should obey the single responsibility principle: have one clearly defined task that it performs.
• Too many magic numbers. There are numbers and strings scattered throughout the code and I have no idea where any of them come from or what they mean. Some of them have comments near them, but it's still inscrutable.
• Lots of repeated code. For instance all the self.data.read('uintle:64') Maybe should be their own method.
• Split out the generation of the string in show_info() into a new method __str__() since representing making the object readable. Then show_info() should just call str(self)
• Need some error/exception detection in places where you work with files.
• Don't call print() in the main logic of your code. What if you code has to be run some place where there isn't a terminal, such as a batch process or GUI? Instead look into using the logging module.
• Thanks for the tips! With regards self.data.read('uintle:64') being its own method. Is it not already its own method just as part of another class? I suppose i could have a specific method called "read_64_bits" that does not require an argument but I'm not sure i understand the benefit. Is it readability? – DanWheeler Dec 19 '17 at 7:47
• @DanWheeler Readability is a concern, yes. Maintainability is the other, more important one. It's because "uintle:64" is a "magic" string, and it's easy to get wrong. When I was trying to answer I typed "unitle64" the first time, even though I thought I was being careful. If the data format ever changes and you need another 64-bit unsigned int, you have to make sure you get the string right. Either the string itself should be put into a variable, or the whole thing should be its own method. Don't repeat yourself – Snowbody Dec 19 '17 at 14:12
• Of course, that makes perfect sense. And for what it's worth, placing it into a method has made the code much nicer to read. Along with clearing out the rest of the magic numbers and trimming the functions down. – DanWheeler Dec 19 '17 at 14:15