https://bugs.gpodder.org/show_bug.cgi?id=1263
--- Comment #2 from Guy Sheffer <[email protected]> 2011-01-13 15:36:48 GMT --- Good replies, let me just add a few inline comments: (In reply to comment #1) > Finally had time to review the patch. Some feedback: > > - Please avoid introducing a new dependency on "kaa" This is only required to get the video's size, then resizing it to for the right aspect ratio of the device's screen. Might anyone know of a way to get the dimensions of the screen? I'll search within ffempg which this depends on anyway. > - Do not refer to the GUI code (import message) from libconverter > (because your code should work in CLI mode and in a different UI as well) message.py was added just to workaround. To do this I need somehow to pass the notify function to the converter, I can't track on what scope to pass it to the converter class function. > - Please remove all "print" statements - use liblogger if you really need to Yes, sorry about that, will be removed. Its just good for debugging around here. > - Write a decoder script and use it instead of "oggdec" I wrote a bash script initially, oggdec is not a script. What language should it be in? At the time you said I should avoid anything external like that. > - Move the MP3 encoding command to the add_converter call (so you can > have a different one for your mp4 decoder) Could be done, but note that the command is very different than the piping done in oggdec (no stdout and stuff like that). I would need to add a new class that extends the current one. > - The same is true for the output filename extension - make this > a parameter as well (mp3 for the ogg converter, mpg for yours) Will do. I knew there sould be something like this once i saw the line "+ 'mp3'" > - The deviceWidth commands and stuff like that should be moved to > your converter script and not placed into libconverters.py It should be placed as a config parameter in gpodder, it seems to be the right thing to do. Should be at the user's reach, because they will want to change this. > > If you need more help, just ask me on IRC. I will gladly :) -- Configure bugmail: https://bugs.gpodder.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes. _______________________________________________ gPodder-Bugs mailing list [email protected] https://lists.berlios.de/mailman/listinfo/gpodder-bugs
