Skip to content
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.

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

[RFC] Better format conversion handling #277

Closed
LouTerrailloune opened this issue Dec 2, 2013 · 8 comments
Closed

[RFC] Better format conversion handling #277

LouTerrailloune opened this issue Dec 2, 2013 · 8 comments
Milestone

Comments

@LouTerrailloune
Copy link
Contributor

We have a few issues related to formats (like #231 and #276).

I don't think trying every allowed format in all data loader is the right way to do it, so I suggest these 2 things:

  1. Dataloader should only load an Image corresponding to an ID (which may be filename but it's not mandatory, it can be a database id).
  2. The urls of the pictures should include the source ID AND the wanted format. I suggest the following format: /prefix/filter_name/ID.to.FORMAT

Remarks:

  • If the Data loader will only receive the ID.
  • The picture is generated in format FORMAT.
  • If the ID is a filename and is already in the right format, don't add .to.FORMAT
  • This is BC break
  • I started something to implement this but it's not yet ready (and it will not be merged anyway because of the BC break)

What do you think about that ?

@dbu
Copy link
Member

dbu commented Dec 3, 2013

i don't like the .to.FORMAT part. i would say: if the format is already in the id, keep it. otherwise just append .FORMAT and when loading, we could ask with the id and hint the format (in case there is both image.png and image.jpg in the same folder for example).

@dbu
Copy link
Member

dbu commented Dec 3, 2013

sorry, did not want to sound negative. apart from the details, i agree that we should improve on this topic.

this is probably also a bit related to how we create paths in general. right now this goes through the cache resolver and assumes a path, while in the cmf the thing we want to convert to an image path is an object: symfony-cmf/media-bundle#59 . there is an almost duplicate issue to this one at symfony-cmf/media-bundle#78 , talking about the phpcr-odm case. and #170.

@LouTerrailloune
Copy link
Contributor Author

The .to. is here to distinguish between format present in the ID and added format.

For exemple:

  1. Without the .to. :

    ID = 123, wanted format = jpg => 123.jpg
    ID = 123.jpg, wanted format = jpg => 123.jpg

    The controller cannot guess the real ID.

  2. With .to. :

    ID = 123, wanted format = jpg => 123.to.jpg
    ID = 123.jpg, wanted format = jpg => 123.jpg

    Now the controller can extract the real ID, because the filename ends with .to.FORMAT.

My idea was to allow .to. to be configurable.

@dbu
Copy link
Member

dbu commented Dec 3, 2013

yep, i see that. and then ID = 123.png, wanted format jpg => 123.png.to.jp ? feels a bit clumsy to me. what if we simply request twice? once 123.jpg and then if that yields nothing, 123 which means the loader can decide on the format? or as i proposed above, request '123', 'jpg', where 'jpg' is the format hint which might be respected but does not need to be?

@LouTerrailloune
Copy link
Contributor Author

Yes that means 123.png.to.jpg. This not very beautiful but extension, mime type and content will match, and the id is extractable.

Requesting twice might be problematic for some data loader. For exemple if you use a database id as the ID (like in phpcr), the first request will always fail, the second will get content (not very efficient).

@dbu
Copy link
Member

dbu commented Dec 4, 2013

the same is with filesystem access. i would not be worried by that, once its loaded it will be cached.

but lets not get sidetracked by this, its not that important. i can live with the .to.jpg if others think its better.

@havvg @lsmith77 any input from your side?

@havvg
Copy link
Contributor

havvg commented Dec 13, 2013

I don't like the extension of the name to "pass" information. The whole format conversion topic is buggy and I don't have a good solution on this. It's especially tricky if you ignore the fact that your actually handling files. The bundle must not rely on the fact there are files.

We are currently in process of getting 1.0.0 out the door. I will move this into the respective milestone and we take a look into this feature later.

@makasim
Copy link
Collaborator

makasim commented Mar 14, 2014

in 1.0 the identificator (path or db id) is only required. The format and mimetype is guessed using file content.

@makasim makasim closed this as completed Mar 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants