Skip to content

Adding support for animated gifs #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Adding support for animated gifs #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions src/main/scala/com/rdio/thor/AnimatedGifWriter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package com.rdio.thor

import com.sksamuel.scrimage.Image
import com.sksamuel.scrimage.io.ImageWriter
import com.twitter.logging.Logger
import java.io.OutputStream
import java.awt.image.BufferedImage
import javax.imageio.{ IIOImage, ImageTypeSpecifier, ImageWriteParam, ImageIO }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove spaces inside the {}.

import javax.imageio.metadata.IIOMetadataNode
import org.apache.commons.io.IOUtils
import javax.imageio.stream.MemoryCacheImageOutputStream

case class Frame(image: Image, durationInMs: Int)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\n after case class.

class AnimatedGifWriter(frames: List[Frame]) extends ImageWriter {


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra \n

protected lazy val log = Logger.get(this.getClass)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should remove all logging from utility classes.

EDIT: To clarify, only application-level code should log things.


def write(out: OutputStream) {
val writer = ImageIO.getImageWritersByFormatName("gif").next()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, that is a handy function.

val params = writer.getDefaultWriteParam
// TODO : OH SHIT!! What is our BufferedImage type?!?!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what this means but it sounds bad. Should be fixed before merging.


val metaData = writer.getDefaultImageMetadata(ImageTypeSpecifier.createFromBufferedImageType(BufferedImage.TYPE_INT_ARGB), params)
val root:IIOMetadataNode = new IIOMetadataNode(metaData.getNativeMetadataFormatName())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the types need to be like root: IIOMetadataNode


val graphicsControlExtensionNode:IIOMetadataNode = new IIOMetadataNode("GraphicControlExtension")
graphicsControlExtensionNode.setAttribute("disposalMethod", "none")
graphicsControlExtensionNode.setAttribute("userInputFlag", "FALSE")
graphicsControlExtensionNode.setAttribute("transparentColorFlag", "FALSE")
graphicsControlExtensionNode.setAttribute("delayTime", (10).toString) // stupid! "Delay Time (1/100ths of a second)"
graphicsControlExtensionNode.setAttribute("transparentColorIndex", "0") // somewhere else has it as 255
root.appendChild(graphicsControlExtensionNode)

val appEntensionsNode:IIOMetadataNode = new IIOMetadataNode("ApplicationExtensions")
val child:IIOMetadataNode = new IIOMetadataNode("ApplicationExtension")
child.setAttribute("applicationID", "NETSCAPE")
child.setAttribute("authenticationCode", "2.0")
val byteArray:Array[Byte] = Array(0x01, 0x00, 0x00) // The last two bytes is the unsigned short (little endian) that represents the number of times to loop. 0 means loop forever.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its just my OCD but the different comment styles makes me sad:

Single space on either side of //:

graphicsControlExtensionNode.setAttribute("delayTime", (10).toString) // stupid! "Delay Time (1/100ths of a second)"

Double-space before //, single-space after:

graphicsControlExtensionNode.setAttribute("transparentColorIndex", "0")  // somewhere else has it as 255

Double-space on both sides:

val byteArray:Array[Byte] = Array(0x01, 0x00, 0x00)  //  The last two bytes...

child.setUserObject(byteArray)
appEntensionsNode.appendChild(child)
root.appendChild(appEntensionsNode)

metaData.mergeTree(metaData.getNativeMetadataFormatName(), root)

val output = new MemoryCacheImageOutputStream(out)
writer.setOutput(output)
writer.prepareWriteSequence(null)
frames.zipWithIndex foreach {
case (frame:Frame, i) => {
val count = frame.durationInMs
graphicsControlExtensionNode.setAttribute("delayTime", (count / 10).toString) // stupid! "Delay Time (1/100ths of a second)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make a timeStep constant? msDuration / timeStep

val TimeStep = 10 // 1/100th of a second

root.appendChild(graphicsControlExtensionNode)
metaData.mergeTree(metaData.getNativeMetadataFormatName(), root)
writer.writeToSequence(new IIOImage(frame.image.awt, null, metaData), params)
}
}
writer.endWriteSequence()
writer.dispose()
output.close()
IOUtils.closeQuietly(out)
}
}

object AnimatedGifWriter {
def apply(frames: List[Frame]): AnimatedGifWriter = new AnimatedGifWriter(frames)
}
8 changes: 8 additions & 0 deletions src/main/scala/com/rdio/thor/BaseImageService.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,21 @@ abstract class BaseImageService(conf: Config) extends Service[Request, Response]
.name("thor-client")
.build()

def buildAnimatedResponse[T <: ImageWriter](req: Request, frames: List[Frame], format: Format[T], compression: Int = 98): Response = {
val bytes = AnimatedGifWriter(frames).write()
furtherBuildOutResponse(req, bytes, format, compression)
}

def buildResponse[T <: ImageWriter](req: Request, image: Image, format: Format[T], compression: Int = 98): Response = {
val bytes = format match {
case Format.JPEG => image.writer(format).withCompression(compression).withProgressive(true).write()
case Format.PNG => image.writer(format).withMaxCompression.write()
case Format.GIF => image.writer(format).withProgressive(true).write()
}
furtherBuildOutResponse(req, bytes, format, compression)
}

