読者です 読者をやめる 読者になる 読者になる

なんとなく

誰得感満載な記事が多いかも。Mono関係とLinuxのサーバ関係、レビューとか。

念力デバッグの答え合わせ(RockDisk Nextのminidlnaのバグ関連)

7/21に依頼して、紆余曲折はありましたが、完全ではないのですがRockDisk Nextのminildnaのソースを開示してもらえました。

RockDiskNextのminidlnaのバグとか - なんとなく

において念力デバッグしたものと答え合わせをしようと思います。

2GB以上のDSFファイルがタグ取得に失敗

DSFのヘッダにあるPointer to Metadata chunkのデータを取得し、fseekoを使わず、fseekを使っているからおかしいのかと想像したけど、ログのエラーはファイルを開く際のエラーだったということを記述しました。 ソースを見たところ、当初想像したようにPointer to Metadata chunkをunsigned long longの型の変数に格納しているものの、fseeko使ってませんでした。

ログのエラーよりfopenでエラーを返しているように見えた理由は、

  • /tmp/id3.tagができているのが気持ち悪かったので、1分おきに存在を確認して、存在した場合に削除するということを私がしていた
  • /tmp/id3.tagが開けないエラーとして、スキャンしているファイルが開けないというメッセージを出力していた

ためでした。

システムが不安定に、スキャンの時間がやたら長い

上述した続きですが、想像したようにfseekを使っているため、実装したプログラマーが想定した動作をしないようになっていました。

ソースを見るとタグを処理するために

  • Pointer to Metadata chunkを取得する
  • Pointer to Metadata chunkまでfseekする
  • その後、1024バイトずつEOFになるまで、/tmp/id3.tagに書き出す

ということをやっていたので、fseekに失敗するとファイルのoffsetはそのままです。そのため、SEEK_CUR(ファイルの現在地)からそのファイルのEOFまでの容量を書き出す処理をすることになります。発生条件がPointer to Metadata chunkが2147483647Lより大きいわけですから、書き出す容量もそれなりに大きいです。RockDisk Nextの/にはそれを満たすほどの容量はないため、書き出している間で/の容量がいっぱいになってしまったら、システムは不安定になるようです。

参考までにDSFのヘッダの一部は以下のようになっています。

4 byte 'D' , 'S' , 'D', ' ' (includes 1 space) DSD chunk header
8 byte Size of this chunk byte 28
8 byte Total file size byte
8 byte Pointer to Metadata chunk byte If Metadata doesn’t exist, set 0. If the file has ID3v2 tag, then set the pointer to it. ID3v2 tag should be located in the end of the file.

詳しくは http://dsd-guide.com/sites/default/files/white-papers/DSFFileFormatSpec_E.pdf を参照してください

私の環境では一部NFSでマウントしていたところをminidlnaにscanさせていたので、書き出す速度が転送速度に引っ張られ、2GB程度で11分ほどかかっていました。そのため、システムが不安定に、スキャンの時間がやたら長いという現象が発生していたようです。

ですので、HDDのみで運用されている方は、私のようにDSFファイル400個程度のスキャンに200分程度かかることはまずないと思います。 ただ、2GB以上のファイルが多いとスキャンにやや時間がかかり、その間システムが不安定にはなると思います。

改修してみましたが、fseekをfseekoにするだけだではなく、

unsigned long long id3TagPoint = 0;
id3TagPoint = (buffer[0] & 0xff) | (buffer[1] & 0xff) << 8 | (buffer[2] & 0xff) << 16 | (buffer[3] & 0xff) << 24 ;

となっているのを8byteビットシフトするようにして、明示的にunsigned long longの整数定数ULLを付加してあげないとダメでした。私の場合はuint64_tを使っていて整数定数を気にしていなかったので、ちょっとハマったのですが、良い勉強になりました。 これで、200分程度だったスキャンが1分以内で終わるようになりました。

tagutils-dsf.cにおいて以下。

@@ -34,14 +34,14 @@ _get_dsftags(char *file, struct song_metadata *psong)
    fseek(infile, 20  , SEEK_SET);
-   fread(buffer, 1, 4, infile );
+   fread(buffer, 1, 8, infile );
 
