...было неочевидно, есть ли в нем ошибки.
Афоризм классный, жизненный, не мой :)
В том, что он жизненный, мне довелось убедиться на днях. Захотелось посмотреть, насколько безопасно передавать уже использовавшийся AVPacket в av_read_frame. Здравый смысл подсказывал, что должно быть безопасно и что внутри av_read_frame сам FFMPEG должен вызывать av_packet_unref(). Но хотелось убедиться.
Заглянул в потроха FFMEG, дошел до внутренней функции read_frame_internal и слегка офигел. Этот самый афоризм тут же вспомнился. В той его части, которая про "неочевидно, есть ли в нем ошибки" :(
Ну, например: если смотреть по диагонали, то кажется, что перед возвратом ошибки делается вызов av_packet_unref() для очистки содержимого AVPacket. Вот как здесь. Но если попробовать присмотреться, то находятся места, где это не так (тыц или тыц). И ведь сходу и не поймешь, правильно ли это или нет. Неочевидненько...
Был бы я помоложе, наверняка бы высказал свое "фи" в той или иной форме. Но годы таки берут свое и сейчас я уже вовсе не уверен в правильности своих оценок. Поэтому смотрю я на эти 200 строк одной функции и не понимаю: программирование в таком духе -- это нормально или нет? Может это просто я безнадежно застрял в своем манямирке и уже не представляю, как пишется промышленный код в больших проектах?
В общем, вопрос, который поставил меня в тупик. И ответ на него искать боязно, ибо если мои представления о "хорошем коде" безнадежно устарели, то как-то совсем уж безнадегой веет от перспективы и дальше оставаться в этой профессии.
Еще один аспект: мне сложно представить, как можно было бы улучшить данный код оставаясь в рамках чистого Си.
Это вообще больной вопрос, когда приходится сталкиваться с Си: вроде бы код на Си читаю и понимаю что происходит. Но стоит задуматься о том, как на чистом Си что-то реализовать, как возникает ощущение, что не понимаю, как на этом языке вообще можно что-то надежно и качественно писать. Тут ведь все на внимании и скрупулезности программиста зиждется. А вот чего у меня нет, того нет... :(
В рамках C++ первое, что пришло в голову, так это создание вспомогательной обертки, в которую бы передавался указатель на AVPacket и в которой бы содержался признак актуальности значения по этому указателю (назовем этот признак committed). В деструкторе этой обертке проверялось бы значение committed и, если оно не выставлено, то для AVPacket вызывался бы av_packet_unref:
class AVPacketCleaner {
AVPacket * pkt_;
bool committed_{false};
public:
AVPacketCleaner(AVPacket * pkt) : pkt_{pkt} {}
~AVPacketCleaner() { if(!committed_) av_packet_unref(pkt_); }
void commit() noexcept { committed_ = true; }
};Тогда бы в read_frame_internal (или даже выше, еще в av_read_frame) можно было бы написать что-то вроде:
int av_read_frame(AVFormatContext * s, AVPacket * pkt) {
...
AVPacketCleaner cleaner{pkt};
...
// Перед тем return-ом, который возвращает признак успеха:
cleaner.commit();
return 0;
err:
... // Обработка ошибок.
}И все, не пришлось бы париться на предмет того, следует ли при очередном возврате кода ошибки вызывать вручную av_packet_unref() или нет.
Вторая идея, которая пришла в голову чуть позже: это использование идиомы swap с временным значением. Т.е., чтобы было что-то вроде:
int av_read_frame(AVFormatContext * s, AVPacket * pkt) {
...
AVPacket tmp_pkt{...}; // Создание временного экземпляра.
... // Далее работа только с tmp_pkt.
// Перед тем return-ом, который возвращает признак успеха:
swap(tmp_pkt, *pkt);
return 0;
err:
... // Обработка ошибок.
}При этом подразумевается, что у AVPacket есть деструктор, который вызывает av_packet_unref().
Т.е., за годы работы на C++ выработалась привычка использовать возможности C++ так, чтобы код требовал меньшего внимания с моей стороны и всякий мелкий (и не очень) мусор бы подчищался бы автоматически. Но в стандартном чистом Си ничего подобного нет, поэтому даже сама мысль о том, чтобы что-то запрограммировать на "теплой и ламповой Сишечке", пробивает на холодный пот ;)
