Handle media files that has an audio stream whose duration is shorter than format=duration #75
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed that we are currently requesting
format=duration
fromffprobe
and we use it as the duration of the audio.format=duration
seems to return the duration of the stream with the longest duration in the media file. Some media files might have a video stream that is longer than the audio stream (see Experiment 1 below),format=duration
returns the duration of the video stream for such files, this is not what we want.subed-waveform.el
creates the waveforms from an audio stream, so we should specifically use the duration of the audio stream instead of the video stream.subed-config.el
defines the variablessubed-video-extensions
(link). In all of those extensions, the duration of the audio stream is stored in the fieldstream=duration
. However, for*.mkv
and for*.webm
files, the duration of the audio stream is stored in the fieldstream_tags=duration
. See Experiment 2 below.To sum up, the changes in this pull request make sure that we correctly get the duration of the audio stream and store it in
subed-waveform-file-duration-ms-cache
.I made sure that the tests were run successfully.
PS: I also added
lexical-binding: t
in the tests files, because buttercup was showing some errors. A user opened an issue regarding this, see #74Experiment 1: File with an audio stream that is shorter than the video stream
The following command creates a
.mp4
file that has 1 audio stream and 1 video stream. The audio stream is shorter than the video stream.We can use
ffprobe
to display the information on the format and the streams.As we can see below, the field
duration
under the fieldformat
equals the duration of the video stream.Experiment 2:
ffprobe
field where the duration of the audio stream is stored for different media file typesIn "Experiment 1", we saw that we shouldn't consider
format=duration
as the duration of the audio stream. Instead, we should directly use the duration of the audio stream.The script shown below creates 6 files (1 file for each of the following extensions: *.mkv, *.mp4, *.webm, *.avi, *.ts, *.ogv). Each of the files has 1 video stream with 5 seconds duration and 1 audio stream with 3 seconds duration.
We then use
ffprobe
to query the duration of the audio stream using the fieldsstream=duration
andstream_tags=duration
. In the output, we can see that for*.mkv
and*.webm
files, the duration is not stored instream=duration
, but instead instream_tags=duration
. In the introduced changes, we create a special case for *.mkv and *.webm files (link to relevant part).We can also see that
*.ts
is the only extension that shows two durations for the audio stream. See Experiment 3 below.Experiment 3: Output by
ffprobe
when providing a*.ts
fileIn "Experiment 2", we saw that the
ffprobe
returned two durations for the extension*.ts
even though we selected the audio stream through the flagselect_streams a
.The command in the code block below creates one *.ts file with a single audio stream and a single video stream. In the output, we can see the field
duration
is reported once for the audio stream.In the introduced changes, we query and parse the entire JSON that is returned when using the flags
-show_streams
and-show_format
just as shown in the output below (link to relevant part) Therefore, we don't need to specially handle the duration returned byffprobe
for*.ts
.