symfony-cmf / media-bundle

UNMAINTAINED - Minimalistic interfaces to handle media in the context of the CMF
http://cmf.symfony.com/
30 stars 40 forks source link

improve integration with LiipImagineBundle #78

Open lsmith77 opened 10 years ago

lsmith77 commented 10 years ago

it would be good to have a way to ensure that the file format is included in the paths generated via LiipImagineBundle.

f.e.

{{ /cms/media/foo | imagine_filter('small_filter') }}

might then result in

/media/cache/small_filter/cms/media/foo

however this can cause issues with some web servers and reverse proxies, as they would then set the content type to text/plain.

so we need some solution which would allow forcing the format for the mimetype into the resulting URL.

see also #77 for how to convert the mimetype to an extension

levuro commented 10 years ago

if i might add a example -

{{ /cms/media/foo | imagine_filter('small_filter') }}

so if /cms/media/foo contains a jpeg the thumbnail will be named /media/cache/small_filter/media/foo.jpeg

rmsint commented 10 years ago

There is no option to tell the LiipImagineBundle what the extension is from the image object?

In the CmfMediaDoctrineLoader the image object is loaded, if it is possible to tell then what the extension should be the FileInterface::getExtension method can be used. And/ or the guessExtension method could be added and used like done in the symfony core File.

lsmith77 commented 10 years ago

@havvg do you maybe have an idea here?

havvg commented 10 years ago

Well, if you configure a format on the filter-set, it should add the configured format as extension to the generated url. However based on your root problem this could end up in erroneous images, if you are serving e.g. a png image with .jpg file extension and an image/png content-type - dunno how the broken server/proxy will handle this. It looks strange to me, that the reverse proxy is messing it up, shouldn't it use the content-type provided by the backend, regardless of the uri?

rmsint commented 10 years ago

I see in the `FilterManager::get that the format is detected based on the path if it is not found in the config: https://github.com/liip/LiipImagineBundle/blob/master/Imagine/Filter/FilterManager.php#L73 Isn't the content-type set by the LiipImagineBundlle here?

And if the format is set to 'png' is that not only the generated cache file? Although the original image can have a different format.

lsmith77 commented 10 years ago

@havvg the goal is to determine the output format type dynamically.

/foo/bar finds the image stored under foo/bar in the storage. then the format is determined dynamically (from the data or via some meta data API in the storage) and is then placed into the final url. for example ``/media/cache/foo/bar.jpg)

havvg commented 10 years ago

Well, a possible solution could be combining the DataLoader with the CacheResolver into a duplex resolver.

Assumption: The DataLoader is reading the image from your storage, including the meta information.

The Resolver part (called with /foo/bar) will trigger the resolving into /media/cache/foo/bar.jpg as a string. In order to do this, the DataLoader is combined, so you read the image from the storage. The image will still be retrieved from the DataLoader by its original name /foo/bar. The DataLoader::find method will be called twice, so cache the result. The CacheManager will then store the filtered image under the resolved path /media/cache/foo/bar.jpg, thus the Resolver::store will be issued with the final filename.

On future calls (above has taken place once) the Resolver is required to do its normal duty. It will still be called with /foo/bar and should be able to determine the existence of the final file (e.g. /foo/bar.jpg on AmazonS3). If it's not there, this is the first call, above process is due.

havvg commented 10 years ago

The second part should be wrapped by the https://github.com/liip/LiipImagineBundle/blob/v0.16.0/Imagine/Cache/Resolver/CacheResolver.php to avoid retrieving the meta information every time (see https://github.com/liip/LiipImagineBundle/blob/v0.16.0/Resources/doc/cache-resolver/cache.md).

dbu commented 10 years ago

for the CmfMediaBundle, the call to make up the path from an id would need to end up in https://github.com/symfony-cmf/MediaBundle/blob/master/Doctrine/Phpcr/MediaManager.php#L121 where we would append the extension based on the mime type if there is no explicit extension asked for.

dbu commented 10 years ago

seems this won't happen for release 1.1

maybe in a couple months, the LiipImagineBundle has a stable 1.0 out and we can revisit the integration