Index: main/trunk/greenstone2/perllib/plugins/ConvertBinaryFile.pm
===================================================================
--- main/trunk/greenstone2/perllib/plugins/ConvertBinaryFile.pm	(revision 31765)
+++ main/trunk/greenstone2/perllib/plugins/ConvertBinaryFile.pm	(revision 31766)
@@ -263,4 +263,22 @@
     my $ensure_path_absolute = 1; # true
     &FileUtils::softLink($input_filename, $tmp_filename, $ensure_path_absolute);
+
+    my $output_filename = $self->run_conversion_command($tmp_dirname, $tmp_filename,
+							$utf8_tailname, $lc_suffix, $tailname, $suffix);
+
+    return $output_filename;
+}
+
+# The latter half of tmp_area_convert_file: runs the conversion command and returns the output file name
+# Split from tmp_area_convert_file because UnknownConverterPlugin can then inherit all of 
+# tmp_area_convert_file and only needs to override this part:
+sub run_conversion_command {
+    my $self = shift (@_);
+    my ($tmp_dirname, $tmp_filename, $utf8_tailname, $lc_suffix, $tailname, $suffix) = @_;    
+
+    my $outhandle = $self->{'outhandle'};
+    my $convert_to = $self->{'convert_to'};
+    my $failhandle = $self->{'failhandle'};
+
     my $verbosity = $self->{'verbosity'};
     if ($verbosity > 0) {
Index: main/trunk/greenstone2/perllib/plugins/UnknownConverterPlugin.pm
===================================================================
--- main/trunk/greenstone2/perllib/plugins/UnknownConverterPlugin.pm	(revision 31765)
+++ main/trunk/greenstone2/perllib/plugins/UnknownConverterPlugin.pm	(revision 31766)
@@ -45,5 +45,6 @@
 # At present, a file or folder of files is assumed.
 # Need to look in there for files with extension process_ext.
-# Do we also need a html_multi option to convert_to? Support html_multi as output?
+# Do we also need a html_multi option to convert_to? If supporting html_multi as output, 
+# see PowerPointPlugin::read(), and revision 31764 of UnknownConverterPlugin.pm
 # Then a folder of html files is generated per document? 
 # OR Flag that indicates whether an html file + associated folder (such as of images) gets generated. And name of assoc folder. Such output gets generated for instance when a doc file is replaced by its html version.
@@ -104,8 +105,8 @@
 
 my $outhandle = $self->{'outhandle'};
-    print STDERR "\n\n**** convert_to is |" . $self->{'convert_to'} . "|\n\n";
     if(!defined $self->{'convert_to'}) {
-	$self->{'convert_to'} = "text"; # why do I have to set a value for convert_to here, when a default's already set at the start of this file???????
-    }
+	$self->{'convert_to'} = "text"; # why do I have to set a value for convert_to here, when a default's already set in $convert_to_list declaration????
+    }
+    #print STDERR "\n\n**** convert_to is |" . $self->{'convert_to'} . "|\n\n";
 
     # Convert_To set up, including secondary_plugins for processing the text or html generated
@@ -151,4 +152,5 @@
 
 # Are init, begin and deinit necessary (will they not get called automatically)?
+# Dr Bainbridge says it doesn't hurt for these to be explicitly defined here.
 # Copied here from PDFPlugin, PowerPointPlugin
 # https://stackoverflow.com/questions/42885207/why-doesnt-class-supernew-call-the-constructors-of-all-parent-classes-when
@@ -176,60 +178,27 @@
 }
 
-# overridden to run the custom conversion command here in place of gsConvert.pl called by ConvertBinaryFile.pm
-sub tmp_area_convert_file {
-    # should we first hardlink the output files/folder to tmp area, so we won't be working across drives?
-
-    my $self = shift (@_);
-    my ($output_ext, $input_filename, $textref) = @_;
-
-    #### COPIED FROM ConvertBinaryFile::tmp_area_convert_file()
+# Called by ConvertBinaryFile::tmp_area_convert_file() to do the actual conversion
+# In order to call the custom conversion command, UnknownConverterPlugin needs to know the actual 
+# input filename (which is the tmp_filename parameter) and the output file name, which this subroutine
+# will work out. Then it will run the conversion command.
+sub run_conversion_command {
+    my $self = shift (@_);
+    my ($tmp_dirname, $tmp_filename, $utf8_tailname, $lc_suffix, $tailname, $suffix) = @_;    
+    
     my $outhandle = $self->{'outhandle'};
     my $convert_to = $self->{'convert_to'};
     my $failhandle = $self->{'failhandle'};
-    my $convert_to_ext = $self->{'convert_to_ext'}; #set by ConvertBinaryFile::set_standard_convert_settings()
-    
-
-    my $upgraded_input_filename = &util::upgrade_if_dos_filename($input_filename);
-
-    # derive tmp filename from input filename
-    my ($tailname, $dirname, $suffix)
-	= &File::Basename::fileparse($upgraded_input_filename, "\\.[^\\.]+\$");
-
-    # softlink to collection tmp dir
-    my $tmp_dirname = &util::get_timestamped_tmp_folder();
-    if (defined $tmp_dirname) {
-	$self->{'tmp_dir'} = $tmp_dirname;
-    } else {
-	$tmp_dirname = $dirname;
-    }
-    
-#    # convert to utf-8 otherwise we have problems with the doc.xml file later on
-#    my $utf8_tailname = (&unicode::check_is_utf8($tailname)) ? $tailname : $self->filepath_to_utf8($tailname);
-
-    # make sure filename to be used can be stored OK in a UTF-8 compliant doc.xml file
-     my $utf8_tailname = &unicode::raw_filename_to_utf8_url_encoded($tailname);
-
-
-    # URLEncode this since htmls with images where the html filename is utf8 don't seem
-    # to work on Windows (IE or Firefox), as browsers are looking for filesystem-encoded
-    # files on the filesystem.
-    $utf8_tailname = &util::rename_file($utf8_tailname, $self->{'file_rename_method'}, "without_suffix");
-
-    my $lc_suffix = lc($suffix);
-    my $tmp_filename = &FileUtils::filenameConcatenate($tmp_dirname, "$utf8_tailname$lc_suffix");
-    
-    # If gsdl is remote, we're given relative path to input file, of the form import/utf8_tailname.suffix
-    # But we can't softlink to relative paths. Therefore, we need to ensure that
-    # the input_filename is the absolute path, see http://perldoc.perl.org/File/Spec.html
-    my $ensure_path_absolute = 1; # true
-    &FileUtils::softLink($input_filename, $tmp_filename, $ensure_path_absolute);
     my $verbosity = $self->{'verbosity'};
+    
+    my $convert_to_ext = $self->{'convert_to_ext'};
     if ($verbosity > 0) {
 	print $outhandle "Converting $tailname$suffix to $convert_to format with extension $convert_to_ext\n";
     }
 
-    my $errlog = &FileUtils::filenameConcatenate($tmp_dirname, "err.log");
-    
- 
+    # The command to be executed must be provided the input filename and output file/dir name
+    # input filename = tmp_filename
+    # 1. We now work out the output filename. Code for it comes from
+    # ConvertBinaryFile::tmp_area_convert_file(), but slightly modified
+
     my $output_type=$self->{'convert_to'};
 
@@ -259,7 +228,6 @@
     }
 
-    #### END COPIED FROM ConvertBinaryFile::tmp_area_convert_file()
-
-    # Execute the conversion command and get the type of the result,
+
+    # 2. Execute the conversion command and get the type of the result,
     # making sure the converter gives us the appropriate output type
 
@@ -276,9 +244,4 @@
 	return "";
     }
-
-    # HARDCODING CMD FOR NOW
-    #$cmd ="/Scratch/ak19/gs3-svn-15Nov2016/packages/jre/bin/java -cp \"/Scratch/ak19/gs3-svn-15Nov2016/gs2build/ext/pdf-box/lib/java/pdfbox-app.jar\" -Dline.separator=\"<br />\" org.apache.pdfbox.ExtractText -html \"/Scratch/ak19/tutorial_sample_files/pdfbox/A9-access-best-practices.pdf\" \"/Scratch/ak19/gs3-svn-15Nov2016/pdf-tmp/1.html\"";
-
-    #$cmd ="/Scratch/ak19/gs3-svn-15Nov2016/packages/jre/bin/java -cp \"/Scratch/ak19/gs3-svn-15Nov2016/gs2build/ext/pdf-box/lib/java/pdfbox-app.jar\" -Dline.separator=\"<br />\" org.apache.pdfbox.ExtractText -html %INPUT_FILE %OUTPUT";
 
     # replace occurrences of placeholders in cmd string
@@ -294,5 +257,5 @@
     if ($self->{'verbosity'} > 2) {
 	print STDERR "$plugin_name: executing conversion cmd \n|$cmd|\n";
-	print STDERR "   on infile |$input_filename|\n";
+	print STDERR "   on infile |$tmp_filename|\n";
 	print STDERR "   to produce expected $output_filename\n";
     }
@@ -341,116 +304,7 @@
     
     return $output_filename;
