Assigns standard values to matroska video properties #251

nkaplan4250 · 2018-08-27T18:34:14Z

This application checks if a matroska video is consistent with a set of
standards properties and if so, it will assign expected values to other
properties that are either undefined or differ from expectations. The
properties that will be changed to their expected values are as
follows:
Interlaced = 1
Display width = 4
Display height = 3
Display unit = 3
Colour range = 1
Colour transfer = 1
Colour primaries = 6
If these properties are already defined but differ from the value above
the user will be asked if they would like to change the assigned value
to the expected value. Using -f will force the changes to the
properties regardless of existing values and without requesting any
further user input. Using -r will retain any existing values assigned
to properties without requiring user input. Undefined properties will
be assigned their expected values in either case.

This application checks if a matroska video is consistent with a set of standards properties and if so, it will assign expected values to other properties that are either undefined or differ from expectations. The properties that will be changed to their expected values are as follows: Interlaced = 1 Display width = 4 Display height = 3 Display unit = 3 Colour range = 1 Colour transfer = 1 Colour primaries = 6 If these properties are already defined but differ from the value above the user will be asked if they would like to change the assigned value to the expected value. Using -f will force the changes to the properties regardless of existing values and without requesting any further user input. Using -r will retain any existing values assigned to properties without requiring user input. Undefined properties will be assigned their expected values in either case.

dericed · 2018-08-29T13:18:21Z

standardguess

+#!/bin/bash
+#checks to see if a video's properties are consistent with a set of standards and if so it assigns expected values to other properties if they are undefined or differ from expectations 
+
+SCRIPTDIR=$(dirname $(which "${0}"))


worth quoting the $() layer too, such as SCRIPTDIR="$(dirname $(which "${0}")")

dericed · 2018-08-29T13:20:07Z

standardguess

+. "${SCRIPTDIR}/mmfunctions" || { echo "Missing '${SCRIPTDIR}/mmfunctions'. Exiting." ; exit 1 ;};
+
+_get_frame_rate(){
+    FRAMERATE=$(ffprobe -i "${1}" -v error -select_streams v:0 -show_entries stream=avg_frame_rate -of default=noprint_wrappers=1:nokey=0 | grep "^avg_frame_rate=" | cut -d = -f 2)


should add a dependency check on the tools used, such as ffprobe, mkvinfo, and mkvpropedit. The dependencies are listed in an array such as

mm/fix_rewrap

Line 6 in 437c6c7

DEPENDENCIES=(ffmpeg ffprobe)

and then checked with a function from mmfunction such as

mm/fix_rewrap

Line 21 in 437c6c7

_check_dependencies "${DEPENDENCIES[@]}"

Also is -v error useful here, it may just make the output noisy.

If you set nokey=1 then you won't need to pipe to grep and cut to parse the value.

I think that -v error tells ffprobe to report errors only. Without it prints out all the version info for ffmpeg, ffprobe, etc...

dericed · 2018-08-29T13:22:43Z

standardguess

+    EXPECTATION="$3"
+    MKVINFO_PROP=$(mkvinfo -t "${1}")
+    SELECTED_ELEMENT=$(echo "${MKVINFO_PROP}" | grep -c "+ ${ELEMENT}:")
+    SELECTED_ELEMENT_VALUE=$(echo "${MKVINFO_PROP}" | grep -i "+ ${ELEMENT}: " | cut -d: -f 2 | sed 's/ //g' | sed 's/(aspectratio)//g')


why is aspectratio referenced like this?

aspect ratio shows up in the MKVinfo report next to the assigned value. I used sed to take it out and just get the value but now I'm thinking that if there was any other value assigned to the display unit it might the definition too, so i have changed that line to:

SELECTED_ELEMENT_VALUE=$(echo "${MKVINFO_PROP}" | grep -i "+ ${ELEMENT}: " | cut -d: -f 2 | cut -d" " -f 2)

dericed · 2018-08-29T13:24:12Z

standardguess

+    echo
+    echo "Height"
+    _MKV_Prop_Check "${INPUT}" "Display height" "3"
+    _MKV_HEIGHT "${INPUT}"


These prior two lines seem to do the same thing.

dericed · 2018-08-29T13:24:55Z

standardguess

+}
+_MKV_PRIMARIES(){
+    MKVPRIMARIES=$(_MKV_Prop_Check "${1}" "Colour primaries" "6")
+}


Are these above functions needed, from _MKV_Interlaced through _MKV_PRIMARIES? It seems that when they are used below the _MKV_Prop_Check version is also used, so many tests happen twice.

dericed · 2018-08-29T13:26:08Z

standardguess

+    fi
+}
+_usage(){
+    echo "This application checks if a matroska video is consistent with a set of standards properties and if so, it will assign expected values to other properties that are either undefined or differ from expectations. The properties that will be changed to their expected values are as follows:"


IIUC this would only apply these properties if the input is NTSC. If so then the usage should clarify this.

dericed · 2018-08-29T13:27:10Z

standardguess

+        continue
+    fi
+
+


lots of multiple consecutive line breaks in this doc can be removed.

dericed · 2018-08-29T13:27:29Z

standardguess

+#EXPECTATION is expected value of property
+#MKVINFO_PROP generates mkvINFO report
+#SELECTED_ELEMENT checks to see if property is present in the mkvINFO report
+#SELECTED_ELEMENT_VALUE checks the value of the value of the property


keep indentation consistent

dericed · 2018-08-29T13:28:51Z

standardguess

+        fi
+    else 
+        echo "${ELEMENT} already set to expected value of ${EXPECTATION}"
+    fi


The above section seems to be re-used within the context of several different properties. It make reduce line count and make this easier to maintain if this section was adjusted to be a generic function which could be re-used within each context.

dericed · 2018-08-29T13:29:16Z

standardguess

+        echo "${ELEMENT} already set to expected value of ${EXPECTATION}"
+    fi
+done
+exit


An exit as a last line is not needed.

In addition to a minor additions the following changes were made: -Dependencies added and checked -Redundant functions and variables were eliminated -Usage clarifies that this is application if for NTSC inputs -_ASSESSMENT function added

dericed reviewed Aug 29, 2018

View reviewed changes

Updated to address comments

f34f324

In addition to a minor additions the following changes were made: -Dependencies added and checked -Redundant functions and variables were eliminated -Usage clarifies that this is application if for NTSC inputs -_ASSESSMENT function added

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Assigns standard values to matroska video properties #251

Assigns standard values to matroska video properties #251

nkaplan4250 commented Aug 27, 2018

dericed Aug 29, 2018

dericed Aug 29, 2018

dericed Aug 29, 2018

dericed Aug 29, 2018

nkaplan4250 Aug 29, 2018

dericed Aug 29, 2018

nkaplan4250 Aug 29, 2018

dericed Aug 29, 2018

dericed Aug 29, 2018

dericed Aug 29, 2018

dericed Aug 29, 2018

dericed Aug 29, 2018

dericed Aug 29, 2018

dericed Aug 29, 2018

Assigns standard values to matroska video properties #251

Are you sure you want to change the base?

Assigns standard values to matroska video properties #251

Conversation

nkaplan4250 commented Aug 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment