fixedpoint.jp


ケーススタディ -- ffmpeg の開発者は C をどう扱うか (2008-01-31)

ffmpeg-cvslog では複数の開発者によるコミットログとともに、それに対する議論が並行して行われています。そして議論の結果はすぐにソースコードに反映されます。

今日見かけた議論は1行の変更からなるコミットから始まりました;

http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/2008-January/011368.html
Author: bcoudurier
Date: Thu Jan 31 01:05:44 2008
New Revision: 11677

Log:
init to 0, fix warning: h263.c:4730: warning: 'dc_pred_dir' may be used uninitialized in this function

Modified:
   trunk/libavcodec/h263.c

Modified: trunk/libavcodec/h263.c
==============================================================================
--- trunk/libavcodec/h263.c(original)
+++ trunk/libavcodec/h263.cThu Jan 31 01:05:44 2008
@@ -4727,7 +4727,7 @@ static inline int mpeg4_decode_block(Mpe
                               int n, int coded, int intra, int rvlc)
 {
     int level, i, last, run;
-    int dc_pred_dir;
+    int dc_pred_dir = 0;
     RLTable * rl;
     RL_VLC_ELEM * rl_vlc;
     const uint8_t * scan_table;

すなわち、「ローカル変数dc_pred_dirが初期化されずに使われているかも」という gcc 警告に従って、bcoudurier は1行の変更を行いました。

それに対して即座にプロジェクトリーダーの Michael Niedermayer が問題提起します;

http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/2008-January/011379.html
I would consider this function speed critical, after all its executed once per
8x8 block, that is 216000 times per second for 640x480 YV12 30fps in the
worst case.

彼は初期化する/しないがプログラムの実行速度の点でクリティカルに効いてくるということを数字でアピールします。bcoudurier はこれを聞いて恐れをなしてリバートします:

http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/2008-January/011381.html
I was fearing that, reverted.

一方、Michael は上の警告を抑制する別の方法を提案します;

http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/2008-January/011382.html
Author: michael
Date: Thu Jan 31 02:38:00 2008
New Revision: 11689

Log:
fix "uninitalized" warning, the alternative way.

I hope this is valid C, flame me if not.
md5sum of h263.o doesnt change, so this doesnt cause a slowdown.


Modified:
   trunk/libavcodec/h263.c

Modified: trunk/libavcodec/h263.c
==============================================================================
--- trunk/libavcodec/h263.c(original)
+++ trunk/libavcodec/h263.cThu Jan 31 02:38:00 2008
@@ -4727,7 +4727,7 @@ static inline int mpeg4_decode_block(Mpe
                               int n, int coded, int intra, int rvlc)
 {
     int level, i, last, run;
-    int dc_pred_dir;
+    int dc_pred_dir= dc_pred_dir;
     RLTable * rl;
     RL_VLC_ELEM * rl_vlc;
     const uint8_t * scan_table;

元のコードと同じバイナリを生むこのトリックは gcc のバグだとして、Måns Rullgår と Ismail Dönmez の2人が苦情を言います;

http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/2008-January/011384.html
Valid or not, the variable is just as uninitialised as before.  If
this silences a warning, that is a bug in gcc.

Weird-looking code like this should probably have a comment to avoid
confusing future readers.
http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/2008-January/011387.html
As far as I know 

int x = x;

is undefined and gcc is free to do whatever.

それを受けて Michael はこの奇妙な行の意図を説明するコメントを加えましたが、やはり承服出来ない Rich が意見を出します;

http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/2008-January/011392.html
This code is INVALID C and needs to be removed. A program containing
this line has undefined behavior. Writing invalid C is not a
legitimate way to silence gcc's bullshit warnings.

この後もいくつかのポストがあり、最終的に上の一連の変更は却下されました;

http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/2008-January/011418.html
Author: michael
Date: Thu Jan 31 15:01:33 2008
New Revision: 11694

Log:
Revert r11689 and r11690 (uninitalized warning fix) as its theoretically
undefined in C.


Modified:
   trunk/libavcodec/h263.c

Modified: trunk/libavcodec/h263.c
==============================================================================
--- trunk/libavcodec/h263.c(original)
+++ trunk/libavcodec/h263.cThu Jan 31 15:01:33 2008
@@ -4727,7 +4727,7 @@ static inline int mpeg4_decode_block(Mpe
                               int n, int coded, int intra, int rvlc)
 {
     int level, i, last, run;
-    int dc_pred_dir= dc_pred_dir; //weird init to prevent uninitalized warning
+    int dc_pred_dir;
     RLTable * rl;
     RL_VLC_ELEM * rl_vlc;
     const uint8_t * scan_table;

さて、このケースは ffmpeg のプログラムとしての以下の特徴を如実に表しています:

いずれにしても、開発のスタイルとして

という点が素晴らしいと思います。

(何よりメーリングリストでこういった情報が公開されているということが素晴らしい。)


© 2006-2023 fixedpoint.jp