-   id3TagPoint = (buffer[0] & 0xff) | (buffer[1] & 0xff) << 8 | (buffer[2] & 0xff) << 16 | (buffer[3] & 0xff) << 24 ;
+   id3TagPoint = (buffer[0] & 0xffULL) | (buffer[1] & 0xffULL) << 8ULL | (buffer[2] & 0xffULL) << 16ULL | (buffer[3] & 0xffULL) << 24ULL| (buffer[4] & 0xffULL) << 32ULL | (buffer[5] & 0xffull) << 40ULL |(buffer[6] & 0xffULL) << 48ULL | (buffer[7] & 0xffULL) << 56ULL ;
 
    //DEBUG    DPRINTF(E_DEBUG,L_SCANNER,"id3TagPoint : %lld\n",id3TagPoint);
    //DEBUG    DPRINTF(E_DEBUG,L_SCANNER,"id3TagPoint : 0x%08x\n",id3TagPoint);
 
    // Jump to the Position of ID3 Tag & read
    if (id3TagPoint > 0) {
-       fseek(infile, id3TagPoint  , SEEK_SET);
+       fseeko(infile, id3TagPoint  , SEEK_SET);
        memset(buffer , 0, sizeof(buffer));
        //Dump out to /tmp/id3.tag

Latin-1の拡張文字(áとかäとかéなどなど)がファイル名に含まれているとタグ取得に失敗

こちらについては、ソースを見るとそういうことは発生しないようです。これは、おそらく上述した/tmp/id3.tagを定期的に削除していたのが悪さをしていたようです。音楽のファイルですので、ファイル名にLatin-1が含まれていることが多く、たまたまログにエラーとして出力されたファイルにLatin-1が含まれているだけだったようです。大変失礼いたしました。 言い訳を書かせていただくと、スキャンに時間がかかるので、エラーの出るファイルが常に同じファイルかどうか調べませんでした。すみませんでした。

12分41秒以上の曲の再生時間(Duration)が表示されない。。。(DSD64で)

これは、プログラム中で変数の型を64bitで扱わなければいけないのを32bitの型で扱っていると想像した通り、longの型変数に64bitであるSample countを突っ込んでました。そのため(SampleCount/SamplingFrequency)*1000が正しく計算されず、DBには正しく計算されていない値が格納されいたようです。

対処としては、変数の型をlongではなくunsigned long longにし、上述したように整数定数ULLを付加してあげました。

9/16追記:よく見たらSampleCountではなくDataSizeを取得していました。それで(DataSize/SamplingFrequency)*1000と計算していました。それで再生時間を正しく計算できるということについて私がまだ理解できていないです。 チャンネル数で割ったりSamplingFrequencyは1bitなのでbyteに合わせるために8倍するとかしていないようですし。

dsfの仕様書にも

[Annotation2]

Sample count is the num per 1 channel.

Ex) n second data: Sample count would be Sampling frequency * n

という記述があります。

http://dsd-guide.com/sites/default/files/white-papers/DSFFileFormatSpec_E.pdf

9/30追記:その後、ソースがDataSizeとなっているだけで、ヘッダについてはSampleCountのところを4byte分取得していました。本来SampleCountは8byteと定義されているので、8byteを取得しないと4byte以上の値に対して不都合が生じます。ですので、変数の型をlongではなくunsigned long longにし、上述したように整数定数ULLを付加してあげる他に、ヘッダを8byte取得するようにしなければなりませんでした。

tagutils-dsf.cにおいて以下。

@@ -319,11 +319,11 @@ static int _get_dsffileinfo(char *file, struct song_metadata *psong)
 
    char *buffer = NULL;
 
-   long FileSize=0;
+   unsigned long long FileSize=0;
 //    int FileSize_H=0;
 //    int ChannelType = 0;
    int ChannelNum = 0;
-   long SongSize = 0;
+   unsigned long long SongSize = 0;
 //    int SongSize_H = 0;
    long length = 0 ;
    long SampleFreq = 0;

@@ -340,7 +340,7 @@ static int _get_dsffileinfo(char *file, struct song_metadata *psong)
    }
    
    fread(buffer, 1, 28, infile);
-   FileSize = (buffer[12]&0xff)|((buffer[13]&0xff)<< 8 )|((buffer[14]&0xff)<< 16)|((buffer[15]&0xff)<<24);
+   FileSize = (buffer[12]&0xffULL)|((buffer[13]&0xffULL)<< 8ULL )|((buffer[14]&0xffULL)<< 16ULL)|((buffer[15]&0xffULL)<<24ULL)|(buffer[16]&0xffULL)<<32ULL|((buffer[17]&0xffULL)<< 40ULL )|((buffer[18]&0xffULL)<< 48ULL)|((buffer[19]&0xffULL)<<56ULL);
 //    FileSize_H = (buffer[16]&0xff)|((buffer[17]&0xff)<< 8 )|((buffer[18]&0xff)<< 16)|((buffer[19]&0xff)<<24);
 
    fseek(infile, 28, SEEK_SET);

@@ -354,7 +354,7 @@ static int _get_dsffileinfo(char *file, struct song_metadata *psong)
    fseek(infile, 60, SEEK_SET);
 
    fread(buffer, 1, 12, infile);
-   SongSize = (buffer[4]&0xff)|((buffer[5]&0xff)<< 8 )|((buffer[6]&0xff)<< 16)|((buffer[7]&0xff)<<24);
+   SongSize = (buffer[4]&0xffULL)|((buffer[5]&0xffULL)<< 8ULL )|((buffer[6]&0xffULL)<< 16ULL)|((buffer[7]&0xffULL)<< 24ULL)|(buffer[8]&0xffULL)<< 32ULL|((buffer[9]&0xffULL)<< 40ULL )|((buffer[10]&0xffULL)<< 48ULL)|((buffer[11]&0xffULL)<<56ULL);
 //    SongSize_H = (buffer[8]&0xff)|((buffer[9]&0xff)<< 8 )|((buffer[10]&0xff)<< 16)|((buffer[11]&0xff)<<24);
 
    length = SongSize / SampleFreq;

