Performance improvements #12

Merged
merged 13 commits into from Apr 5, 2012

Conversation

Projects
None yet
3 participants
Collaborator

zane131 commented Mar 3, 2012

  • Added support for .m4v and .mov files.
  • Various performance enhancements, including a removing redundant call to get the entire list of video files when displaying the details screen.
  • Improvements in code organization to remove redundancies in code and increase maintainability.
  • Improved look and feel of Roku interface and mythtv_*_test.php.
  • Added the ability to limit the number of video files that are displayed on the poster screen (huge performance increase when you have thousands of video files).
Collaborator

rshendershot commented Mar 4, 2012

I'm trying your branch but seem to have settings.php removed? can you pls check and maybe re-add it?

Collaborator

zane131 commented Mar 4, 2012

Oops. That file can get annoying when you want to make changes to it but not include your personal settings. I fixed the mistake and amended the last commit.

Collaborator

rshendershot commented Mar 6, 2012

I'm now getting errors in the brightscript as well as mythtv_tv_test.php.

Type Mismatch. (runtime error &h18) in .../pkg:/source/categoryFeed.brs(16)

Class 'DomDocument' not found in /usr/share/mythweb/MythRokuPlayer_zane131/MythRokuPlayer/mythroku/mythtv_tv_test.php on line 14

I do get a valid looking XML from mythtv.php and mythtv_tv_xml.php when run from the server CLI so that's good ;)

Let me know what you find out.

Collaborator

zane131 commented Mar 6, 2012

@rshendershot

The error in categoryFeed.brs must have been a merge bug. I'll push a fix sometime tomorrow. In the meantime, you can simply replace "ServerURL" with "MythRokuServerURL" in that line of code. it probably wasn't necessary to change the name of this registry, but I was unsure of the scoping mechanisms of the Roku's registry, so i tried to create a more unique name.

Apparently, PHP class 'DomDocument' is only in PHP 5 and I am assuming you are on a down level version of PHP, which is unfortunate. I am sure there are others that will have the same problem so I will investigate a solution for this bug.

Collaborator

rshendershot commented Mar 6, 2012

here's my PHP version. Let me know when you think it's ready to try. Are
you testing a fresh checkout as part of your process?

php -version
PHP 5.3.10 (cli) (built: Feb 2 2012 22:12:16)
Copyright (c) 1997-2012 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2012 Zend Technologies

Collaborator

zane131 commented Mar 7, 2012

@rshendershot I pushed a fix for the registry bug. I am baffled by the PHP issue. You have a newer version of PHP than I do:

$ php -version
PHP 5.3.5-1ubuntu7.7 with Suhosin-Patch (cli) (built: Feb 11 2012 06:42:47)
Copyright (c) 1997-2009 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend Technologies

According to php.net, class 'DomDocument' should be supported in PHP 5. I will investigate this further. For reference, I did not like the dual maintenance between mythtv_[movies|tv]xml.php and mythtv[movies|tv]test.php. I tried to eliminate it by simply applying a style sheet on the XML generated from mythtv[movies|tv]_xml.php. I hope we can find an easy solution, because I really don't want to revert back to the dual maintenance.

Collaborator

rshendershot commented Mar 7, 2012

@zane131

I installed fedora package php-xml and now have DomDocument class available.

with mythtv_movies_xml.php I get: "PHP Notice: Undefined variable:
episode in
/usr/local/src/MythRokuPlayer_zane131/MythRokuPlayer/mythroku/mythtv_movies_xml.php
on line 118"

and with both movies and tv xml I get: "PHP Notice: Undefined offset: 1
in
/usr/local/src/MythRokuPlayer_zane131/MythRokuPlayer/mythroku/mythtv_movies_xml.php
on line 9" when run in CLI. I don't pass any args. What is your thinking
with argV? It's already in the session as $_GET. I think we should take
this offline. I have a few other concerns about the implementation.
rshendershot@gmail.com

Collaborator

zane131 commented Mar 9, 2012

I swear this code works, but apparently it wasn't as clean as I had hoped. These errors never showed up when I was testing in Firefox, but they definitely show when I run the scripts from the command line. I have fixed all errors that I could find and amended the top most commit in this branch. Please let me know if you have any other issues.

Collaborator

rshendershot commented Mar 9, 2012

you might want to put mysql_close($db_handle); inside the if or a try/catch. When I set it to point to a non-existent mysql server ip it still tries to close the non-existent connection.

I'm still looking at other things so I'll reply your email asap.

Collaborator

zane131 commented Mar 11, 2012

I fixed the problem with mysql_close($db_handle). Also, I changed how the test.php scripts get the XML data. The previous implementation was undesirable.

Collaborator

rshendershot commented Mar 13, 2012

I'm getting "no feed body found" from the roku debug port. This seems to be a URL problem. mythtv_xml.php?type=rec&sort=title works but the url gets parsed by brightscript to mythtv_xml.php?type=rec&sort=title which is what is actually requested. From apache access_log:

GET /mythweb/zane/mythtv_xml.php?type=rec&sort=title HTTP/1.1" 200 136 "-" "Roku/DVP-4.2 (024.02E01006A)

which is the logged request from the roku to display mythroku TV at the Title listing.

Collaborator

zane131 commented Mar 14, 2012

@rshendershot I am not sure I understand the reason for the bug. If I hard code '&', it fails, however, if I use htmlspecialchars(), which replaces '&' with '&', it works. Regardless, it is working now in the latest commit.

Collaborator

rshendershot commented Mar 14, 2012

there are still some problems:

if I test it mythweb/zane/mythtv_test.php I get two links. Vid works but TV does not.
http://192.168.1.8/mythweb/zane/mythtv_test.php?type=rec

If I view page source from mythweb/zane/mythtv.php and if I then copy one of the links (which I show below) to my browser address, I get the no feed body problem still. Here's the rendering in page source:
feed="http://192.168.1.8/mythweb/zane/mythtv_xml.php?type=rec&sort=title" />
This seems to be the extra amp; as I can remove that and it will work.

Collaborator

zane131 commented Mar 15, 2012

@rshendershot I introduced a bug when I joined two tables in one of the SQL queries. It is fixed in the latest commit.

@ear9mrn ear9mrn added a commit that referenced this pull request Apr 5, 2012

@ear9mrn ear9mrn Merge pull request #12 from zane131/performance_improvements
Performance improvements
a3e01ac

@ear9mrn ear9mrn merged commit a3e01ac into ear9mrn:master Apr 5, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment