Optionally Have Segment Exporters Derive From AbstractSegmentExporter. #381
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| package org.vitrivr.cineast.core.features.abstracts; | ||
|
|
||
| import java.io.ByteArrayOutputStream; | ||
| import java.io.IOException; | ||
| import java.io.OutputStream; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.util.Base64; | ||
| import java.util.HashMap; | ||
| import java.util.function.Supplier; | ||
| import org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; | ||
| import org.vitrivr.cineast.core.data.segments.SegmentContainer; | ||
| import org.vitrivr.cineast.core.db.PersistencyWriterSupplier; | ||
| import org.vitrivr.cineast.core.db.setup.EntityCreator; | ||
| import org.vitrivr.cineast.core.features.extractor.Extractor; | ||
| import org.vitrivr.cineast.core.util.LogHelper; | ||
|
|
||
| abstract public class AbstractSegmentExporter implements Extractor { | ||
|
|
||
| protected static final Logger LOGGER = LogManager.getLogger(); | ||
|
|
||
| private static final String PROPERTY_NAME_DESTINATION = "destination"; | ||
|
|
||
| /** | ||
| * Destination path for the audio-segment. | ||
| */ | ||
| private final Path destination; | ||
|
There was a problem hiding this comment. Why is this private object property documented and the following protected properties aren't? There was a problem hiding this comment. I assume the path should be a folder? The documentation could be more precise. |
||
|
|
||
| protected String fileExtension; | ||
|
There was a problem hiding this comment. I would refactor this property to a protected abstract getter to force implementing classes to provide sensible values here |
||
|
|
||
| protected String dataUrlPrefix; | ||
|
There was a problem hiding this comment. I would refactor this property to a protected abstract getter to force implementing classes to provide sensible values here |
||
|
|
||
| /** | ||
| * Default constructor | ||
| */ | ||
| public AbstractSegmentExporter() { | ||
| this(new HashMap<>()); | ||
| } | ||
|
|
||
| /** | ||
| * Default constructor. The AudioSegmentExport can be configured via named properties in the provided HashMap. | ||
|
There was a problem hiding this comment. Copy-paster error: This isn't the |
||
| * <p> | ||
| * Supported parameters: | ||
| * | ||
| * <ol> | ||
| * <li>destination: Path where files should be stored.</li> | ||
|
There was a problem hiding this comment. I would highlight in the documentation that its literally There was a problem hiding this comment. Can you please clarify this? There was a problem hiding this comment. You have |
||
| * </ol> | ||
| * | ||
| * @param properties HashMap containing named properties | ||
| */ | ||
| public AbstractSegmentExporter(HashMap<String, String> properties) { | ||
| this.destination = Path.of(properties.getOrDefault(PROPERTY_NAME_DESTINATION, ".")); | ||
|
There was a problem hiding this comment. I am not convinced that There was a problem hiding this comment. I agree with this and am fixing this. Just for the record, originally AudioSegmentExporter did use '.' |
||
| } | ||
|
|
||
| abstract public void exportToStream(SegmentContainer sc, OutputStream stream); | ||
sauterl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| abstract public boolean isExportable(SegmentContainer sc); | ||
sauterl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| @Override | ||
| public void processSegment(SegmentContainer shot) { | ||
| if (!isExportable(shot)) { | ||
| return; | ||
| } | ||
| try { | ||
| /* Prepare folder and OutputStream. */ | ||
| Path folder = this.destination.resolve(shot.getSuperId()); | ||
| if (!folder.toFile().exists()) { | ||
| folder.toFile().mkdirs(); | ||
| } | ||
| Path path = folder.resolve(shot.getId() + this.fileExtension); | ||
|
There was a problem hiding this comment. No guarantee that |
||
| OutputStream os = Files.newOutputStream(path); | ||
|
|
||
| /* Write audio data to OutputStream. */ | ||
|
There was a problem hiding this comment. This is data-type agnostic at this point, no? |
||
| exportToStream(shot, os); | ||
|
|
||
| /* Close OutputStream. */ | ||
| os.close(); | ||
| } catch (Exception e) { | ||
| LOGGER.error("Could not data to file for segment {} due to {}.", shot.getId(), LogHelper.getStackTrace(e)); | ||
|
There was a problem hiding this comment. Suggested error message: |
||
| } | ||
| } | ||
|
|
||
| public String exportToDataUrl(SegmentContainer shot) { | ||
|
There was a problem hiding this comment. Documentation missing. Please add documentation |
||
| // create a ByteArrayOutputStream | ||
|
There was a problem hiding this comment. I'm fairly sure that these one-line comments are not necessary and do not help understand the code, please remove them |
||
| ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
|
|
||
| // call the exportToStream method with the ByteArrayOutputStream | ||
| exportToStream(shot, baos); | ||
sauterl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // convert the ByteArrayOutputStream's data to a byte array | ||
|
There was a problem hiding this comment. I'm fairly sure that these one-line comments are not necessary and do not help understand the code, please remove them |
||
| byte[] bytes = baos.toByteArray(); | ||
|
|
||
| // encode the byte array to a Base64 string | ||
| String base64 = Base64.getEncoder().encodeToString(bytes); | ||
|
|
||
| // concatenate the data URL prefix and the Base64 string | ||
| return this.dataUrlPrefix + base64; | ||
|
There was a problem hiding this comment. There is no guarantee at this point that |
||
| } | ||
|
|
||
| public byte[] exportToBinary(SegmentContainer shot) { | ||
sauterl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // create a ByteArrayOutputStream | ||
|
There was a problem hiding this comment. I'm fairly sure that these one-line comments are not necessary and do not help understand the code, please remove them |
||
| ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
|
|
||
| // call the exportToStream method with the ByteArrayOutputStream | ||
| exportToStream(shot, baos); | ||
|
|
||
| // convert the ByteArrayOutputStream's data to a byte array | ||
| return baos.toByteArray(); | ||
| } | ||
|
|
||
| @Override | ||
| public void init(PersistencyWriterSupplier phandlerSupply) { /* Nothing to init. */ } | ||
|
|
||
| @Override | ||
| public void finish() { /* Nothing to finish. */} | ||
|
There was a problem hiding this comment. What about closing the stream? There was a problem hiding this comment. To put emphasis on this comment. I'd expect the |
||
|
|
||
| @Override | ||
| public void initalizePersistentLayer(Supplier<EntityCreator> supply) { /* Nothing to init. */ } | ||
|
There was a problem hiding this comment. Doesn't in your case mean persistent layer database + outgoing stream? In this case I'd expect this to be |
||
|
|
||
| @Override | ||
| public void dropPersistentLayer(Supplier<EntityCreator> supply) { /* Nothing to drop. */} | ||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer the property name to be public.