-}
-
-# Copied from PowerPointPlugin, with some modifications
-# override default read in some situations, as the conversion of ppt to html results in many files, and we want them all to be processed.
-sub read {
-    my $self = shift (@_);  
-    my ($pluginfo, $base_dir, $file, $block_hash, $metadata, $processor, $maxdocs, $total_count, $gli) = @_;
-    
-    # can we process this file??
-    my ($filename_full_path, $filename_no_path) = &util::get_full_filenames($base_dir, $file);
-    
-    return undef unless $self->can_process_this_file($filename_full_path);
-    
-    my $is_output_dir = (defined $self->{'output_dirname'}) ? 1 : 0;
-
-    # we are only doing something special if we have a directory of html files
-    #if ($is_output_dir || $self->{'convert_to'} ne "html") {
-    if ($self->{'convert_to'} ne "html_multi") {
-	return $self->BaseImporter::read(@_); # no read in ConvertBinaryFile.pm
-    }
-    my $outhandle = $self->{'outhandle'};
-    print STDERR "<Processing n='$file' p='$self->{'plugin_type'}'>\n" if ($gli);
-    print $outhandle "$self->{'plugin_type'} processing $file\n"
-	    if $self->{'verbosity'} > 1;
-
-    my $conv_filename = $self->tmp_area_convert_file("html", $filename_full_path); # uses our overridden version
-    if ("$conv_filename" eq "") {return -1;} # had an error, will be passed down pipeline 
-    if (! -e "$conv_filename") {return -1;} 
-
-    my ($tailname, $html_dirname, $suffix)
-	= &File::Basename::fileparse($conv_filename, "\\.[^\\.]+\$");
-
-    my $collect_file = &util::filename_within_collection($filename_full_path);
-    my $dirname_within_collection = &util::filename_within_collection($html_dirname);
-    my $secondary_plugin = $self->{'secondary_plugins'}->{"HTMLPlugin"};
-
-    my @dir;
-    if (!opendir (DIR, $html_dirname)) {
-	print $outhandle "PowerPointPlugin: Couldn't read directory $html_dirname\n";
-	# just process the original file
-	@dir = ("$tailname.$suffix");
-	
-    } else {
-	@dir = readdir (DIR);
-	closedir (DIR);
-    }
-
-    foreach my $file (@dir) {
-	next unless $file =~ /\.html$/;
-	
-	my ($rv, $doc_obj) = 
-	    $secondary_plugin->read_into_doc_obj ($pluginfo,"", &util::filename_cat($html_dirname,$file), $block_hash, {}, $processor, $maxdocs, $total_count, $gli);
-	if ((!defined $rv) || ($rv<1)) {
-	    # wasn't processed
-	    return $rv;
-	}
-
-	# next block copied from ConvertBinaryFile
-	# from here ...
-	# Override previous gsdlsourcefilename set by secondary plugin
-	
-	$doc_obj->set_source_filename ($collect_file, $self->{'file_rename_method'}); 
-	## set_source_filename does not set the doc_obj source_path which is used in archives dbs for incremental
-	# build. so set it manually.
-	$doc_obj->set_source_path($filename_full_path);
-	$doc_obj->set_converted_filename(&util::filename_cat($dirname_within_collection, $file));
-	
-	my $plugin_filename_encoding = $self->{'filename_encoding'};
-    my $filename_encoding = $self->deduce_filename_encoding($file,$metadata,$plugin_filename_encoding);
-	$self->set_Source_metadata($doc_obj, $filename_full_path,$filename_encoding);
-        
-	$doc_obj->set_utf8_metadata_element($doc_obj->get_top_section(), "Plugin", "$self->{'plugin_type'}");
-	$doc_obj->set_utf8_metadata_element($doc_obj->get_top_section(), "FileSize", (-s $filename_full_path));
-
-	
-	my ($tailname, $dirname, $suffix)
-	    = &File::Basename::fileparse($filename_full_path, "\\.[^\\.]+\$");
-	$doc_obj->set_utf8_metadata_element($doc_obj->get_top_section(), "FilenameRoot", $tailname);
-	
-
-	my $topsection = $doc_obj->get_top_section();
-	$self->add_associated_files($doc_obj, $filename_full_path);
-	
-	# extra_metadata is already called by sec plugin in process??
-	$self->extra_metadata($doc_obj, $topsection, $metadata); # do we need this here??
-	# do any automatic metadata extraction
-	$self->auto_extract_metadata ($doc_obj);
-	
-	# have we found a Title??
-	$self->title_fallback($doc_obj,$topsection,$filename_no_path);
-	
-	# use the one generated by HTMLPlugin, otherwise they all end up with same id.
-	#$self->add_OID($doc_obj);
-	# to here...
-
-	# process it
-	$processor->process($doc_obj);
-	undef $doc_obj;
-    }
-    $self->{'num_processed'} ++;
-
-    # deleted some commented out code here that exists in PowerPointPlugin
-
-    # for UnknownConverterPlugin, don't delete any temp files that the conversion may have created?
-    # as we don't know where it was created. No. Now creating in tmp.
-    $self->clean_up_after_doc_obj_processing();
-
-
-    # if process_status == 1, then the file has been processed.
-    return 1;
-
-}
+
+}
+
 
 # use the read_into_doc_obj inherited from ConvertBinaryFile "to call secondary plugin stuff"
