From c77654d0fed9c100cc5f1bc6c684caab20f0a37a Mon Sep 17 00:00:00 2001
From: Romain <rprieto@users.noreply.github.com>
Date: Wed, 5 Apr 2023 21:35:01 +0000
Subject: [PATCH] feat: enforce bitrate option with HW acceleration

---
 src/cli/options.js       | 16 ++++++++++++++--
 test/cli/options.spec.js | 12 ++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/cli/options.js b/src/cli/options.js
index b86bd86..2ee301d 100644
--- a/src/cli/options.js
+++ b/src/cli/options.js
@@ -106,7 +106,7 @@ const OPTIONS = {
   },
   'video-hwaccel': {
     group: 'Output options:',
-    description: 'Use Hardware acceleration, requires bitrate too',
+    description: 'Use hardware acceleration (requires bitrate)',
     choices: ['none', 'vaapi'],
     'default': 'none'
   },
@@ -444,7 +444,7 @@ const OPTIONS = {
 
 // explicitly pass <process.argv> so we can unit test this logic
 // otherwise it pre-loads all process arguments on require()
-exports.get = (args) => {
+exports.get = (args, exitOnFailure = true) => {
   const parsedOptions = yargs(args)
     .usage(messages.USAGE())
     .wrap(null)
@@ -452,6 +452,8 @@ exports.get = (args) => {
     .config('config')
     .options(OPTIONS)
     .epilogue(messages.CONFIG_USAGE())
+    .exitProcess(exitOnFailure)
+    .check(validation)
     .argv
 
   // Warn users when they use deprecated options
@@ -512,6 +514,16 @@ exports.get = (args) => {
   return opts
 }
 
+function validation (opts) {
+  // all custom validation rules go here
+  // this way, they are reported the same way as invalid arguments
+  if (opts.videoHwaccel !== 'none' && !opts.videoBitrate) {
+    throw new Error('--video-hwaccel requires a value for --bitrate')
+  }
+  // everything is OK
+  return true
+}
+
 function replaceInArray (list, match, replacement) {
   for (let i = 0; i < list.length; ++i) {
     if (list[i] === match) {
diff --git a/test/cli/options.spec.js b/test/cli/options.spec.js
index 911773c..cc4878d 100644
--- a/test/cli/options.spec.js
+++ b/test/cli/options.spec.js
@@ -39,6 +39,11 @@ describe('options', function () {
       const opts2 = options.get(BASE_ARGS.concat(['--include-videos', 'FALSE']))
       should(opts2.includeVideos).eql(true)
     })
+    it('rejects invalid values for choices', () => {
+      should.throws(function () {
+        options.get(BASE_ARGS.concat(['--video-format', 'test']), false)
+      }, /mp4/)
+    })
   })
   describe('paths', () => {
     it('--input is converted to an absolute path', () => {
@@ -103,6 +108,13 @@ describe('options', function () {
       should(opts.gmArgs).eql(['-equalize', '-modulate 120'])
     })
   })
+  describe('video', () => {
+    it('hardware acceleration requires a bitrate', () => {
+      should.throws(function () {
+        options.get(BASE_ARGS.concat(['--video-hwaccel', 'vaapi']), false)
+      }, /bitrate/)
+    })
+  })
   describe('misc', () => {
     describe('database file path', () => {
       it('defaults to the output folder', () => {