l3tnun / EPGStation

Mirakurun を使用した録画管理ソフト
https://twitter.com/l3tnun
MIT License
553 stars 149 forks source link

【質問】encode時の環境変数であるDIRについて #443

Closed konikoni428 closed 3 years ago

konikoni428 commented 3 years ago

環境

Issue

エンコードの環境変数であるDIRはdocsに以下のように記述されています。

変数名 種類 説明
DIR string 予約時に設定した directory 文字列

これより私は指定したサブディレクトリ文字列が渡されると考えました。 しかし実際の動作はサブディレクトリを指定してもしなくてもOUTPUTと変わらない文字列が渡されるので調査しました。

// EncodeManageModel.ts 243-250

// DIR
let dir: string = '';
if (typeof encodeCmd.suffix === 'undefined' && typeof queueItem.directory !== 'undefined') {
    dir = queueItem.directory;
} else if (outputFilePath !== null) {
    dir = outputFilePath;
}

該当の箇所がここだったのですが、なぜifでtypeof encodeCmd.suffix === 'undefined'での分岐を行っているのでしょうか? この変数は.mp4などが入り、undefinedになることはほぼないと思われます。 バグなのか意図的なのか判別がつかないため質問させていただきました。

ここからは提案になってしまうのですが、DIRの動作について

の動作にするのはどうでしょうか?

以上よろしくお願いいたします。

tobitti0 commented 3 years ago

encodeでのsuffixは必須ではないからだと思います。

マニュアルに下記のとおり記載があります。 suffix を空欄にした場合、非エンコードコマンドとして実行される https://github.com/l3tnun/EPGStation/blob/master/doc/conf-manual.md#encode

該当処理部分を読んでいないのでおそらくですが、suffixがない場合はコマンド実行後に生成ファイルを登録する処理をしないようになっていると思います。

よって分岐は意図したものであると思います。

konikoni428 commented 3 years ago

ありがとうございます suffixは必須ではないことを読み落としていました。

ただし、動作に関しては疑問が残っています。 該当処理部分についてもう少し長く引用すべきだったので長めに引用すると、

// src/model/service/encode/EncodeManageModel.ts
// 243-269行目

// DIR
let dir: string = '';
if (typeof encodeCmd.suffix === 'undefined' && typeof queueItem.directory !== 'undefined') {
    dir = queueItem.directory;
} else if (outputFilePath !== null) {
    dir = outputFilePath;
}

this.log.encode.info(`encodeCmd.suffix: ${encodeCmd.suffix}`);
this.log.encode.info(`queueItem.directory: ${queueItem.directory}`);
this.log.encode.info(`outputFilePath: ${outputFilePath}`);

// プロセスの生成
const childProcess = await this.processManager.create({
    input: inputFilePath,
    output: outputFilePath,
    cmd: encodeCmd.cmd,
    priority: EncodeManageModel.ENCODE_PRIPORITY,
    spawnOption: {
        env: {
            ...process.env,
            RECORDEDID: recorded.id.toString(10),
            INPUT: inputFilePath,
            OUTPUT: outputFilePath === null ? '' : outputFilePath,
            DIR: dir,
            FFMPEG: config.ffmpeg,
            FFPROBE: config.ffprobe,
            .
            .
            .

このようになっています。 私が疑問としているdir変数なのですが、DIR環境変数の値の設定のみに使用されており、これ以降使用されていません。 そのため

該当処理部分を読んでいないのでおそらくですが、suffixがない場合はコマンド実行後に生成ファイルを登録する処理をしないようになっていると思います。

この指摘が当てはまらないと考えています。

気が早くPR(#444)を出してしまったのですが

let dir: string = '';
if (typeof queueItem.directory !== 'undefined') {
    dir = queueItem.directory;
}

dir変数についてはDIR環境変数のみ影響を与えるので、このような動作が妥当だと思うのですがいかがでしょうか?

tobitti0 commented 3 years ago

392 による条件分岐のようです。

ただ、エンコード時、非エンコード時に関わらず設定した文字列を渡すようにするのは理想的な処理だと思います。

konikoni428 commented 3 years ago

過去issue探すの失念していました。 ありがとうございます。

392 を見る限り過去から環境変数DIRではourputDirを出力しており、特定の場合(suffixが存在しない非エンコードモード)にはサブディレクトリを渡すという動作が標準だったように思われます

昔からこの動作が正しいのであれば、変更による影響を考慮して新しい環境変数SUBDIRを導入し、そちらにはエンコード時、非エンコード時にかかわらずサブディレクトリの情報を入れてもいいかもしれません

l3tnun commented 3 years ago

doc の 環境変数 DIR の説明が紛らわしかったですね。 @takuo さんのおっしゃるとおりで、#392 による修正で、v1 の頃の挙動に合わせました。

変更による影響を考慮して新しい環境変数SUBDIRを導入し、そちらにはエンコード時、非エンコード時にかかわらずサブディレクトリの情報を入れてもいいかもしれません

とても良いと思います。 プルリクがあれば受け入れます。

l3tnun commented 3 years ago

プルリクありがとうございました。 マージしましたのでクローズします。