Thumbnail uploaded and cached images

7 files changed, 587 insertions, 175 deletions

diff --git a/synapse/media/v1/base_resource.py b/synapse/media/v1/base_resource.py
new file mode 100644
index 0000000000..1e57a1465f
--- /dev/null
+++ b/synapse/media/v1/base_resource.py
@@ -0,0 +1,318 @@
+# -*- coding: utf-8 -*-
+# Copyright 2014 OpenMarket Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from .thumbnailer import Thumbnailer
+
+from synapse.http.server import respond_with_json
+from synapse.util.stringutils import random_string
+from synapse.api.errors import (
+    cs_exception, CodeMessageException, cs_error, Codes, SynapseError
+)
+
+from twisted.internet import defer
+from twisted.web.resource import Resource
+from twisted.protocols.basic import FileSender
+
+import os
+
+import logging
+
+logger = logging.getLogger(__name__)
+
+
+class BaseMediaResource(Resource):
+    isLeaf = True
+
+    def __init__(self, hs, filepaths):
+        Resource.__init__(self)
+        self.client = hs.get_http_client()
+        self.clock = hs.get_clock()
+        self.server_name = hs.hostname
+        self.store = hs.get_datastore()
+        self.max_upload_size = hs.config.max_upload_size
+        self.filepaths = filepaths
+
+    @staticmethod
+    def catch_errors(request_handler):
+        @defer.inlineCallbacks
+        def wrapped_request_handler(self, request):
+            try:
+                yield request_handler(self, request)
+            except CodeMessageException as e:
+                logger.exception(e)
+                respond_with_json(
+                    request, e.code, cs_exception(e), send_cors=True
+                )
+            except:
+                logger.exception(
+                    "Failed handle request %s.%s on %r",
+                    request_handler.__module__,
+                    request_handler.__name__,
+                    self,
+                )
+                respond_with_json(
+                    request,
+                    500,
+                    {"error": "Internal server error"},
+                    send_cors=True
+                )
+        return wrapped_request_handler
+
+    @staticmethod
+    def _parse_media_id(request):
+        try:
+            server_name, media_id = request.postpath
+            return (server_name, media_id)
+        except:
+            raise SynapseError(
+                404,
+                "Invalid media id token %r" % (request.postpath,),
+                Codes.UNKKOWN,
+            )
+
+    @staticmethod
+    def _parse_integer(request, arg_name, default=None):
+        try:
+            if default is None:
+                return int(request.args[arg_name][0])
+            else:
+                return int(request.args.get(arg_name, [default])[0])
+        except:
+            raise SynapseError(
+                400,
+                "Missing integer argument %r" % (arg_name),
+                Codes.UNKNOWN,
+            )
+
+    @staticmethod
+    def _parse_string(request, arg_name, default=None):
+        try:
+            if default is None:
+                return request.args[arg_name][0]
+            else:
+                return request.args.get(arg_name, [default])[0]
+        except:
+            raise SynapseError(
+                400,
+                "Missing string argument %r" % (arg_name),
+                Codes.UNKNOWN,
+            )
+
+    def _respond_404(self, request):
+        respond_with_json(
+            request, 404,
+            cs_error(
+                "Not found %r" % (request.postpath,),
+                code=Codes.NOT_FOUND,
+            ),
+            send_cors=True
+        )
+
+    @defer.inlineCallbacks
+    def _download_remote_file(self, server_name, media_id):
+        file_id = random_string(24)
+
+        fname = self.filepaths.remote_media_filepath(
+            server_name, file_id
+        )
+        os.makedirs(os.path.dirname(fname))
+
+        try:
+            with open(fname, "wb") as f:
+                request_path = "/".join((
+                    "/_matrix/media/v1/download", server_name, media_id,
+                )),
+                length, headers = yield self.client.get_file(
+                    server_name, request_path, output_stream=f,
+                )
+            media_type = headers["Content-Type"][0]
+            time_now_ms = self.clock.time_msec()
+
+            yield self.store.store_cached_remote_media(
+                origin=server_name,
+                media_id=media_id,
+                media_type=media_type,
+                time_now_ms=self.clock.time_msec(),
+                upload_name=None,
+                media_length=length,
+                file_id=file_id,
+            )
+        except:
+            os.remove(fname)
+            raise
+
+        media_info = {
+            "media_type": media_type,
+            "media_length": length,
+            "upload_name": None,
+            "created_ts": time_now_ms,
+            "file_id": file_id,
+        }
+
+        yield self._generate_remote_thumbnails(
+            server_name, media_id, media_info
+        )
+
+        defer.returnValue(media_info)
+
+    @defer.inlineCallbacks
+    def _respond_with_file(self, request, media_type, file_path):
+        logger.debug("Responding with %r", file_path)
+
+        if os.path.isfile(file_path):
+            request.setHeader(b"Content-Type", media_type.encode("UTF-8"))
+
+            # cache for at least a day.
+            # XXX: we might want to turn this off for data we don't want to
+            # recommend caching as it's sensitive or private - or at least
+            # select private. don't bother setting Expires as all our
+            # clients are smart enough to be happy with Cache-Control
+            request.setHeader(
+                b"Cache-Control", b"public,max-age=86400,s-maxage=86400"
+            )
+
+            with open(file_path, "rb") as f:
+                yield FileSender().beginFileTransfer(f, request)
+
+            request.finish()
+        else:
+            self._respond_404()
+
+    def _get_thumbnail_requirements(self, media_type):
+        if media_type == "image/jpeg":
+            return (
+                (32, 32, "crop", "image/jpeg"),
+                (96, 96, "crop", "image/jpeg"),
+                (320, 240, "scale", "image/jpeg"),
+                (640, 480, "scale", "image/jpeg"),
+            )
+        elif (media_type == "image/png") or (media_type == "image/gif"):
+            return (
+                (32, 32, "crop", "image/png"),
+                (96, 96, "crop", "image/png"),
+                (320, 240, "scale", "image/png"),
+                (640, 480, "scale", "image/png"),
+            )
+        else:
+            return ()
+
+    @defer.inlineCallbacks
+    def _generate_local_thumbnails(self, media_id, media_info):
+        media_type = media_info["media_type"]
+        requirements = self._get_thumbnail_requirements(media_type)
+        if not requirements:
+            return
+
+        input_path = self.filepaths.local_media_path(media_id)
+        thumbnailer = Thumbnailer(input_path)
+        m_width = thumbnailer.width
+        m_height = thumbnailer.height
+        scales = set()
+        crops = set()
+        for r_width, r_height, r_method, r_type in requirements:
+            if r_method == "scale":
+                t_width, t_height = thumbnailer.aspect(r_width, r_height)
+                scales.add((
+                    min(m_width, t_width), min(m_height, t_height), r_type,
+                ))
+            elif r_method == "crop":
+                crops.add((r_width, r_height, r_type))
+
+        for t_width, t_height, t_type in scales:
+            t_method = "scale"
+            t_path = self.filepaths.local_media_thumbnail(
+                media_id, t_width, t_height, t_type, t_method
+            )
+            t_len = thumbnailer.scale(t_path, t_width, t_height, t_type)
+            yield self.store.store_local_thumbnail(
+                media_id, t_width, t_height, t_type, t_method, t_len
+            )
+
+        for t_width, t_height, t_type in crops:
+            if (t_width, t_height, t_type) in scales:
+                # If the aspect ratio of the cropped thumbnail matches a purely
+                # scaled one then there is no point in calculating a separate
+                # thumbnail.
+                continue
+            t_method = "crop"
+            t_path = self.filepaths.local_media_thumbnail(
+                media_id, t_width, t_height, t_type, t_method
+            )
+            t_len = thumbnailer.crop(t_path, t_width, t_height, t_type)
+            yield self.store.store_local_thumbnail(
+                media_id, t_width, t_height, t_type, t_method, t_len
+            )
+
+        defer.returnValue({
+            "width": m_width,
+            "height": m_height,
+        })
+
+    @defer.inlineCallbacks
+    def _generate_remote_thumbnails(self, server_name, media_id, media_info):
+        media_type = media_info["media_type"]
+        file_id = media_info["filesystem_id"]
+        requirements = self._get_requirements(media_type)
+        if not requirements:
+            return
+
+        input_path = self.filepaths.remote_media_path(server_name, file_id)
+        thumbnailer = Thumbnailer(input_path)
+        m_width = thumbnailer.width
+        m_height = thumbnailer.height
+        scales = set()
+        crops = set()
+        for r_width, r_height, r_method, r_type in requirements:
+            if r_method == "scale":
+                t_width, t_height = thumbnailer.aspect(r_width, r_height)
+                scales.add((
+                    min(m_width, t_width), min(m_height, t_height), r_type,
+                ))
+            elif r_method == "crop":
+                crops.add((r_width, r_height, r_type))
+
+        for t_width, t_height, t_type in scales:
+            t_method = "scale"
+            t_path = self.filepaths.remote_media_thumbnail(
+                server_name, media_id, file_id,
+                media_id, t_width, t_height, t_type, t_method
+            )
+            t_len = thumbnailer.scale(t_path, t_width, t_height, t_type)
+            yield self.store.store_remote_media_thumbnail(
+                server_name, media_id, file_id,
+                t_width, t_height, t_type, t_method, t_len
+            )
+
+        for t_width, t_height, t_type in crops:
+            if (t_width, t_height, t_type) in scales:
+                # If the aspect ratio of the cropped thumbnail matches a purely
+                # scaled one then there is no point in calculating a separate
+                # thumbnail.
+                continue
+            t_method = "crop"
+            t_path = self.filepaths.remote_media_thumbnail(
+                server_name, media_id, file_id,
+                t_width, t_height, t_type, t_method
+            )
+            t_len = thumbnailer.crop(t_path, t_width, t_height, t_type)
+            yield self.store.store_remote_media_thumbnail(
+                server_name, media_id, file_id,
+                t_width, t_height, t_type, t_method, t_len
+            )
+
+        defer.returnValue({
+            "width": m_width,
+            "height": m_height,
+        })
diff --git a/synapse/media/v1/download_resource.py b/synapse/media/v1/download_resource.py
index c243f16a74..31c6f25968 100644
--- a/synapse/media/v1/download_resource.py
+++ b/synapse/media/v1/download_resource.py
@@ -13,117 +13,46 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from synapse.http.server import respond_with_json
-from synapse.util.stringutils import random_string
-from synapse.api.errors import (
-    cs_exception, CodeMessageException, cs_error, Codes
-)
-
-from twisted.protocols.basic import FileSender
-from twisted.web.resource import Resource
+from .base_media_resource import BaseMediaResource
+
 from twisted.web.server import NOT_DONE_YET
 from twisted.internet import defer
 
-import os
-
 import logging
 
 logger = logging.getLogger(__name__)
 
 
-class DownloadResource(Resource):
-    isLeaf = True
-
-    def __init__(self, hs, filepaths):
-        Resource.__init__(self)
-        self.client = hs.get_http_client()
-        self.clock = hs.get_clock()
-        self.server_name = hs.hostname
-        self.store = hs.get_datastore()
-        self.filepaths = filepaths
-
+class DownloadResource(BaseMediaResource):
     def render_GET(self, request):
         self._async_render_GET(request)
         return NOT_DONE_YET
 
-    def _respond_404(self, request):
-        respond_with_json(
-            request, 404,
-            cs_error(
-                "Not found %r" % (request.postpath,),
-                code=Codes.NOT_FOUND,
-            ),
-            send_cors=True
-        )
-
+    @BaseMediaResource.catch_errors
     @defer.inlineCallbacks
     def _async_render_GET(self, request):
-
         try:
             server_name, media_id = request.postpath
         except:
             self._respond_404(request)
             return
 
-        try:
-            if server_name == self.server_name:
-                yield self._respond_local_file(request, media_id)
-            else:
-                yield self._respond_remote_file(request, server_name, media_id)
-        except CodeMessageException as e:
-            logger.exception(e)
-            respond_with_json(request, e.code, cs_exception(e), send_cors=True)
-        except:
-            logger.exception("Failed to serve file")
-            respond_with_json(
-                request,
-                500,
-                {"error": "Internal server error"},
-                send_cors=True
-            )
+        if server_name == self.server_name:
+            yield self._respond_local_file(request, media_id)
+        else:
+            yield self._respond_remote_file(request, server_name, media_id)
 
     @defer.inlineCallbacks
-    def _download_remote_file(self, server_name, media_id):
-        filesystem_id = random_string(24)
-
-        fname = self.filepaths.remote_media_filepath(
-            server_name, filesystem_id
-        )
-        os.makedirs(os.path.dirname(fname))
+    def _respond_local_file(self, request, media_id):
+        media_info = yield self.store.get_local_media(media_id)
+        if not media_info:
+            self._respond_404()
+            return
 
-        try:
-            with open(fname, "wb") as f:
-                length, headers = yield self.client.get_file(
-                    server_name,
-                    "/".join((
-                        "/_matrix/media/v1/download", server_name, media_id,
-                    )),
-                    output_stream=f,
-                )
-        except:
-            os.remove(fname)
-            raise
-
-        media_type = headers["Content-Type"][0]
-        time_now_ms = self.clock.time_msec()
-
-        yield self.store.store_cached_remote_media(
-            origin=server_name,
-            media_id=media_id,
-            media_type=media_type,
-            time_now_ms=self.clock.time_msec(),
-            upload_name=None,
-            media_length=length,
-            filesystem_id=filesystem_id,
-        )
+        media_type = media_info["media_type"]
+        file_path = self.filepaths.local_media_filepath(media_id)
 
-        defer.returnValue({
-            "media_type": media_type,
-            "media_length": length,
-            "upload_name": None,
-            "created_ts": time_now_ms,
-            "filesystem_id": filesystem_id,
-        })
+        yield self.respond_with_file(request, media_type, file_path)
 
     @defer.inlineCallbacks
     def _respond_remote_file(self, request, server_name, media_id):
@@ -136,59 +65,11 @@ class DownloadResource(Resource):
                 server_name, media_id
             )
 
+        media_type = media_info["media_type"]
         filesystem_id = media_info["filesystem_id"]
 
         file_path = self.filepaths.remote_media_filepath(
             server_name, filesystem_id
         )
 
-        if os.path.isfile(file_path):
-            media_type = media_info["media_type"]
-            request.setHeader(b"Content-Type", media_type.encode("UTF-8"))
-
-            # cache for at least a day.
-            # XXX: we might want to turn this off for data we don't want to
-            # recommend caching as it's sensitive or private - or at least
-            # select private. don't bother setting Expires as all our
-            # clients are smart enough to be happy with Cache-Control
-            request.setHeader(
-                b"Cache-Control", b"public,max-age=86400,s-maxage=86400"
-            )
-
-            with open(file_path, "rb") as f:
-                yield FileSender().beginFileTransfer(f, request)
-
-            request.finish()
-        else:
-            self._respond_404()
-
-    @defer.inlineCallbacks
-    def _respond_local_file(self, request, media_id):
-        media_info = yield self.store.get_local_media(media_id)
-        if not media_info:
-            self._respond_404()
-            return
-
-        file_path = self.filepaths.local_media_filepath(media_id)
-
-        logger.debug("Searching for %s", file_path)
-
-        if os.path.isfile(file_path):
-            media_type = media_info["media_type"]
-            request.setHeader(b"Content-Type", media_type.encode("UTF-8"))
-
-            # cache for at least a day.
-            # XXX: we might want to turn this off for data we don't want to
-            # recommend caching as it's sensitive or private - or at least
-            # select private. don't bother setting Expires as all our
-            # clients are smart enough to be happy with Cache-Control
-            request.setHeader(
-                b"Cache-Control", b"public,max-age=86400,s-maxage=86400"
-            )
-
-            with open(file_path, "rb") as f:
-                yield FileSender().beginFileTransfer(f, request)
-
-            request.finish()
-        else:
-            self._respond_404()
+        yield self.respond_with_file(request, media_type, file_path)
diff --git a/synapse/media/v1/filepath.py b/synapse/media/v1/filepath.py
index d23564e03e..0078bc3d40 100644
--- a/synapse/media/v1/filepath.py
+++ b/synapse/media/v1/filepath.py
@@ -21,33 +21,47 @@ class MediaFilePaths(object):
     def __init__(self, base_path):
         self.base_path = base_path
 
+    def default_thumbnail(self, default_top_level, default_sub_type, width,
+                          height, content_type, method):
+        top_level_type, sub_type = content_type.split("/")
+        file_name = "%i-%i-%s-%s-%s" % (
+            width, height, top_level_type, sub_type, method
+        )
+        return os.path.join(
+            self.base_path, "default_thumbnails", default_top_level,
+            default_sub_type, file_name
+        )
+
     def local_media_filepath(self, media_id):
         return os.path.join(
-            self.base_path, "local", "content",
+            self.base_path, "local_content",
             media_id[0:2], media_id[2:4], media_id[4:]
         )
 
-    def local_media_thumbnail(self, media_id, width, height, content_type):
+    def local_media_thumbnail(self, media_id, width, height, content_type,
+                              method):
         top_level_type, sub_type = content_type.split("/")
-        file_name = "%i-%i-%s-%s" % (width, height, top_level_type, sub_type)
+        file_name = "%i-%i-%s-%s-%s" % (
+            width, height, top_level_type, sub_type, method
+        )
         return os.path.join(
-            self.base_path, "local", "thumbnails",
+            self.base_path, "local_thumbnails",
             media_id[0:2], media_id[2:4], media_id[4:],
             file_name
         )
 
     def remote_media_filepath(self, server_name, file_id):
         return os.path.join(
-            self.base_path, "remote", "content", server_name,
+            self.base_path, "remote_content", server_name,
             file_id[0:2], file_id[2:4], file_id[4:]
         )
 
     def remote_media_thumbnail(self, server_name, file_id, width, height,
-                               content_type):
+                               content_type, method):
         top_level_type, sub_type = content_type.split("/")
         file_name = "%i-%i-%s-%s" % (width, height, top_level_type, sub_type)
         return os.path.join(
-            self.base_path, "remote", "content", server_name,
+            self.base_path, "remote_thumbnail", server_name,
             file_id[0:2], file_id[2:4], file_id[4:],
             file_name
         )
diff --git a/synapse/media/v1/thumbnail_resource.py b/synapse/media/v1/thumbnail_resource.py
new file mode 100644
index 0000000000..331ba87e00
--- /dev/null
+++ b/synapse/media/v1/thumbnail_resource.py
@@ -0,0 +1,191 @@
+# -*- coding: utf-8 -*-
+# Copyright 2014 OpenMarket Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+from .base_media_resource import BaseMediaResource
+
+from twisted.web.server import NOT_DONE_YET
+from twisted.internet import defer
+
+import logging
+
+logger = logging.getLogger(__name__)
+
+
+class ThumbnailResource(BaseMediaResource):
+    isLeaf = True
+
+    def render_GET(self, request):
+        self._async_render_GET(request)
+        return NOT_DONE_YET
+
+    @BaseMediaResource.catch_errors
+    @defer.inlineCallbacks
+    def _async_render_GET(self, request):
+        server_name, media_id = self._parse_media_id(request)
+        width = self._parse_integer(request, "width")
+        height = self._parse_integer(request, "height")
+        method = self._parse_string(request, "method", "scale")
+        m_type = self._parse_string(request, "type", "image/png")
+
+        if server_name == self.server_name:
+            yield self._respond_local_thumbnail(
+                request, media_id, width, height, method, m_type
+            )
+        else:
+            yield self._respond_remote_thumbnail(
+                request, server_name, media_id,
+                width, height, method, m_type
+            )
+
+    @defer.inlineCallbacks
+    def _respond_local_thumbnail(self, request, media_id, width, height,
+                                 method, m_type):
+        media_info = yield self.store.get_local_media(media_id)
+
+        if not media_info:
+            self._respond_404(request)
+            return
+
+        thumbnail_infos = yield self.store.get_local_thumbnail(media_id)
+
+        if thumbnail_infos:
+            thumbnail_info = self._select_thumbnail(
+                width, height, method, m_type, thumbnail_infos
+            )
+            thumbnail_width = thumbnail_info["thumbnail_width"]
+            thumbnail_height = thumbnail_info["thumbnail_height"]
+            thumbnail_type = thumbnail_info["thumbnail_type"]
+            thumbnail_method = thumbnail_info["thumbnail_method"]
+
+            file_path = self.filepaths.local_media_thumbnail(
+                media_id, thumbnail_width, thumbnail_height, thumbnail_type,
+                thumbnail_method,
+            )
+            yield self._respond_with_file(request, thumbnail_type, file_path)
+
+        else:
+            yield self._respond_default_thumbnail(
+                self, request, media_info, width, height, method, m_type,
+            )
+
+    @defer.inlineCallbacks
+    def _respond_remote_thumbnail(self, request, server_name, media_id, width,
+                                  height, method, m_type):
+        media_info = yield self.store.get_cached_remote_media(
+            server_name, media_id
+        )
+
+        if not media_info:
+            # TODO: Don't download the whole remote file
+            # We should proxy the thumbnail from the remote server instead.
+            media_info = yield self._download_remote_file(
+                server_name, media_id
+            )
+
+        thumbnail_infos = yield self.store.get_remote_media_thumbnails(
+            server_name, media_id,
+        )
+
+        if thumbnail_infos:
+            thumbnail_info = self._select_thumbnail(
+                width, height, method, m_type, thumbnail_infos
+            )
+            thumbnail_width = thumbnail_info["thumbnail_width"]
+            thumbnail_height = thumbnail_info["thumbnail_height"]
+            thumbnail_type = thumbnail_info["thumbnail_type"]
+            thumbnail_method = thumbnail_info["thumbnail_method"]
+
+            file_path = self.filepaths.remote_media_thumbnail(
+                server_name, media_id, thumbnail_width, thumbnail_height,
+                thumbnail_type, thumbnail_method,
+            )
+            yield self._respond_with_file(request, thumbnail_type, file_path)
+        else:
+            yield self._respond_default_thumbnail(
+                self, request, media_info, width, height, method, m_type,
+            )
+
+    @defer.inlineCallbacks
+    def _respond_default_thumbnail(self, request, media_info, width, height,
+                                   method, m_type):
+        media_type = media_info["media_type"]
+        top_level_type = media_type.split("/")[0]
+        sub_type = media_type.split("/")[-1].split(";")[0]
+        thumbnail_infos = yield self.store.get_default_thumbnails(
+            top_level_type, sub_type,
+        )
+        if not thumbnail_infos:
+            thumbnail_infos = yield self.store.get_default_thumbnails(
+                top_level_type, "_default",
+            )
+        if not thumbnail_infos:
+            thumbnail_infos = yield self.store.get_default_thumbnails(
+                "_default", "_default",
+            )
+        if not thumbnail_infos:
+            self._respond_404(request)
+            return
+
+        thumbnail_info = self._select_thumbnail(
+            width, height, "crop", m_type, thumbnail_infos
+        )
+
+        thumbnail_width = thumbnail_info["thumbnail_width"]
+        thumbnail_height = thumbnail_info["thumbnail_height"]
+        thumbnail_type = thumbnail_info["thumbnail_type"]
+        thumbnail_method = thumbnail_info["thumbnail_method"]
+
+        file_path = self.filepaths.default_thumbnail(
+            top_level_type, sub_type, thumbnail_width, thumbnail_height,
+            thumbnail_type, thumbnail_method,
+        )
+        yield self.respond_with_file(request, thumbnail_type, file_path)
+
+    def _select_thumbnail(self, desired_width, desired_height, desired_method,
+                          desired_type, thumbnail_infos):
+        d_w = desired_width
+        d_h = desired_height
+
+        if desired_method.lower() == "crop":
+            info_list = []
+            for info in thumbnail_infos:
+                t_w = info["thumbnail_width"]
+                t_h = info["thumbnail_height"]
+                t_method = info["thumnail_method"]
+                if t_method == "scale" or t_method == "crop":
+                    aspect_quality = abs(d_w * t_h - d_h * t_w)
+                    size_quality = abs((d_w - t_w) * (d_h - t_h))
+                    type_quality = desired_type != info["thumbnail_type"]
+                    length_quality = info["thumbnail_length"]
+                    info_list.append((
+                        aspect_quality, size_quality, type_quality,
+                        length_quality, info
+                    ))
+            return min(info_list)[-1]
+        else:
+            info_list = []
+            for info in thumbnail_infos:
+                t_w = info["thumbnail_width"]
+                t_h = info["thumbnail_height"]
+                t_method = info["thumnail_method"]
+                if t_method == "scale" and (t_w >= d_w or t_h >= d_h):
+                    size_quality = abs((d_w - t_w) * (d_h - t_h))
+                    type_quality = desired_type != info["thumbnail_type"]
+                    length_quality = info["thumbnail_length"]
+                    info_list.append((
+                        size_quality, type_quality, length_quality, info
+                    ))
+            return min(info_list)[-1]
diff --git a/synapse/media/v1/thumbnailer.py b/synapse/media/v1/thumbnailer.py
index ed09283b27..47160721e7 100644
--- a/synapse/media/v1/thumbnailer.py
+++ b/synapse/media/v1/thumbnailer.py
@@ -13,18 +13,22 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import PIL.Image
+import Image
+from io import BytesIO
+
 
 class Thumbnailer(object):
 
