Comments 42
Я не догадался почему не использовали ImageMagick, намекните.
для анимированных GraphicsMagick лучше
ну как же, про это писано переписано: во-первых, ImageMagick не включен в стандартную поставку PHP, а от сюда и дополнительные телодвижения с установкой/настройкой или еще хуже просьбой хостеров поставить даный пакет; во-вторых, не настолько прост и очевиден код ресайза анимированных gif изображений; и в в-третьих, у даного программного продукта также существует проблемы с корректным ресайзом даных файлов; (первая и третья причины основные)
Ну и последнее замечание и ссылка на класс: в классе полностью отсутствует форматирование кода, это связано как с оптимизацией размера самого файла так и с принципом «пользователь не должен знать внутреннюю структуру, только открытые функции», а кто хочет модернизировать или дополнить класс без проблем отформатирует так как ему надо.
Ну и какой смысл от вашего поделия на гитхабе тогда? Если уж выкладываете, то будьте готовы к оцениванию вашего кода другими разработчиками, но с таким форматированием это отбивает всякое желание.
Пользователь будет смотреть на картинку, а не копаться в коде. Ваши бессмысленные оптимизации размера файла мне непонятны. Экономия на спичках.
в классе полностью отсутствует форматирование кода, это связано как с оптимизацией размера самого файла так и с принципом «пользователь не должен знать внутреннюю структуру, только открытые функции»
Место на диске экономите? Выкладывать такое на гитхаб, простите, мудатсвто :)
Нет, просто автор хотел незаметнее авторство своё увековечить.
Ну правда, отформатируйте код, тем более в IDE выдаются ошибки, типа Undefined variable ($buffer_add, $lc_mod, $lc_palet, $head, $gr_mod, $palet, $f_buf, $con).
$con = "\x21\xFE\x0Eyuriy_khomenko\x00" . $con;
Ну правда, отформатируйте код, тем более в IDE выдаются ошибки, типа Undefined variable ($buffer_add, $lc_mod, $lc_palet, $head, $gr_mod, $palet, $f_buf, $con).
Нет, просто автор хотел незаметнее авторство своё увековечить.))))) увековечить да, скрыть нет, но в шапке описания также автор присутствует не пойму разве у меня нет прав сделать это? IDE выдаются ошибки, типа Undefined variable наверное это не ошибки, а предупреждения «хорошего стиля программирования», переменные определяются и инициализируются только там где они применяются не более и не менее, и не нужно к коду PHP применять параноидальные требования аля С++
Так с отсутствия «хорошего стиля программирования» и начинаются:
— Ну а че? Пусть себе лежат нигде не используемые приватные функции, тебе что мешают?
— Зачем включать параноидальные E_NOTICE, подумаешь, необъявленная параменная проскочит.
— Ну и что, что WARNING'и сыпятся, код-то работает!
— Что-то error.log файлы распухают, давай их выключим, кто в них смотрит?
— Ну а че? Пусть себе лежат нигде не используемые приватные функции, тебе что мешают?
— Зачем включать параноидальные E_NOTICE, подумаешь, необъявленная параменная проскочит.
— Ну и что, что WARNING'и сыпятся, код-то работает!
— Что-то error.log файлы распухают, давай их выключим, кто в них смотрит?
еще можно покритиковать сам PHP за слабую типизацию переменных и тд и тп, скрипт написан не выходя из тех рамок которые определены средой разработки то есть самим PHP, NeLexa ошибся указывая на ошибку типа Undefined variable (переменная не определена), здесь присутствует место «не инициализация переменной при объявлении», а на это решено было пойти намеренно через оптимизацию, сейчас объясню почему: переменные перечисленные NeLexa (и не только они) объявляются до входа в область видимости цикла для доступности к ним после выхода из него (хотя PHP позволяет и доступ к лок переменной после выхода из лок области, все вопросы к разработчикам PHP, я как раз тут и придерживался общепринятого стиля), почему же я ее сразу не инициализировал? а все потому что сразу при входе в лок область цикла переменной присваивается значение, существуют GIF файлы состоящие с 500 и даже более фреймов (количество не ограничено), для каждый фрейм прогоняется несколько раз через циклы для разбора, и это не такая быстрая задача скорее наоборот, а теперь давайте посчитаем 500 * 1(в лучшем случае, здесь могло быть и 2 и 3) * 10 (количество переменных, их больше десять просто для наглядности) = 5000 раз бессмысленной инициализации(и это в одном месте) за которой идет присваивание, когда и так не факт что скрипт будет выполнятся дефолтное для PHP скрипта время (30 сек); 500 фреймов может и редкость, но 100 обыденное дело и тут уже борьба за скорость ресайза (боролся за каждую долю секунды, для конечного пользователя), а теперь представте что пользователь ресайзит сразу пачку файлов, я в своем проекте масштабирую сразу 3 картинки (один исходный файл в 3 вариантах размера), в скрипте кстати можно увидеть очень длинные выражения и все из за того чтобы не использовать промежуточные переменные… думаю приблизительно объяснил причину
ссылку на гитхаб убрал
какой то не конструктивное обсуждение, «зачем», «почему», «гитхаб», «форматрование»… жду поста типа «нашел файл с проблемой ресайза»
сейчас на PhpClasses скину отформатированную версию со всеми правилами
вернул ссылку на GitHub с полным форматированием кода
Шикарный вывод ошибок. :)
if ($this->er) {
printf("ERROR: signature file is incorrectly");
return 0;
}if ($new_x == 0 || $new_y == 0) {
printf("ERROR: size height or width can not be equal to zero");
return 0;
}$des = Array(0, 0, 0);
Ладно вам =)
Позиция автора ясна — сделать так чтобы работало быстро.
Если это причесать, убрать странности типа:
Получится вполне ничего ;)
Позиция автора ясна — сделать так чтобы работало быстро.
Если это причесать, убрать странности типа:
$lc_i = ord($this->gif[$this->pnt + 2]);
$sum = 2;
while (($lc_i = ord($this->gif[$this->pnt + $sum])) != 0x00) {
$sum+=$lc_i + 1;
}
Получится вполне ничего ;)
укажите на странность, чтобы было что объяснять, как я писал выше код три раза переписывался (практически изменился до неузнаваемости), сейчас в коде практически все вылизано и заточено под немогу, первый вариант скрипта «ложился» через нехватку памяти, для файла размером 6 Мб требовалось памяти 700 МБ (это была первая проблема), вторая как я писал скорость самого ресайза, которую тоже пришлось решать и вроде получилось, вами приведенный код наверное странноват (сам правда не пойму причину странности) из за оптимизации ресайза, уточните непонятный кусок кода все объясню или весь приведенный код объяснять?
Ну хотя бы зачем первое присваивание
Раз уж вы ратуете за экономию тактов процессора?
$lc_i = ord($this->gif[$this->pnt + 2]);Раз уж вы ратуете за экономию тактов процессора?
в первом варианте скрипта я циклом проходился по всем битам файла во время анализа и рассортировки всей структуры по полочкам, но после когда код был реализован были большие проблемы с производительностью особенно на больших файлах, в последующем я изменил тактику, теперь я не бегу по всем битам файла, а откусываю кусками, в данном случае идет разбор расширения (в формате файла GIF89a их предусмотрено 4), а именно расширения-коментаррия, стандарт дает гарантию (и его обычно придерживаются), что в третьем бите хранится число обозначающее размер данных мы его сразу и получаем, то есть если в этом поле будет сохранена книга Война и мир (что вполне реально), мы ее откусим одним куском без пробега по ее символам, дальше стандарт можно сказать ничего не гарантирует (точнее его никто не соблюдает) то приходится просчитывать размер всего расширения циклом(но обычно основной объем данных ЗНАЧИТЕЛЬНО превышает размер технических данных отсюда и выигрыш), и все суммируя, после уже суму которая набежала и которая равна размеру расширения, передаем в функцию которая непосредственно выдирает эти данные из файла и сохраняет в переменную класса… думаю ясно объяснил
Понятно, но бесполезно.
Я намекал на то что эту строку и еще пару аналогичных можно выкинуть, т.к. они не несут никакой практической пользы. При входе в цикл $lc_i в любом случае инициализируется значением ord($this->gif[$this->pnt + $sum]), где $sum = 2.
Я намекал на то что эту строку и еще пару аналогичных можно выкинуть, т.к. они не несут никакой практической пользы. При входе в цикл $lc_i в любом случае инициализируется значением ord($this->gif[$this->pnt + $sum]), где $sum = 2.
я только что присмотрелся, да вы полностью правы, это торчат уши с второй версии скрипта, забыл убрать, убрал только в 88 строчке скрипта, а во всем проекте забыл, спасибо что ткнули носом, если вы не против, могу отблагодарить вас в шапке следующей версии скрипта как человека помогающего развитию кода…
Шикарный вывод ошибок. :)и что, в чем собственно проблема.......?
кого «их»? причем здесь ob_start? пишите яснее чтобы я не догадывался… скрипт отлавливает всего два простых вида ошибок: первая — неправильные параметры, которые отвечают за новый размер изображения(размер не может быть нулевой); вторая — неправильный формат файла, или другими словами все не gif файлы отметаются;
Я таки поясню =)
Обычно при возникновении ошибки функция должна вернуть false/NULL/кинуть исключение, чтобы на уровне выше можно было адекватно отреагировать на неё. В вашем случае отдается 0, что можно понять и… в stdout срется текст, что не есть гуд.
Почему? Если ваш скрипт будет использован на фронтенде, а не как консольная приблуда — чтобы гарантировать нормальный вывод данных пользователю необходимо использовать ob_start чтобы заглушить вывод сообщения.
Обычно при возникновении ошибки функция должна вернуть false/NULL/кинуть исключение, чтобы на уровне выше можно было адекватно отреагировать на неё. В вашем случае отдается 0, что можно понять и… в stdout срется текст, что не есть гуд.
Почему? Если ваш скрипт будет использован на фронтенде, а не как консольная приблуда — чтобы гарантировать нормальный вывод данных пользователю необходимо использовать ob_start чтобы заглушить вывод сообщения.
да в принципе правильное замечание, сперва хотел прикрутить выброс исключения, но потом ради двух ошибок плюнул на это дело, думал также возвращать ошибку в виде картинки, или массив в первом элементе код ошибки во втором описание, но так у меня в коде где используется данный класс параноидальная проверка всего входящего то от этих идей отказался, в принципе не только у меня и а каждого должна быть может не такая но все же достаточная фильтрация дабы некорректные данные в сам класс не дошли (вылов подобных ошибок в классе мягко говоря не верно), здесь она(обработка ошибок) скорее для отладки, и последнее как я писал в паспорте скрипта скрипт разрешается изменять/модифицировать, пусть пользователи реализуют такую обработку которая им надо
с подачи AotD вышел профиксеный вариант класса GIF_eXG (текущая версия 1.03)
обновил версию класса GIF_eXG (добавлена поддержка некоторых нестандартных форматов файлов) (текущая версия 1.04)
Вот такое масло масляное
раз уж мы говорим об оптимизации стоит убрать, ибо оно ничем не оправдано.
А здесь я бы на вашем месте использовал бы case. Конечно в данном примере это не даст большого выигрыша, но привычка плохая… Ибо на месте массива, может оказаться ёмкий по времени метод, который будет вызываться несколько раз вместо одного.
А вообще, раз уж вы представляте код комьюнити, стоит оформить его, как для комьюнити. Я конечно не говорю о композере, но пхпдоки и человечное форматирование кода, были бы очень кстати, тем более в любой современной IDE, это делается нажатием одного хоткея ;)
$lc_i = ord($this->gif[$this->pnt + 9]) & 128 ? 1 : 0;
...
if ($lc_i) { ...
раз уж мы говорим об оптимизации стоит убрать, ибо оно ничем не оправдано.
А здесь я бы на вашем месте использовал бы case. Конечно в данном примере это не даст большого выигрыша, но привычка плохая… Ибо на месте массива, может оказаться ёмкий по времени метод, который будет вызываться несколько раз вместо одного.
...
elseif ($this->gif[$this->pnt + 1] == "\xFF") {
...
} elseif ($this->gif[$this->pnt + 1] == "\x01") {
...
/**
А можно так
*/
switch ( $this->gif[$this->pnt + 1] ) {
case "\xFF":
...
case "\x01":
...
}
А вообще, раз уж вы представляте код комьюнити, стоит оформить его, как для комьюнити. Я конечно не говорю о композере, но пхпдоки и человечное форматирование кода, были бы очень кстати, тем более в любой современной IDE, это делается нажатием одного хоткея ;)
Спасибо за комментарий. Сперва спрошу, вы сами анализировали код или с помощью анализаторов? Мне выше посоветовали phpMess Detector но как я его не прикручивал так и не прикрутил. Хочу также заметить что никак не ожидал такой дотошной разборки кода, но никак не критикую, а наоборот рад за это и благодарю каждого кто приложил/приложит усилия для улучшения кода (и каждого обязательно отмечу в следующем релизе класса). А теперь все по порядку:
1) масло масленое
вы это вырвали с контекста
как раз вторая строчка передвигает глобальный указатель, и нам надо успеть проанализировать второй байт(точнее бит) от текущего места, дабы знать куда двигаться после того как в переменную $head будет записан один из блоков GIF файла, в данном случае проверяем присутствие локальной палитры, можете сделать тест, отформатировать так как вы говорите, прогнать через класс анимированный GIF файл, а потом еще раз прогнать отресайзеный результат (как раз в выходном файле появляется локальная палитра) и как раз тут скрипт войдет в крутое пике или ступор (как кому нравится), поэтому здесь такая последовательность операторов;
2) второе замечание принимается, почему то ну недолюбливаю конструкцию switch-case, обычно использую если вариантов выбора более пяти, а класс по вашему совету обязательно откорректирую;
3) третье замечание в принципе тоже верно, но из за неимения свободного времени плюс к этому прабл с ин языками (а комьюнити у нас не только русскоязычное) пришлось ограничится только шапкой, не понял причем здесь компосер, насколько я помню это средство работы с библиотеками, а я использую только одну стандартную библиотеку GD, и последнее, а чем вам форматирование собственно не нравится? это как раз и есть NetBeans, или может вы предпочитаете аля PhpStorm ))))<source
1) масло масленое
$lc_i = ord($this->gif[$this->pnt + 9]) & 128 ? 1 : 0;
...
if ($lc_i) { ...
вы это вырвали с контекста
$lc_i = ord($this->gif[$this->pnt + 9]) & 128 ? 1 : 0;
$head = $this->gtb(10);
if ($lc_i) {
как раз вторая строчка передвигает глобальный указатель, и нам надо успеть проанализировать второй байт(точнее бит) от текущего места, дабы знать куда двигаться после того как в переменную $head будет записан один из блоков GIF файла, в данном случае проверяем присутствие локальной палитры, можете сделать тест, отформатировать так как вы говорите, прогнать через класс анимированный GIF файл, а потом еще раз прогнать отресайзеный результат (как раз в выходном файле появляется локальная палитра) и как раз тут скрипт войдет в крутое пике или ступор (как кому нравится), поэтому здесь такая последовательность операторов;
2) второе замечание принимается, почему то ну недолюбливаю конструкцию switch-case, обычно использую если вариантов выбора более пяти, а класс по вашему совету обязательно откорректирую;
3) третье замечание в принципе тоже верно, но из за неимения свободного времени плюс к этому прабл с ин языками (а комьюнити у нас не только русскоязычное) пришлось ограничится только шапкой, не понял причем здесь компосер, насколько я помню это средство работы с библиотеками, а я использую только одну стандартную библиотеку GD, и последнее, а чем вам форматирование собственно не нравится? это как раз и есть NetBeans, или может вы предпочитаете аля PhpStorm ))))<source
опечатка, в коде выше мы анализируем не второй байт а десятый (что в принципе ясно с кода)…
Ну насчёт первого — я до сих пор не очень понял, почему там так, но видимо там действительно так нужно.
Анализаторы использую редко и ИМХО — это плохая практика. Код ревью может быть так же эффективно и при этом приносит пользу всем участникам :)
Анализаторы использую редко и ИМХО — это плохая практика. Код ревью может быть так же эффективно и при этом приносит пользу всем участникам :)
вышла новая версия класса — 1.05, включающая общие кодовые улучшения, а также поддержка новых нестандартных форматов GIF файлов
версия 1.07 — корректный ресемплинг
Sign up to leave a comment.
Ресайз анимированных GIF картинок с помощью PHP + GD