バグかわからないけど、録音日時のロケールがおかしい。

バグかもしれないし、minidlnaの仕様かもと想像していました。 これもDETAILSテーブルにinsertする際にyyyy-dd-mmとしており、バグでした。データの出力する際に日付は100で割ったもの、月は100で割ったあまりを計算して出力していたのですが、順番を間違えているだけのようでした。

ID3のTDATはDDMMのフォーマットで格納されるのですが、その格納を当方が間違えてMMDDでしていたためで、これはバグではありませんでした。 大変失礼いたしました。(追記:2015/10/28)

そのため、yyyy-mm-ddの順番に変更しました。 metadata.cにおいて以下。

@@ -435,7 +435,7 @@ GetAudioMetadata(const char *path, char *name)
        if (song.date ==0 && song.time ==0)
            xasprintf(&m.date, "%04d-01-01,00:00", song.year);
        else
-           xasprintf(&m.date, "%04d-%02d-%02d,%02d:%02d", song.year, song.date/100, song.date%100, song.time/100, song.time%100);
+           xasprintf(&m.date, "%04d-%02d-%02d,%02d:%02d", song.year, song.date%100, song.date/100, song.time%100, song.time/100);
    }
 #else  // Old Version
        xasprintf(&m.date, "%04d-01-01", song.year);

他にもバグを発見した。

ざっとソースを見て、ファイルが2GBより大きい場合の弊害として、fseekのoffsetに2147483647Lより大きい値が入ることによるバグが有りました。それ以外のものもありました。

前者は

  • dffでファイルサイズを取得できない。
  • dffで曲の長さを表示できない。

後者は

  • dffでDST形式圧縮された形式の場合、曲の長さの計算がおかしい。

でした。

dffでファイルサイズを取得できないのは、ファイルサイズを計算する際に、fseeko,ftelloを使っていないため起こっているようで、更に曲の長さの計算にもファイルサイズを使用しているため、曲の長さも表示できないということが起こっていました。(正確な曲の長さの求め方ではないのですが、ほぼ同じ値が出るので気にならない程度ではありますが。) また、DST形式の場合、曲の長さの計算がおかしいというのはDSD形式なのかDST形式なのかを判定せずにdffの曲の長さはすべてファイルサイズを使用して計算していたため発生していました。これはファイルの容量に関係なく発生します。 DSTの場合は圧縮されているので、フレーム数をフレームレートで割ってあげて曲の長さを求める必要があります。

こちらは、まだ修正していません。dffの情報を取得する実装が仕様書と比較するとこれでよいのか?という部分が結構あってためらわれます。。。

外部に作成を依頼したようだけど

開示されたソースの著作はinXtron,Incというところでした。 RockDisk Nextは中身は、inXtron MyCloud Miniと同じようで、OEM元にDSD対応を依頼したように見えます。 ソースを開示して中身を見て、個人的にはリリースレベルには達していないように感じます。機器の動作確認のテストはしているのに、同時に正しく表示されるかについては確認しないのでしょうか。。。ベータ版の期間も半年以上あったようですが。。。

あんまり、強く突っ込むとお前のはどうなんだよとなるので、これくらいで。。。debianのパッケージからforkしたものは、disable_nlsにするかな。。。

GPLなものを改変するのにお金をかけたのでしょうか。購入したユーザとしてはうれしいのですが、企業としての資産にはならず、公共に帰す形になりますね。それを見越した上でそんなに質の高いものではないのかもしれませんね。

ソースを開示していただけただけでもいいのかなぁと考えるべきなのでしょうか。 おそらく、使えるようにRockDisk Next側のバイナリ改変すると保証対象外になったりすることでしょう。そもそもminidlnaは保証の対象なんだろうかという疑問もあります。保証規定とGPLのソース開示の書面による申し出は同梱してほしいものです。

なお、今回の改修やテストはRaspberryPi上でしてます。ハード環境的にはRockDisk Nextとほぼ同様の環境です。RaspberryPiの環境のほうがソフトウエア的にはかなり新しいです。

今後

きちんと完全なソースが開示いただけたら、どこかに公開して、修正したところについてはIO-DATAさんにも送ろうかと思います。 まぁ、修正した部分は、わたしでもすぐ直せたので、指摘すれば作成した方ならすぐ直せるんですけどね。。。

明らかにバグな部分は改修して新ファームとしてリリースして欲しいんですけどね。たぶん、保証のこととか考えなくてよくなるだろうし。

完全なソースは開示できないとのことだったので、一部開示されたソースを公開しておきます。 minidlna-20140415.tar.zip - Google ドライブ

また、開示されたよく理解できない使われているOSSのリストも公開しておきます。 RockdiskNext_Opensourcelist.pdf - Google ドライブ

ソースはリンク先からと入手してとするのであれば、配布しているバイナリに対応する完全なソースコードである必要があるし、IOdataが管理できるリンク先ではないといけないはずなのにそうなってないので、理解に苦しみます。