-    FORMAT_JPEG="JPEG"
-    FORMAT_PNG="PNG"
+    FORMATS = {
+        "image/jpeg": "JPEG",
+        "image/png": "PNG",
+    }
 
     def __init__(self, input_path):
-        self.image = PIL.Image.open(input_path)
+        self.image = Image.open(input_path)
         self.width, self.height = self.image.size
 
-    def size_preserve(self, max_width, max_height):
+    def aspect(self, max_width, max_height):
         """Calculate the largest size that preserves aspect ratio which
         fits within the given rectangle::
 
@@ -42,12 +46,12 @@ class Thumbnailer(object):
         else:
             return ((max_height * self.width) // self.height, max_height)
 
-    def thumbnail_scale(self, output_path, output_format, width, height):
+    def scale(self, output_path, width, height, output_type):
         """Rescales the image to the given dimensions"""
-        output = self.image.resize((width, height), PIL.Image.BILINEAR)
-        output.save(output_path, output_format)
+        scaled = self.image.resize((width, height), Image.BILINEAR)
+        return self.save_image(scaled, output_type, output_path)
 
-    def thumbnail_crop(self, output_path, output_format, width, height):
+    def crop(self, output_path, width, height, output_type):
         """Rescales and crops the image to the given dimensions preserving
         aspect::
             (w_in / h_in) = (w_scaled / h_scaled)
@@ -61,18 +65,25 @@ class Thumbnailer(object):
         if width * self.height > height * self.width:
             scaled_height = (width * self.height) // self.width
             scaled_image = self.image.resize(
-                (width, scaled_height), PIL.Image.BILINEAR
+                (width, scaled_height), Image.BILINEAR
             )
             crop_top = (scaled_height - height) // 2
             crop_bottom = height + crop_top
             cropped = scaled_image.crop((0, crop_top, width, crop_bottom))
