Skip to content

Commit

Permalink
Fix resizing using the daemon: base64 encode the data before the tran…
Browse files Browse the repository at this point in the history 
…sfer, don't send the length, as it could interfere with the connections expectation to hung up on cr/lf.
  • Loading branch information
@mherger
mherger committed May 20, 2019
1 parent 42dee47 commit cb9d313
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 68 deletions.
78 changes: 37 additions & 41 deletions Slim/Utils/ImageResizer.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package Slim::Utils::ImageResizer;
use strict;

use File::Spec::Functions qw(catdir);
use MIME::Base64 qw(encode_base64);
use Scalar::Util qw(blessed);

use Slim::Utils::ArtworkCache;
Expand All @@ -21,7 +22,7 @@ my $log = logger('artwork');
my ($gdresizein, $gdresizeout, $gdresizeproc);

my $pending_requests = 0;
my $hasDaemon;
my $hasDaemon;

sub hasDaemon {
my ($class, $check) = @_;
Expand All @@ -31,94 +32,89 @@ sub hasDaemon {
unlink SOCKET_PATH;
};
}

return $hasDaemon;
}

sub resize {
my ($class, $file, $cachekey, $specs, $callback, $cache) = @_;

my $isDebug = main::DEBUGLOG && $log->is_debug;

# Check for callback, and that the gdresized daemon running and read/writable
if (hasDaemon() && $callback) {
require AnyEvent::Socket;
require AnyEvent::Handle;

# Get cache root for passing to daemon
$cache ||= Slim::Utils::ArtworkCache->new();
my $cacheroot = $cache->getRoot();

main::DEBUGLOG && $isDebug && $log->debug("Using gdresized daemon to resize (pending requests: $pending_requests)");

$pending_requests++;

# Daemon available, do an async resize
AnyEvent::Socket::tcp_connect( 'unix/', SOCKET_PATH, sub {
my $fh = shift || do {
main::DEBUGLOG && $isDebug && $log->debug("daemon failed to connect: $!");
$log->error("daemon failed to connect: $!");

if ( --$pending_requests == 0 ) {
main::DEBUGLOG && $isDebug && $log->debug("no more pending requests");
}

# Fallback to resizing the old way
sync_resize($file, $cachekey, $specs, $callback, $cache);

return;
};

my $handle;

# Timer in case daemon craps out
my $timeout = sub {
main::DEBUGLOG && $isDebug && $log->debug("daemon timed out");
$log->error("daemon timed out");

$handle && $handle->destroy;

if ( --$pending_requests == 0 ) {
main::DEBUGLOG && $isDebug && $log->debug("no more pending requests");
}

# Fallback to resizing the old way
sync_resize($file, $cachekey, $specs, $callback, $cache);
};
Slim::Utils::Timers::setTimer( undef, Time::HiRes::time() + SOCKET_TIMEOUT, $timeout );

$handle = AnyEvent::Handle->new(
fh => $fh,
on_read => sub {},
on_eof => undef,
on_error => sub {
my $result = delete $_[0]->{rbuf};

main::DEBUGLOG && $isDebug && $log->debug("daemon result: $result");

$_[0]->destroy;

Slim::Utils::Timers::killTimers(undef, $timeout);

if ( --$pending_requests == 0 ) {
main::DEBUGLOG && $isDebug && $log->debug("no more pending requests");
}

$callback && $callback->();
},
);

# need to protect \n\r in the file from ending the sending prematurely
# this is NOT 100% safe, as it might break
if (ref $file) {
$$file =~ s/\n/\\0x12/sg;
$$file =~ s/\r/\\0x15/sg;
}

$handle->push_write( pack('Z* Z* Z* Z* I/a*', ref $file ? 'data' : $file, $specs, $cacheroot, $cachekey, ref $file ? $$file : '') . "\015\012" );

main::INFOLOG && $log->is_info && $log->info(sprintf("file=%s, spec=%s, cacheroot=%s, cachekey=%s, imagedata=%s bytes\n", ref $file ? 'data' : $file, $specs, $cacheroot, $cachekey, (ref $file ? length($$file) : 0)));

$handle->push_write( pack('Z* Z* Z* Z* Z*', ref $file ? 'data' : $file, $specs, $cacheroot, $cachekey, (ref $file ? encode_base64($$file, '') : '')) . "\015\012" );
}, sub {
# prepare callback, used to set the timeout
return SOCKET_TIMEOUT;
} );

return;
}
else {
Expand All @@ -129,13 +125,13 @@ sub resize {

sub sync_resize {
my ( $file, $cachekey, $specs, $callback, $cache ) = @_;

require Slim::Utils::GDResizer;

my $isDebug = main::DEBUGLOG && $log->is_debug;

my ($ref, $format);

my @spec = split(',', $specs);
eval {
($ref, $format) = Slim::Utils::GDResizer->gdresize(
Expand All @@ -146,14 +142,14 @@ sub sync_resize {
debug => $isDebug,
);
};

if ( main::DEBUGLOG && $isDebug && $@ ) {
$file = '' if ref $file;
$log->error("Error resizing $file: $@");
}

$callback && $callback->($ref, $format);

return $@ ? 0 : 1;
}

Expand Down
52 changes: 25 additions & 27 deletions gdresized.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ BEGIN
use Config;
use File::Spec::Functions qw(catdir);
use Slim::Utils::OSDetect; # XXX would be nice to do without this

my $libPath = $Bin;
my @SlimINC = ();

Slim::Utils::OSDetect::init();

if (my $libs = Slim::Utils::OSDetect::dirsFor('libpath')) {
# On Debian, RH and SUSE, our CPAN directory is located in the same dir as strings.txt
$libPath = $libs;
Expand All @@ -44,20 +44,20 @@ BEGIN
my $arch = $Config::Config{'archname'};
$arch =~ s/^i[3456]86-/i386-/;
$arch =~ s/gnu-//;

# Check for use64bitint Perls
my $is64bitint = $arch =~ /64int/;

# Some ARM platforms use different arch strings, just assume any arm*linux system
# can run our binaries, this will fail for some people running invalid versions of Perl
# but that's OK, they'd be broken anyway.
if ( $arch =~ /^arm.*linux/ ) {
$arch = $arch =~ /gnueabihf/
? 'arm-linux-gnueabihf-thread-multi'
$arch = $arch =~ /gnueabihf/
? 'arm-linux-gnueabihf-thread-multi'
: 'arm-linux-gnueabi-thread-multi';
$arch .= '-64int' if $is64bitint;
}

# Same thing with PPC
if ( $arch =~ /^(?:ppc|powerpc).*linux/ ) {
$arch = 'powerpc-linux-thread-multi';
Expand All @@ -76,8 +76,8 @@ BEGIN
catdir($libPath,'CPAN','arch',$perlmajorversion, $Config::Config{'archname'}, 'auto'),
catdir($libPath,'CPAN','arch',$Config::Config{'archname'}),
catdir($libPath,'CPAN','arch',$perlmajorversion),
catdir($libPath,'lib'),
catdir($libPath,'CPAN'),
catdir($libPath,'lib'),
catdir($libPath,'CPAN'),
$libPath,
);

Expand Down Expand Up @@ -127,40 +127,38 @@ BEGIN

while (1) {
my $client = $socket->accept();

eval {
DEBUG && (my $tv = Time::HiRes::time());

# get command
my $buf = <$client>;
my ($file, $spec, $cacheroot, $cachekey, $data) = unpack 'Z* Z* Z* Z* I/a*', $buf;

my ($file, $spec, $cacheroot, $cachekey, $data) = unpack 'Z* Z* Z* Z* Z*', $buf;

if ($data) {
# trim cr/lf from the end
$data =~ s/\015\012$//s;
$data =~ s/\\0x12/\n/sg;
$data =~ s/\\0x15/\r/sg;
require MIME::Base64;
$data = MIME::Base64::decode_base64($data);
}

# An empty spec is allowed, this returns the original image
$spec ||= 'XxX';

my $imageproxy = $cachekey =~ /^imageproxy/;
DEBUG && warn sprintf("file=%s, spec=%s, cacheroot=%s, cachekey=%s, imageproxy=%s, imagedata=%s bytes\n", $file, $spec, $cacheroot, $cachekey, $imageproxy || 0, length($data));

if ( !$file || !$spec || !$cacheroot || !$cachekey ) {
die "Invalid parameters: $file, $spec, $cacheroot, $cachekey\n";
die sprintf("Invalid parameters: file=%s, spec=%s, cacheroot=%s, cachekey=%s, imageproxy=%s, imagedata=%s bytes\n", $file, $spec, $cacheroot, $cachekey, $imageproxy || 0, length($data || ''));
}

my @spec = split ',', $spec;

my $cache = $imageproxy ? $imageProxyCache : $artworkCache;
if ( $cache->getRoot() ne $cacheroot ) {
$cache->setRoot($cacheroot);
$cache->pragma('locking_mode = NORMAL');
}

# do resize
Slim::Utils::GDResizer->gdresize(
file => $data ? \$data : $file,
Expand All @@ -170,13 +168,13 @@ BEGIN
cachekey => $cachekey,
spec => \@spec,
);

# send result
print $client "OK\015\012";

DEBUG && warn "OK (" . (Time::HiRes::time() - $tv) . " seconds)\n";
};

if ( $@ ) {
print $client "Error: $@\015\012";
warn "$@\n";
Expand Down

0 comments on commit cb9d313

Please sign in to comment.