rshendershot / MythRokuPlayer

mythtv front end for Roku player
tbd
8 stars 2 forks source link

Resize images #52

Closed dschwilk closed 9 years ago

dschwilk commented 9 years ago

On the fly downsizing of large images in order to use fanart and coverart. This needs some more changes, I'm sure before merging but I'm opening this to put discussion here.

See discussion in #8

This merge wil introduce a dependency on php imagemagick library. On ubuntu/kubuntu that is apt-get install php-imagick.

Some things this will need before merge:

rshendershot commented 9 years ago

Concerns - First, I'd be happier not to create another dep (ImageMagic, php-pecl-imagick on Fedora) but php-gd doesn't seem to have a createfromVid function. It does do all the necessary ops on a static image though and is already in the deps list. http://webcheatsheet.com/php/create_thumbnail_images.php

Second, the source for the image should be the recording not fanart or cover art. I have several cases for items not found online so there won't be any downloadable art. Where there is, each generated image would be the same for all of the same name because poster art is the same for a season, but screenshots are different for each within a season and help you tell them apart - which is the real reason for a preview image at its core I'd say.

My approach would be to create a snapshot and save it to $snapshots/$img (and update mythconverg.videometadata) so that any subsequent request won't need to re-generate it. From the MythRokuPlayer forum I've become aware of some users who have hundreds of items...

If you are still having problems with fan/screen/cover art file paths - did you define those storage groups in mythtv-setup?

dschwilk commented 9 years ago
  1. Re dependencies: I understand. I'll look into php-gd.
  2. Right, I think the priority order in player_feed works, though. Cover art and fanart only if the screenshot does not exist. Movies are the one case where cover art is more useful than a screenshot preview. Of course all of this is silly icing when we have perfectly good names. In fact, player_feed could be modified so that the resizing only happened for coverart and fanart (which should only occur for movies). My current version sends screenshots through the rescaling fuction too. But resizing must happen somewhere, right, since the roku itself still scales those unless they are the exact dimensions? I suppose make once and store as you suggest is best, but should that then be done for all screenshots as well? Really, I should test how various methods perform with lots of files, I guess.

My separate tv episode mythvideo issue: It seemed to be a problem where a screenshot storage dir was defined in the frontend as well as the backend. For some reason, some screenshots were saved to the frontend defined location, and myth knew about them but the file field obtained by player_feed had the full path which was then garbled by prepending the backend path. I made sure that storage paths are only set in the backend, moved, scanned, replaced and rescanned all files and it works now. So, my error.

rshendershot commented 9 years ago

well it seems faster. I have my server set to SD so the images are rough on my HD TV which wasn't a problem. I would have thought that Roku used ASIC or FPGA to do the resizing but perhaps not.

Since some of my Items don't match online and don't have any downloadable art, I handled the default

            }elseif(!empty($show->coverfile)){
                $coverart = StorageGroup::first( array('conditions' => array('groupname = ?', 'Coverart')) );
                $imgfile = $coverart->dirname . $show->coverfile;
            }else{
                $imgfile = "images/oval_grey.png";
            }

but that displays horribly

dschwilk commented 9 years ago

I would have thought that Roku used ASIC or FPGA to do the resizing

I'm sure you are right.

but that displays horribly

How so? It is wasteful in that that little gray oval png is being scaled up to 150px. One fix would be to check the static image size in image.php and only resize to width if the size was over some threshold. I Just added that, but sizes are all hard coded.

rshendershot commented 9 years ago

it had no background either. I think that's true of all of them b/c I can see a small border of black around them. on my integration branch I made a change so only the downloaded art was resized.

            $imgUrl = "$WebServer/$MythRokuDir/image.php?static=" . rawurlencode("images/oval_grey.png");
            if(!empty($show->screenshot)){
                $screenart = StorageGroup::first( array('conditions' => array('groupname = ?', 'Screenshots')) );
                $imgfile = $screenart->dirname . $show->screenshot;
                $imgUrl = "$WebServer/$MythRokuDir/image.php?imagegen=" . rawurlencode($imgfile);
            }elseif(!empty($show->fanart)){
                $fanart = StorageGroup::first( array('conditions' => array('groupname = ?', 'Fanart')) );
                $imgfile = $fanart->dirname . $show->fanart;
                $imgUrl = "$WebServer/$MythRokuDir/image.php?imagegen=" . rawurlencode($imgfile);
            }elseif(!empty($show->coverfile)){
                $coverart = StorageGroup::first( array('conditions' => array('groupname = ?', 'Coverart')) );
                $imgfile = $coverart->dirname . $show->coverfile;
                $imgUrl = "$WebServer/$MythRokuDir/image.php?imagegen=" . rawurlencode($imgfile);
            }
rshendershot commented 9 years ago

Only MythTV Items (Recorded,VideoMetadata) require generation and only, then, for Preview art. I'd welcome your thoughts.... imagesvc

dschwilk commented 9 years ago

Yes, I think that makes sense. So generated previews (currently image.php?preview=) would be resized as well?

rshendershot commented 9 years ago

Suppose SimpleImage supplied a Stream. The two uses of that in Item can then use simpleImage=>stream to find the StreamURL. Likewise it provides the necessary implementation of HD and SD image. They would generate/find the appropriate sized image for the Preview and what we've called imageGen.

dschwilk commented 9 years ago

Ok, so move more of the logic into the SimpleImage class? Or, perhaps, a separate new class? I may just be just hung up on names, but this would be something like a "video item" class which contains/supplies a stream url and the preview image (generated and resized preview, resized art, or fallback static oval). So that cleans up some of the logic in player_feed.php and encapsulates it in a class? Let me know if I'm misunderstanding.

rshendershot commented 9 years ago

video item class would be specialized as Videometadata and Recorded. They already extend the DB framework so can't, since multiple inheritance is not allowed in PHP, extend something like MythTvItem which would know about supplying the ImageSvc uses. It seems I've coded myself into a corner :(

rshendershot commented 9 years ago

You've demonstrated that resizing on-the-fly from the host can out-perform the Roku My suspicion is that PNG is not handled well by the Roku. $RokuDisplayType is used to select a url for the preview image, but the problem is only marginally reduced setting it to HD.

All of the controllers share the need to supply different sd_img and hd_img - but in many cases those are not differentiated at the source. One of the goals is to work well on old hardware. Offloading image resizing to the Roku is consistent with that goal.

The special case exists for MythTV Items which rely on the backend for the case of Recording, and external sites for the case of Videometadata, regarding the creation and maintenance of these poster images. An especial case exists for videometadata for which mythbackend was able to find online art, but which lacks the Screenshot resource. In that case a grey oval is currently displayed, as it is for most Not-Found conditions. In practice these are pretty rare.

Your strategy of grabbing an alternate from that downloaded art is a viable alternative. With much consideration, it is not going to be my alternative. The current controllers work in isolation with little dependency between them. To abstract the role of image provider to include finding and resizing would, in my view, consecrate a strategy that may well be at odds with future MythTV efforts; I've been told that providing multiple stream files for a Recording (perhaps even Videos) may happen - that would likely force changes in MRP (MythRokuPlayer). Keeping MRP code minimal and flexible is another of my goals.

Relying on the Roku for image resizing does present an issue which I've opened (https://github.com/rshendershot/MythRokuPlayer/issues/56) but with all the previous in mind, I close this request.

dschwilk commented 9 years ago

Fair enough, I agree that a simple and minimal MRP is desirable. I'll probably continue and try to clean up my fork but I won't worry about keeping it consistent with yours.