def furtherBuildOutResponse[T <: ImageWriter](req: Request, bytes: Array[Byte], format: Format[T], compression: Int = 98): Response = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not a fan of this name. Can it be buildResponse which accepts a byte array and have a separate getBytes for converting an image to bytes based on its format?

val expires: Calendar = Calendar.getInstance()
expires.add(Calendar.YEAR, 1)

Expand Down
35 changes: 24 additions & 11 deletions src/main/scala/com/rdio/thor/ImageService.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.typesafe.config.Config
import org.jboss.netty.handler.codec.http._
import org.jboss.netty.buffer.ChannelBuffers


/** ImageService serves images optionally filtered and blended. */
class ImageService(conf: Config) extends BaseImageService(conf) {

Expand Down Expand Up @@ -179,19 +180,24 @@ class ImageService(conf: Config) extends BaseImageService(conf) {
case _: NoopNode => Some(image)
}
}

def applyLayerFilters(imageMap: Map[String, Image], layers: List[LayerNode], width: Int, height: Int): Option[Image] = {
def applyLayerFilters(imageMap: Map[String, Image], layers: List[LayerNode], width: Int, height: Int): (Option[Image], List[Frame]) = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break RdioThor. I don't think we should modify a generic function we use for other things to explicitly include frames. Can we separate the two somehow without much duplication?

// Apply each layer in order
val completedLayers = ArrayBuffer.empty[Image]
var completedFrames:List[Frame] = List()
layers foreach {
case LayerNode(path: ImageNode, filter: FilterNode) => {
tryGetImage(path, imageMap, completedLayers.toArray, width, height) match {
case Some(baseImage) => {
applyFilter(baseImage, filter, imageMap, completedLayers.toArray, width, height) match {
case Some(filteredImage) => completedLayers += filteredImage
case None => {
log.error(s"Failed to apply layer filter: $path $filter")
None
filter match {
case frame:FrameNode => completedFrames = completedFrames :+ Frame(baseImage, frame.durationInMs)
case _ => {
applyFilter(baseImage, filter, imageMap, completedLayers.toArray, width, height) match {
case Some(filteredImage) => completedLayers += filteredImage
case None => {
log.error(s"Failed to apply layer filter: $path $filter")
None
}
}
}
}
}
Expand All @@ -202,7 +208,8 @@ class ImageService(conf: Config) extends BaseImageService(conf) {
}
}
}
completedLayers.lastOption
log.info(s" Yo rebecca i see ${completedFrames.length}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove :)

(completedLayers.lastOption, completedFrames)
}

def scaleTo(image: Image, width: Int, height: Int): Image = {
Expand All @@ -226,6 +233,7 @@ class ImageService(conf: Config) extends BaseImageService(conf) {
val compression: Int = math.min(math.max(req.params.getIntOrElse("c", 98), 0), 100)

val format: Format[ImageWriter] = req.params.get("f") match {
case Some("gif") => Format.GIF.asInstanceOf[Format[ImageWriter]]
case Some("png") => Format.PNG.asInstanceOf[Format[ImageWriter]]
case _ => Format.JPEG.asInstanceOf[Format[ImageWriter]]
}
Expand Down Expand Up @@ -257,11 +265,16 @@ class ImageService(conf: Config) extends BaseImageService(conf) {

// Apply any filters to each image and return the final image
applyLayerFilters(imageMap, layers, width, height) match {
case Some(image) => {
case (_, frames) if frames.length > 1 => {
// Apply final resize and build response
buildResponse(req, scaleTo(image, width, height), format, compression)
val resizedFrames = frames.map( f => Frame(scaleTo(f.image, width, height), f.durationInMs))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

frames.map(f => (remove internal space)

buildAnimatedResponse(req, resizedFrames, format, compression)
}
case None => {
case (Some(image), _) => {
// Apply final resize and build response
buildResponse(req, image, format, compression)
}
case _ => {
Response(HttpVersion.HTTP_1_1, HttpResponseStatus.NOT_FOUND)
}
}
Expand Down
10 changes: 8 additions & 2 deletions src/main/scala/com/rdio/thor/LayerParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ case class RoundCornersPercentNode(radius: Float) extends FilterNode
case class OverlayNode(overlay: ImageNode) extends FilterNode
case class MaskNode(overlay: ImageNode, mask: ImageNode) extends FilterNode
case class CoverNode(width: Int, height: Int) extends FilterNode
case class FrameNode(durationInMs: Int) extends FilterNode

case class LayerNode(path: ImageNode, filter: FilterNode)

Expand Down Expand Up @@ -225,13 +226,18 @@ class LayerParser(width: Int, height: Int) extends JavaTokenParsers {
case width ~ _ ~ height => CoverNode(width, height)
}

// frame filter
def frame: Parser[FrameNode] = "frame(" ~> integer <~ ")" ^^ {
case durationInMs => FrameNode(durationInMs)
}

// all filters
def filters: Parser[FilterNode] =
text | linear | boxblur | boxblurpercent |
blur | scaleto | zoom | scale |
grid | round | roundpercent | mask |
colorize | overlay | pad | padpercent

colorize | overlay | pad | padpercent | cover | frame
// layer - matches a single layer
def layer: Parser[LayerNode] =
// Match a path without filters
Expand Down