-            cropped.save(output_path, output_format)
         else:
             scaled_width = (height * self.width) // self.height
             scaled_image = self.image.resize(
-                (scaled_width, height), PIL.Image.BILINEAR
+                (scaled_width, height), Image.BILINEAR
             )
             crop_left = (scaled_width - width) // 2
             crop_right = width + crop_left
             cropped = scaled_image.crop((crop_left, 0, crop_right, height))
-            cropped.save(output_path, output_format)
+        return self.save_image(cropped, output_type, output_path)
+
+    def save_image(self, output_image, output_type, output_path):
+        output_bytes_io = BytesIO()
+        output_image.save(output_bytes_io, self.FORMATS[output_type])
+        output_bytes = output_bytes_io.getvalue()
+        with open(output_path, "wb") as output_file:
+            output_file.write(output_bytes)
+        return len(output_bytes)
diff --git a/synapse/media/v1/upload_resource.py b/synapse/media/v1/upload_resource.py
index 91bcc5caff..a78cc3cff3 100644
--- a/synapse/media/v1/upload_resource.py
+++ b/synapse/media/v1/upload_resource.py
@@ -20,10 +20,11 @@ from synapse.api.errors import (
     cs_exception, SynapseError, CodeMessageException
 )
 
-from twisted.web.resource import Resource
 from twisted.web.server import NOT_DONE_YET
 from twisted.internet import defer
 
+from .baseresource import BaseMediaResource
+
 import os
 
 import logging
@@ -31,17 +32,7 @@ import logging
 logger = logging.getLogger(__name__)
 
 
-class UploadResource(Resource):
-    isLeaf = True
-
-    def __init__(self, hs, filepaths):
-        Resource.__init__(self)
-        self.auth = hs.get_auth()
-        self.clock = hs.get_clock()
-        self.store = hs.get_datastore()
-        self.max_upload_size = hs.config.max_upload_size
-        self.filepaths = filepaths
-
+class UploadResource(BaseMediaResource):
     def render_POST(self, request):
         self._async_render_POST(request)
         return NOT_DONE_YET
@@ -99,6 +90,12 @@ class UploadResource(Resource):
                 media_length=content_length,
                 user_id=auth_user,
             )
+            media_info = {
+                "media_type": media_type,
+                "media_length": content_length,
+            }
+
+            yield self._generate_local_thumbnails(self, media_id, media_info)
 
             respond_with_json(
                 request, 200, {"content_token": media_id}, send_cors=True
diff --git a/synapse/storage/media_repository.py b/synapse/storage/media_repository.py
index b3f1fc087b..a848662716 100644
--- a/synapse/storage/media_repository.py
+++ b/synapse/storage/media_repository.py
@@ -56,8 +56,8 @@ class MediaRepositoryStore(SQLBaseStore):
         )
 
     def store_local_thumbnail(self, media_id, thumbnail_width,
-                              thumbnail_height, thumbnail_method,
-                              thumbnail_type, thumbnail_length):
+                              thumbnail_height, thumbnail_type,
+                              thumbnail_method, thumbnail_length):
         return self._simple_insert(
             "local_media_thumbnails",
             {
@@ -108,10 +108,10 @@ class MediaRepositoryStore(SQLBaseStore):
             )
         )
 
-    def store_remote_media_thumbnail(self, origin, media_id, thumbnail_width,
-                                     thumbnail_height, thumbnail_method,
-                                     thumbnail_type, thumbnail_length,
-                                     filesystem_id):
+    def store_remote_media_thumbnail(self, origin, media_id, filesystem_id,
+                                     thumbnail_width, thumbnail_height,
+                                     thumbnail_type, thumbnail_method,
+                                     thumbnail_length):
         return self._simple_insert(
             "remote_media_cache_thumbnails",
             {