Здравствуйте, B0FEE664, Вы писали:

BFE>>>Рад, что вы ждёте конструктивной критики.
S>>Для конструктивной нужно, чтобы критик понимал предмет разговора. Так что нет, не жду.
BFE>Т.е. вы думаете, что ваш код никто не поймёт?

Я очень надеюсь, что критиковать код начнут после того как поймут, а не до.

BFE>>>Похоже, что image_bytes объявлен как указатель на const, если это так — то это зачем?
S>>Да, это const std::byte*, приходящий извне.
BFE>Но зачем нарушать контракт про "const"?

Потому, что в FFMPEG нужно иметь неконстантный указатель.

BFE>>>Почему обращение к fresh_frame->data по индексу, а не по итератору?
S>>Потому что это Си-ный массив. Имитировать итераторы через вызовы std::begin/end не вижу смысла.
BFE>Почему "эмитировать"? То, что в данном случае итератор — это указатель, ничего не меняет.

Потому, что мне здесь "итераторы" в каком-либо виде не нужны. Если вам кажется, что нужны, то, наверное, для этого есть какие-то доводы?

BFE>Это дело вкуса.

Скорее опыта.

BFE>То, что assert-ы не работают в release — опасное заблуждение.

Если assert-ы работают в релизе, то это уже не assert-ы.

S>>Во-вторых, это гарантируется в принципе Upd: гарантируется, что (std::size(fresh_frame->data) >= std::size(data_plane_sizes)). By design, так сказать.
BFE>Если By design, то где static_assert?

В Караганде, вероятно. Потому что by design. Т.к. если в FFMPEG внезапно сделают так, что в AVFrame::data будет меньше элементов, чем в аргументе linesizes для av_image_fill_plane_sizes, то нужно будет что-то менять в консерватории. Может быть законченный параноик будет напишет (не один) static_assert чтобы защититься от такой гипотетической ситуации, но я подобной паранойей еще не страдаю.

Вот по поводу Си-шных кастов паранойя есть, т.к. там опасности не такие гипотетические.

BFE>>>Зачем столько вызовов ptr_cast? Не проще ли завести один указатель?
S>>Потому что исходный указатель, image_bytes, постоянно изменяется (это даже в приведенном кусочке видно). А по ходу дела его приходится приводить к std::uint8_t* (да, без const, такова специфика используемой внешней библиотеки). Можно было бы, конечно, вести сразу два указателя, image_bytes и второй, который имеет тип std::uint8_t. Но какой смысл дублировать одно и то же?
BFE>Чтобы не полагаться на оптимизатор.

Оптимизатор-то здесь причем? Или вы думаете, что на reinterpret_cast+const_cast есть какие-то накладные расходы в run-time?

BFE>>>Уверены, что []( void *, std::uint8_t * ) -> void {} не ведёт к утечке памяти? (>24 байта за вызов)
S>>Мне сложно представить откуда здесь может взяться утечка, т.к. этот фрагмент -- это получение указателя на функцию, которая ничего не делает. Данный указатель требуется для av_buffer_create в качестве "деаллокатора".
BFE>А то я не вижу!

Раз высказываете такие опасения, значит не видите. Или видите что-то свое.

S>>Но т.к. деаллокатор здесь не нужен, то и функция пуста.
BFE>Вот и хотелось бы понять почему деаллокатор не нужен.

Потому что это деаллокатор не для AVBuffer, а для AVBuffer::data.
Вызывается этот деаллокатор здесь (через здесь):

static void buffer_replace(AVBufferRef **dst, AVBufferRef **src)
 {
     AVBuffer *b;
  
     b = (*dst)->buffer;
  
     if (src) {
         **dst = **src;
         av_freep(src);
     } else
         av_freep(dst);
  
     if (atomic_fetch_sub_explicit(&b->refcount, 1, memory_order_acq_rel) == 1) {
         /* b->free below might already free the structure containing *b,
          * so we have to read the flag now to avoid use-after-free. */
         int free_avbuffer = !(b->flags_internal & BUFFER_FLAG_NO_FREE);
         b->free(b->opaque, b->data);
         if (free_avbuffer)
             av_free(b);
     }
 }

BFE>Таким образом: вы всё ещё уверены, что здесь нет утечки памяти?

Я ни в чем на 100% не уверен, но в данном случае очень надеюсь.

Здравствуйте, so5team, Вы писали:

S>Я очень надеюсь, что критиковать код начнут после того как поймут, а не до.
Ну уж нет. Код должен читаться, а не являться ребусом.

BFE>>>>Похоже, что image_bytes объявлен как указатель на const, если это так — то это зачем?
S>>>Да, это const std::byte*, приходящий извне.
Да, это я не правильный вопрос задал. Попытаюсь ещё раз. Почему извне приходить указатель на константные данные?

S>Потому, что мне здесь "итераторы" в каком-либо виде не нужны. Если вам кажется, что нужны, то, наверное, для этого есть какие-то доводы?
Вот этот цикл:

std::size_t plane_index = 0u;
for( const auto plane_size : data_plane_sizes )
{
    fresh_frame->data[ plane_index ] = ptr_cast( image_bytes );
    image_bytes += plane_size;
    ++plane_index;
}

я бы переписал так:

auto pImgBytes = ptr_cast( image_bytes );
auto pData     = std::begin(fresh_frame->data);
for( const auto plane_size : data_plane_sizes )
{
    *pData++   = pImgBytes;
    pImgBytes += plane_size;
}

BFE>>То, что assert-ы не работают в release — опасное заблуждение.
S>Если assert-ы работают в релизе, то это уже не assert-ы.
А что тогда?

S>В Караганде, вероятно. Потому что by design. Т.к. если в FFMPEG внезапно сделают так, что в AVFrame::data будет меньше элементов, чем в аргументе linesizes для av_image_fill_plane_sizes, то нужно будет что-то менять в консерватории. Может быть законченный параноик будет напишет (не один) static_assert чтобы защититься от такой гипотетической ситуации, но я подобной паранойей еще не страдаю.
Вот видите, для понимания вашего кода нужно знать некий внешний контекст. Т.е. гарантии того, что код будет работать лежат где-то ещё, а не в самом коде. В результате код не очень-то хорош.

S>Потому что это деаллокатор не для AVBuffer, а для AVBuffer::data.
Ага. теперь понятно.

Здравствуйте, B0FEE664, Вы писали:

S>>Я очень надеюсь, что критиковать код начнут после того как поймут, а не до.
BFE>Ну уж нет. Код должен читаться, а не являться ребусом.

Код должен решать задачу. При этом (но не вместо!) он должен быть понятным, корректным, надежным, сопровождабельным и т.д.
Но код пишется для решения задачи. И так бывает, что для решения задачи нужно понимание особенностей предметной области (хотя бы на уровне знакомства с используемым инструментарием), без которого (без понимания в смысле) читабельность кода -- это очень эфемерное, но спекулятивное понятие.

BFE>>>>>Похоже, что image_bytes объявлен как указатель на const, если это так — то это зачем?
S>>>>Да, это const std::byte*, приходящий извне.
BFE>Да, это я не правильный вопрос задал. Попытаюсь ещё раз. Почему извне приходить указатель на константные данные?

Объективная реальность. Скажем, какие-то данные берутся из shared-memory, замапленной в память процесса в режиме read-only.

S>>Потому, что мне здесь "итераторы" в каком-либо виде не нужны. Если вам кажется, что нужны, то, наверное, для этого есть какие-то доводы?
BFE>Вот этот цикл:
BFE>

BFE>std::size_t plane_index = 0u;
BFE>for( const auto plane_size : data_plane_sizes )
BFE>{
BFE>    fresh_frame->data[ plane_index ] = ptr_cast( image_bytes );
BFE>    image_bytes += plane_size;
BFE>    ++plane_index;
BFE>}
BFE>

BFE>я бы переписал так:
BFE>

BFE>auto pImgBytes = ptr_cast( image_bytes );
BFE>auto pData     = std::begin(fresh_frame->data);
BFE>for( const auto plane_size : data_plane_sizes )
BFE>{
BFE>    *pData++   = pImgBytes;
BFE>    pImgBytes += plane_size;
BFE>}
BFE>

Можно и так. Только это дело вкуса. Объективных преимуществ ни у одного из способов нет. Процитированный мной код получился в результате эволюции, на предыдущих стадиях которых, вероятно, plane_index был более нужен именно как индекс.

BFE>>>То, что assert-ы не работают в release — опасное заблуждение.
S>>Если assert-ы работают в релизе, то это уже не assert-ы.
BFE>А что тогда?

Обычные run-time проверки но с самым, пожалуй, жестким способом реакции на проблему.

BFE>Вот видите, для понимания вашего кода нужно знать некий внешний контекст. Т.е. гарантии того, что код будет работать лежат где-то ещё, а не в самом коде. В результате код не очень-то хорош.

См. выше про потребность быть в предметной области.

Здравствуйте, so5team, Вы писали:

S>Объективная реальность. Скажем, какие-то данные берутся из shared-memory, замапленной в память процесса в режиме read-only.

Не, ну так не честно! Я думал речь про нормальную программу, а не про хакерство.
И всё равно не понятно, зачем вызывать cast на каждый доступ.

Здравствуйте, B0FEE664, Вы писали:

S>>Объективная реальность. Скажем, какие-то данные берутся из shared-memory, замапленной в память процесса в режиме read-only.

BFE>Не, ну так не честно! Я думал речь про нормальную программу, а не про хакерство.

Хакерство здесь не при чем. Можно смотреть на вещи так: есть подсистема A, у которой откуда-то в памяти появляются данные. Часть этих данных должна попасть к подсистеме B, но только на чтение, поэтому подсистема A отдает подсистеме B const-указатель. Далее подсистеме B нужно сформировать AVFrame, который бы использовал данные по полученному указателю. Но тут засада, в AVBuffer указатель неконстантный. Отсюда и происхождение const_cast.

При этом параллельно с подсистемой B над теми же самыми данными может работать и какая-нибудь подсистема C или D. И они так же не должны ничего по указателю менять, поэтому и к ним так же пойдет const-указатель.

BFE>И всё равно не понятно, зачем вызывать cast на каждый доступ.

Там же не зря image_bytes постоянно инкрементируется, т.к. его результирующее значение важно и используется впоследствии (за пределами процитированного фрагмента).
Посему выбор просто:

1) либо работать с исходным константным image_bytes, применяя cast только в тех местах, где реально нужен неконстантный указатель;
2) либо создать временный неконстантный указатель из image_bytes, работать с ним, а потом его значение возвращать, но уже в виде константного.

Второй вариант мне нравится сильно меньше, т.к. значительно увеличивается область, в которой живет неконстантный указатель на константные, по сути, данные.

Здравствуйте, so5team, Вы писали:

S>Но тут засада, в AVBuffer указатель неконстантный.
Вот я тормоз!
Я правильно понимаю, что данные на самом деле не меняются, а просто в библиотеке не отличают константный буфер от неконстантного?

Здравствуйте, B0FEE664, Вы писали:

S>>Но тут засада, в AVBuffer указатель неконстантный.
BFE>Вот я тормоз!
BFE>Я правильно понимаю, что данные на самом деле не меняются, а просто в библиотеке не отличают константный буфер от неконстантного?

Именно так.

Здравствуйте, Marty, Вы писали:

M>Собери свой проект с /Wall /WX, удивись, сколько варнингов [...]

(Внезапно!) все, кто серьезно относятся к качеству кода с дня нумер один разработки (а не постфактум, да ради прикола), уже собирают со всеми флагами "хорошего тона", и никаких удивлений у них нет.
Иногда еще даже с -Werror впридачу.

Здравствуйте, Marty, Вы писали:

M>Почему сишным кастом пользоваться плохо?

Потому что он умеет втихаря снимать константность и реинтерпретировать. Это позволяет на ровном месте устроить ад и израиль.
Тогда как плюсовые касты явно выражают намерение.

M>Например, я убираю возможную знаковость у char переменной и конвертирую её в unsigned как-то так:
M>

M>auto u = (unsigned)(std::uint8_t)ch;
M>

M>Но можно и так:
M>

M>auto u = static_cast<unsigned>(static_cast<std::uint8_t>(ch));
M>

Но можно и так

constexpr unsigned unsigned_from_char(char c) { return static_cast<unsigned>(static_cast<unsigned char>(c)); }
constexpr unsigned unsigned_from_char(auto) = delete;  // во избежание неявных кастов

...
auto u = unsigned_from_char(ch);
...

M>Во втором случае выражение гораздо менее наглядно, приходится скрипеть мозгами, чтобы понять, что там происходит.

А в третьем случае вызов наглядный, а вся машинерия спрятана под капот, — не приходится скрипеть мозгами, зачем обязательно надо делать два каста, а не сразу единственный (unsigned)ch.

M>Да, для удобства поиска использую именно сишный каст, а не constructor style cast вида int(...)

M>ОбсудиПохоливарим?

Придёт ПМ, включит линтер и-или

-Werror=old-style-cast

(или как оно там в комстроке...) — и холиварь с начальством сколько влезет

Здравствуйте, Marty, Вы писали:

M>Собери свой проект с /Wall /WX, удивись, сколько варнингов будет в простейших битовых выражениях, расставь там плюсовые касты и попробуй это прочитать

Я последние несколько гигантских проектов собирал (в том числе форк хромиума и форк Robotic OS) исключительно с параноидными настройками компилятора.
И ничего.
Сложно первые полгода, пока всё говнище от старых дореформенных разработчиков не вычистишь. Естетственно, не в одно лицо, там трудозатрат на десятки человеко-лет.
А потом — просто не говнокодишь; а линтер и тесты, (а в некоторых случаях — живые ревьюверы) тебе помогают-заставляют.

Здравствуйте, Marty, Вы писали:

M>И это всё как-то работает
Ты же понимаешь, что это вообще не аргумент в разговоре о языковых средствах, которые позволяют уменьшить вероятность ошибки.