Skip to content

Commit

Permalink
Fix wrong default quality
Browse files Browse the repository at this point in the history
Also adapted tests and README. The quality should really be an integer from 1-31.
See https://trac.ffmpeg.org/wiki/Encode/MPEG-4. Thanks to @LaKing for reporting the issue.

Fixes #9.
  • Loading branch information
tpraxl committed Jun 14, 2018
1 parent 578d5be commit fc62356
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 20 deletions.
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ convert-footage will not convert files twice and it will not convert your conver
#
# Options:
# -h ... help message
# -q n ... quality of the encoded video. Defaults to 0 for
# -q n ... quality of the encoded video. Defaults to 1 for
# best quality.
# -e ... show all example usages
#
Expand All @@ -96,17 +96,17 @@ convert-footage will not convert files twice and it will not convert your conver

convert-footage .

# Convert the current folder with quality 1
# Convert the current folder with quality 2

convert-footage -q 1 .
convert-footage -q 2 .

# Convert folder ../myvideos with best quality (default)

convert-footage ../myvideos

# Convert file ./myvideo.mp4 with quality 1
# Convert file ./myvideo.mp4 with quality 2

convert-footage -q 1 ./myvideo.mp4
convert-footage -q 2 ./myvideo.mp4

# Show help

Expand Down
20 changes: 10 additions & 10 deletions convert-footage
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#
# Options:
# -h ... help message
# -q n ... quality of the encoded video. Defaults to 0 for
# -q n ... quality of the encoded video. Defaults to 1 for
# best quality.
# -e ... show all example usages
#
Expand All @@ -31,7 +31,7 @@ set -o nounset

main() {
#defaults
local quality=0
local quality=1
#getopts initialization for usage in function
local OPTIND opt

Expand All @@ -47,8 +47,8 @@ main() {
;;
q)
quality="${OPTARG}"
if ! is_positive_integer "${quality}"; then
print_error "Quality needs to be a positive integer. The lower the value, the better the quality."
if ! is_between_1_and_31 "${quality}"; then
print_error "Quality needs to be a positive integer between 1 and 31. The lower the value, the better the quality."
print_usage
exit 1
fi
Expand Down Expand Up @@ -94,17 +94,17 @@ print_examples() {
convert-footage .
# Convert the current folder with quality 1
# Convert the current folder with quality 2
convert-footage -q 1 .
convert-footage -q 2 .
# Convert folder ../myvideos with best quality (default)
convert-footage ../myvideos
# Convert file ./myvideo.mp4 with quality 1
# Convert file ./myvideo.mp4 with quality 2
convert-footage -q 1 ./myvideo.mp4
convert-footage -q 2 ./myvideo.mp4
# Show help
Expand All @@ -123,8 +123,8 @@ print_error()
(>&2 echo -e "\033[91m${1}\033[0m")
}

is_positive_integer() {
[[ ${1} =~ ^[0-9]+$ ]]
is_between_1_and_31() {
[[ "${1}" =~ ^[0-9]+$ ]] && [ "${1}" -ge 1 -a "${1}" -le 31 ]
}

convert_folder() {
Expand Down
16 changes: 11 additions & 5 deletions test/command-execution.bats
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ assert_usage_in() {
assert_examples_in() {
local output="$1"
[[ "${output}" == *"# Convert current folder with best quality"* ]]
[[ "${output}" == *"# Convert the current folder with quality 1"* ]]
[[ "${output}" == *"# Convert the current folder with quality 2"* ]]
[[ "${output}" == *"# Convert folder ../myvideos with best quality (default)"* ]]
[[ "${output}" == *"# Convert file ./myvideo.mp4 with quality 1"* ]]
[[ "${output}" == *"# Convert file ./myvideo.mp4 with quality 2"* ]]
[[ "${output}" == *"# Show help"* ]]
}

@test "calling convert-footage with illegal -q value reports useful error message" {
local expected="Quality needs to be a positive integer. The lower the value, the better the quality."
local expected="Quality needs to be a positive integer between 1 and 31. The lower the value, the better the quality."
run ./convert-footage -q a
[ "$status" -eq 1 ]
[[ "${output}" == *"${expected}"* ]]
Expand All @@ -50,6 +50,14 @@ assert_examples_in() {
[ "$status" -eq 1 ]
[[ "${output}" == *"${expected}"* ]]

run ./convert-footage -q 0
[ "$status" -eq 1 ]
[[ "${output}" == *"${expected}"* ]]

run ./convert-footage -q 32
[ "$status" -eq 1 ]
[[ "${output}" == *"${expected}"* ]]

run ./convert-footage -q
[ "$status" -eq 1 ]
assert_usage_in "${output}"
Expand Down Expand Up @@ -111,5 +119,3 @@ remove_test_dir() {
# Afterwards, we clean up the test directory
remove_test_dir "${test_dir}"
}


0 comments on commit fc62356

Please sign in to comment.