I added support for multiple encoding profiles. For each profile, there is a button to delete or copy the profile (the buttons work).

I am also added a change that required the preset name to be machine readable (alphanumeric with underscores and dashes). The key of the preset name is now also equal to the preset name. This way if your first preset is named 'high-quality', then ffmpeg_converter_options('high-quality') will return the options for the 'high-quality' preset. Previously you would have to call ffmpeg_converter_options('preset-1').

Let me know if you want an upgrade path. (It is a matter of updating the keys of variable_get('ffmpeg_converter_presets') to be equal to their names.)

I am new to submitting patches so please excuse any mistakes. I am also not sure whether I have made the patch correctly. Let me know if I've done anything wrong and I'll try to fix it.

Thanks!

CommentFileSizeAuthor
ffmpeg_patch.txt10.37 KBvenkatd

Comments

zoo33’s picture

zoo33 commented
Status: Needs review » Needs work

Thanks, that's great! It's about time we implement this functionality.

I'm a little curious as to what the change was that required you to change how presets names are handled. Why can't we hava a machine readable key and a human readable name?

The patch is kind of big. If you could make it a little smaller, for example by separating out the "copy preset" functionality into its own patch once the rest is done, it would probably be easier have this reviewed and committed.

Also, I'd like to see all functions properly documented as described in the coding standards.

Again, thanks!

venkatd’s picture

venkatd commented

Hi. Thanks for the feedback. I'll try breaking the patch up into smaller pieces. I'll also try to make the code adhere to the coding standards.

I changed how the presets are handled because I think the machine readable keys will sometimes be used by humans and should also be human readable. I was using ffmpeg_wrapper in code and I had to refer to an encoding profile as 'preset-1'. I would have preferred to refer to the preset as 'high-quality'. Then every time in my code I need to encode a video with high quality I can use the 'high-quality' key and not have to remember 'preset-1' or figure the key out with print_r statements.

In hindsight, maybe allowing the user to change the machine readable key isn't such a good idea. If the key is changed then modules referencing the key will break. Any ideas?

zoo33’s picture

zoo33 commented
Title: Patch to Support Multiple Encoding Profiles » Support Multiple Encoding Profiles

Well, CCK and other various modules let the user choose the machine readable names for their objects, so maybe it isn't such a bad idea after all. But if we can avoid that part for the time being the patch will be smaller and easier to review and test. After all, it doesn't seem to be necessary for what this issue tries to achieve. What do you think?

venkatd’s picture

venkatd commented

I agree. I will leave out the ability to pick a machine-readable name in these patches to keep it simple.

I've been a bit busy lately. I expect to have the patches written sometime this week or next week.

sinasalek’s picture

sinasalek commented

+1

zoo33’s picture

zoo33 commented
Status: Needs work » Closed (duplicate)

The ability to add more profiles is there now: #528974: patch to add new encoding preset

If someone wants to work on user provided (machine readable) preset names, please open a new issue.